From 3fa7bddcde4422c509179eb032faeb49dae18383 Mon Sep 17 00:00:00 2001 From: Mirco Dotta Date: Sat, 1 Oct 2016 14:35:40 +0200 Subject: [PATCH] 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. --- .../typesafe/tools/mima/core/ClassInfo.scala | 6 ++--- .../tools/mima/core/PackageInfo.scala | 26 +++++++++++++++++-- .../problems.txt | 2 ++ .../v1/A.scala | 5 ++++ .../v2/A.scala | 2 ++ .../problems.txt | 1 + .../v1/A.scala | 5 ++++ .../v2/A.scala | 2 ++ 8 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 reporter/functional-tests/src/test/class-removing-inner-object-nok/problems.txt create mode 100644 reporter/functional-tests/src/test/class-removing-inner-object-nok/v1/A.scala create mode 100644 reporter/functional-tests/src/test/class-removing-inner-object-nok/v2/A.scala create mode 100644 reporter/functional-tests/src/test/object-removing-inner-object-nok/problems.txt create mode 100644 reporter/functional-tests/src/test/object-removing-inner-object-nok/v1/A.scala create mode 100644 reporter/functional-tests/src/test/object-removing-inner-object-nok/v2/A.scala diff --git a/core/src/main/scala/com/typesafe/tools/mima/core/ClassInfo.scala b/core/src/main/scala/com/typesafe/tools/mima/core/ClassInfo.scala index b48ca233..c280415d 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/core/ClassInfo.scala +++ b/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? */ /* diff --git a/core/src/main/scala/com/typesafe/tools/mima/core/PackageInfo.scala b/core/src/main/scala/com/typesafe/tools/mima/core/PackageInfo.scala index e55c444c..cc83c96b 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/core/PackageInfo.scala +++ b/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`. + val isInnerModule = { + 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) + } + } + isInnerModule || + // 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 + } } } clazz.isPublic && isReachable diff --git a/reporter/functional-tests/src/test/class-removing-inner-object-nok/problems.txt b/reporter/functional-tests/src/test/class-removing-inner-object-nok/problems.txt new file mode 100644 index 00000000..57dea9bc --- /dev/null +++ b/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 \ No newline at end of file diff --git a/reporter/functional-tests/src/test/class-removing-inner-object-nok/v1/A.scala b/reporter/functional-tests/src/test/class-removing-inner-object-nok/v1/A.scala new file mode 100644 index 00000000..6e207331 --- /dev/null +++ b/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 + } +} diff --git a/reporter/functional-tests/src/test/class-removing-inner-object-nok/v2/A.scala b/reporter/functional-tests/src/test/class-removing-inner-object-nok/v2/A.scala new file mode 100644 index 00000000..2e2439c3 --- /dev/null +++ b/reporter/functional-tests/src/test/class-removing-inner-object-nok/v2/A.scala @@ -0,0 +1,2 @@ +class A { +} diff --git a/reporter/functional-tests/src/test/object-removing-inner-object-nok/problems.txt b/reporter/functional-tests/src/test/object-removing-inner-object-nok/problems.txt new file mode 100644 index 00000000..e4037ccc --- /dev/null +++ b/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 \ No newline at end of file diff --git a/reporter/functional-tests/src/test/object-removing-inner-object-nok/v1/A.scala b/reporter/functional-tests/src/test/object-removing-inner-object-nok/v1/A.scala new file mode 100644 index 00000000..598070d7 --- /dev/null +++ b/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 + } +} diff --git a/reporter/functional-tests/src/test/object-removing-inner-object-nok/v2/A.scala b/reporter/functional-tests/src/test/object-removing-inner-object-nok/v2/A.scala new file mode 100644 index 00000000..169ef293 --- /dev/null +++ b/reporter/functional-tests/src/test/object-removing-inner-object-nok/v2/A.scala @@ -0,0 +1,2 @@ +object A { +}