[SPARK-2069] MIMA false positives

Fixes SPARK 2070 and 2071

Author: Prashant Sharma <prashant.s@imaginea.com>

Closes #1021 from ScrapCodes/SPARK-2070/package-private-methods and squashes the following commits:

7979a57 [Prashant Sharma] addressed code review comments
558546d [Prashant Sharma] A little fancy error message.
59275ab [Prashant Sharma] SPARK-2071 Mima ignores classes and its members from previous versions too.
0c4ff2b [Prashant Sharma] SPARK-2070 Ignore methods along with annotated classes.

1 files changed, 67 insertions, 56 deletions

diff --git a/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala b/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala
index 6a261e19a3..03a73f92b2 100644
--- a/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala
+++ b/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala
@@ -40,74 +40,78 @@ object GenerateMIMAIgnore {
   private val classLoader = Thread.currentThread().getContextClassLoader
   private val mirror = runtimeMirror(classLoader)
 
-  private def classesPrivateWithin(packageName: String): Set[String] = {
+
+  private def isDeveloperApi(sym: unv.Symbol) =
+    sym.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.DeveloperApi])
+
+  private def isExperimental(sym: unv.Symbol) =
+    sym.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.Experimental])
+
+
+  private def isPackagePrivate(sym: unv.Symbol) =
+    !sym.privateWithin.fullName.startsWith("<none>")
+
+  private def isPackagePrivateModule(moduleSymbol: unv.ModuleSymbol) =
+    !moduleSymbol.privateWithin.fullName.startsWith("<none>")
+
+  /**
+   * For every class checks via scala reflection if the class itself or contained members
+   * have DeveloperApi or Experimental annotations or they are package private.
+   * Returns the tuple of such classes and members.
+   */
+  private def privateWithin(packageName: String): (Set[String], Set[String]) = {
 
     val classes = getClasses(packageName)
     val ignoredClasses = mutable.HashSet[String]()
+    val ignoredMembers = mutable.HashSet[String]()
 
-    def isPackagePrivate(className: String) = {
+    for (className <- classes) {
       try {
-        /* Couldn't figure out if it's possible to determine a-priori whether a given symbol
-           is a module or class. */
-
-        val privateAsClass = mirror
-          .classSymbol(Class.forName(className, false, classLoader))
-          .privateWithin
-          .fullName
-          .startsWith(packageName)
-
-        val privateAsModule = mirror
-          .staticModule(className)
-          .privateWithin
-          .fullName
-          .startsWith(packageName)
-
-        privateAsClass || privateAsModule
-      } catch {
-        case _: Throwable => {
-          println("Error determining visibility: " + className)
-          false
+        val classSymbol = mirror.classSymbol(Class.forName(className, false, classLoader))
+        val moduleSymbol = mirror.staticModule(className) // TODO: see if it is necessary.
+        val directlyPrivateSpark =
+          isPackagePrivate(classSymbol) || isPackagePrivateModule(moduleSymbol)
+        val developerApi = isDeveloperApi(classSymbol)
+        val experimental = isExperimental(classSymbol)
+
+        /* Inner classes defined within a private[spark] class or object are effectively
+         invisible, so we account for them as package private. */
+        lazy val indirectlyPrivateSpark = {
+          val maybeOuter = className.toString.takeWhile(_ != '$')
+          if (maybeOuter != className) {
+            isPackagePrivate(mirror.classSymbol(Class.forName(maybeOuter, false, classLoader))) ||
+              isPackagePrivateModule(mirror.staticModule(maybeOuter))
+          } else {
+            false
+          }
+        }
+        if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi || experimental) {
+          ignoredClasses += className
+        } else {
+          // check if this class has package-private/annotated members.
+          ignoredMembers ++= getAnnotatedOrPackagePrivateMembers(classSymbol)
         }
-      }
-    }
 
-    def isDeveloperApi(className: String) = {
-      try {
-        val clazz = mirror.classSymbol(Class.forName(className, false, classLoader))
-        clazz.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.DeveloperApi])
       } catch {
-        case _: Throwable => {
-          println("Error determining Annotations: " + className)
-          false
-        }
+        case _: Throwable => println("Error instrumenting class:" + className)
       }
     }
+    (ignoredClasses.flatMap(c => Seq(c, c.replace("$", "#"))).toSet, ignoredMembers.toSet)
+  }
 
-    for (className <- classes) {
-      val directlyPrivateSpark = isPackagePrivate(className)
-      val developerApi = isDeveloperApi(className)
-
-      /* Inner classes defined within a private[spark] class or object are effectively
-         invisible, so we account for them as package private. */
-      val indirectlyPrivateSpark = {
-        val maybeOuter = className.toString.takeWhile(_ != '$')
-        if (maybeOuter != className) {
-          isPackagePrivate(maybeOuter)
-        } else {
-          false
-        }
-      }
-      if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi) {
-        ignoredClasses += className
-      }
-    }
-    ignoredClasses.flatMap(c => Seq(c, c.replace("$", "#"))).toSet
+  private def getAnnotatedOrPackagePrivateMembers(classSymbol: unv.ClassSymbol) = {
+    classSymbol.typeSignature.members
+      .filter(x => isPackagePrivate(x) || isDeveloperApi(x) || isExperimental(x)).map(_.fullName)
   }
 
   def main(args: Array[String]) {
-    scala.tools.nsc.io.File(".generated-mima-excludes").
-      writeAll(classesPrivateWithin("org.apache.spark").mkString("\n"))
-    println("Created : .generated-mima-excludes in current directory.")
+    val (privateClasses, privateMembers) = privateWithin("org.apache.spark")
+    scala.tools.nsc.io.File(".generated-mima-class-excludes").
+      writeAll(privateClasses.mkString("\n"))
+    println("Created : .generated-mima-class-excludes in current directory.")
+    scala.tools.nsc.io.File(".generated-mima-member-excludes").
+      writeAll(privateMembers.mkString("\n"))
+    println("Created : .generated-mima-member-excludes in current directory.")
   }
 
 
@@ -140,10 +144,17 @@ object GenerateMIMAIgnore {
    * Get all classes in a package from a jar file.
    */
   private def getClassesFromJar(jarPath: String, packageName: String) = {
+    import scala.collection.mutable
     val jar = new JarFile(new File(jarPath))
     val enums = jar.entries().map(_.getName).filter(_.startsWith(packageName))
-    val classes = for (entry <- enums if entry.endsWith(".class"))
-      yield Class.forName(entry.replace('/', '.').stripSuffix(".class"), false, classLoader)
+    val classes = mutable.HashSet[Class[_]]()
+    for (entry <- enums if entry.endsWith(".class")) {
+      try {
+        classes += Class.forName(entry.replace('/', '.').stripSuffix(".class"), false, classLoader)
+      } catch {
+        case _: Throwable => println("Unable to load:" + entry)
+      }
+    }
     classes
   }
 }