Exploring inner modules for binary incompatibilities. Fix #127

The former implementation of PackageInfo#accessibleClasses expects that for any
module `Foo` compiled by scalac, both `Foo.class` and `Foo$.class` classfiles
are produced. Because `Foo$.class` and `Foo.class` are essentially mirrors, and
users never access directly `Foo$.class`, MiMa only traverses the `Foo.class`. This
works well for top-level modules, but it breaks for inner modules, because no
mirror class is created for inner modules (only the module class is created).

The fix consist in treating inner modules differently, hence making sure inner
modules are part of the set returned by PackageInfo#accessibleClasses.

Author: dotta

core/src/main/scala/com/typesafe/tools/mima/core/ClassInfo.scala

@@ -65,10 +65,10 @@ abstract class ClassInfo(val owner: PackageInfo) extends HasDeclarationName with
 
   override def canEqual(other: Any) = other.isInstanceOf[ClassInfo]
 
-  def formattedFullName = formatClassName(if (isObject) fullName.init else fullName)
+  def formattedFullName = formatClassName(if (isModule) fullName.init else fullName)
 
   def declarationPrefix = {
-    if (isObject) "object"
+    if (isModule) "object"
     else if (isTrait) "trait"
     else if (loaded && isInterface) "interface" // java interfaces and traits with no implementation methods
     else "class"
@@ -290,7 +290,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends HasDeclarationName with
     ClassfileParser.isInterface(flags)
   }
 
-  def isObject: Boolean = bytecodeName.endsWith("$")
+  def isModule: Boolean = bytecodeName.endsWith("$")
 
   /** Is this class public? */
   /*

core/src/main/scala/com/typesafe/tools/mima/core/PackageInfo.scala

@@ -64,8 +64,9 @@ abstract class PackageInfo(val owner: PackageInfo) {
   def classes: mutable.Map[String, ClassInfo]
 
   lazy val accessibleClasses: Set[ClassInfo] = {
+    // FIXME: Make this a tailrecursive implementation
     /** Fixed point iteration for finding all accessible classes. */
-    def accessibleClassesUnder(prefix: Set[ClassInfo]): Set[ClassInfo] = {
+    def accessibleClassesUnder(prefix: Set[ClassInfo]): Set[ClassInfo] = { 
       val vclasses = (classes.valuesIterator filter (isAccessible(_, prefix))).toSet
       if (vclasses.isEmpty) vclasses
       else vclasses union accessibleClassesUnder(vclasses)
@@ -77,7 +78,28 @@ abstract class PackageInfo(val owner: PackageInfo) {
         else {
           val idx = clazz.decodedName.lastIndexOf("$")
           if (idx < 0) prefix.isEmpty // class name contains no $
-          else prefix exists (_.decodedName == clazz.decodedName.substring(0, idx)) // prefix before dollar is an accessible class detected previously
+          else {
+            // A top-level module is compiled into a class plus a module class (for instance,
+            // for a top-level `object A`, scalac creates two classfiles `A.class` and `A$.class`.).
+            // While, for an inner module, scalac creates only one classfile, i.e., the module class 
+            // classfile.
+            //
+            // `clazz` is an inner module if:
+            // 1) `clazz` is a module, and 
+            // 2) `clazz` decoded name starts with one of the passed `prefix`.
+            def isInnerModule: Boolean = {
+              clazz.isModule && 
+              prefix.exists { outer =>
+                // this condition is to avoid looping indefinitely 
+                clazz.decodedName != outer.decodedName &&
+                // checks if `outer` is a prefix of `clazz` name 
+                clazz.decodedName.startsWith(outer.decodedName)
+              }
+            }
+            // class, trait, and top-level module go through this condition
+            (prefix exists (_.decodedName == clazz.decodedName.substring(0, idx))) || // prefix before dollar is an accessible class detected previously
+            isInnerModule
+          }
         }
       }
       clazz.isPublic && isReachable

reporter/functional-tests/src/test/class-removing-inner-object-nok/problems.txt

@@ -0,0 +1,2 @@
+object A#B does not have a correspondent in new version
+method B()A#B# in class A does not have a correspondent in new version

reporter/functional-tests/src/test/class-removing-inner-object-nok/v1/A.scala

@@ -0,0 +1,5 @@
+class A {
+  object B {
+    def foo: Int = 2
+  }
+}

reporter/functional-tests/src/test/class-removing-inner-object-nok/v2/A.scala

@@ -0,0 +1,2 @@
+class A {
+}

reporter/functional-tests/src/test/object-removing-inner-object-nok/problems.txt

@@ -0,0 +1 @@
+object A#B does not have a correspondent in new version

reporter/functional-tests/src/test/object-removing-inner-object-nok/v1/A.scala

@@ -0,0 +1,5 @@
+object A {
+  object B {
+    def foo: Int = 2
+  }
+}

reporter/functional-tests/src/test/object-removing-inner-object-nok/v2/A.scala

@@ -0,0 +1,2 @@
+object A {
+}