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 authored and szeiger committed Nov 30, 2016
1 parent e083f6d commit f3f8939
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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? */
/*
Expand Down
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 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
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 f3f8939

Please sign in to comment.