Skip to content

Commit

Permalink
Exploring inner modules for binary incompatibilities. Fix #127
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dotta committed Oct 1, 2016
1 parent 7c956c5 commit 186a52f
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 2 deletions.
26 changes: 24 additions & 2 deletions core/src/main/scala/com/typesafe/tools/mima/core/PackageInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 together with 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.
// Hence, in this branch we need to identify when `clazz` is an inner module. The implemented
// heuristic assumes that only module classes ends with a '$'. `clazz` is an inner module iff
// the `clazz` decoded name ends with a '$' and if the `clazz` decoded name starts with one
// of the passed `prefix`.
val isInnerObject = {
clazz.decodedName.endsWith("$") &&
prefix.exists { outer =>
// this condition is to avoid looping indefinitely
clazz.decodedName != outer.decodedName &&
// checks if `outer` is a prefix of `clazz`
clazz.decodedName.startsWith(outer.decodedName)
}
}
isInnerObject ||
// 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class A {
object B {
def foo: Int = 2
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class A {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
object A#B does not have a correspondent in new version
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {
object B {
def foo: Int = 2
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
object A {
}

0 comments on commit 186a52f

Please sign in to comment.