Merge pull request #49 from dotta/issue/use-decoded-names-when-checki…

…ng-accessibility-of-class-46

Use decoded names when checking if a classfile is reachable from client code

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

@@ -17,7 +17,7 @@ import com.typesafe.tools.mima.core.util.log.{ConsoleLogging, Logging}
 /** A placeholder class info for a class that is not found on the classpath or in a given
  *  package.
  */
-class SyntheticClassInfo(owner: PackageInfo, val name: String) extends ClassInfo(owner) {
+class SyntheticClassInfo(owner: PackageInfo, override val bytecodeName: String) extends ClassInfo(owner) {
   loaded = true
   def file: AbstractFile = throw new UnsupportedOperationException
   override lazy val superClasses = Nil
@@ -30,10 +30,10 @@ object NoClass extends SyntheticClassInfo(null, "<noclass>")
 
 /** A class for which we have the classfile. */
 class ConcreteClassInfo(owner: PackageInfo, val file: AbstractFile) extends ClassInfo(owner) {
-  def name = PackageInfo.className(file.name)
+  override def bytecodeName = PackageInfo.className(file.name)
 }
 
-abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
+abstract class ClassInfo(val owner: PackageInfo) extends HasDeclarationName with WithAccessFlags {
   import ClassInfo._
 
   def file: AbstractFile
@@ -45,12 +45,10 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
     case _                    => "compiler generated"
   }
 
-  def name: String
-
   lazy val fullName: String = {
-    assert(name != null)
-    if (owner.isRoot) name
-    else owner.fullName + "." + name
+    assert(bytecodeName != null)
+    if (owner.isRoot) bytecodeName
+    else owner.fullName + "." + bytecodeName
   }
 
   def formattedFullName = formatClassName(if (isObject) fullName.init else fullName)
@@ -115,7 +113,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
     lookupClassMethods(name) ++ lookupInterfaceMethods(name)
 
   def lookupConcreteTraitMethods(name: String): Iterator[MemberInfo] =
-    allTraits.toList.flatten(_.concreteMethods).filter(_.name == name).toIterator
+    allTraits.toList.flatten(_.concreteMethods).filter(_.bytecodeName == bytecodeName).toIterator
 
   /** Is this class a non-trait that inherits !from a trait */
   lazy val isClassInheritsTrait = !isInterface && _interfaces.exists(_.isTrait)
@@ -174,7 +172,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
     if (this == ClassInfo.ObjectClass) Set.empty
     else superClass.allTraits ++ directTraits
 
-  /** All traits inherited directly or indirectly by this class */
+  /** All interfaces inherited directly or indirectly by this class */
   lazy val allInterfaces: Set[ClassInfo] =
     if (this == ClassInfo.ObjectClass) Set.empty
     else superClass.allInterfaces ++ interfaces ++ (interfaces flatMap (_.allInterfaces))
@@ -198,7 +196,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
 
   /** Does this class have an implementation (forwarder or accessor) of given method `m'? */
   private def hasInstanceImpl(m: MemberInfo) =
-    methods.get(m.name) exists (_.sig == m.sig)
+    methods.get(m.bytecodeName) exists (_.sig == m.sig)
 
   /** Does this implementation class have a static implementation of given method `m'? */
   def hasStaticImpl(m: MemberInfo) = staticImpl(m).isDefined
@@ -209,7 +207,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
       implClass match {
         case impl: ConcreteClassInfo =>
           assert(impl.isImplClass, impl)
-          impl.methods.get(m.name) find (im => hasImplSig(im.sig, m.sig))
+          impl.methods.get(m.bytecodeName) find (im => hasImplSig(im.sig, m.sig))
 
         case _ => None
       }
@@ -236,7 +234,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
   }
 
   /** Is this class an implementation class? */
-  lazy val isImplClass: Boolean = name endsWith PackageInfo.implClassSuffix
+  lazy val isImplClass: Boolean = bytecodeName endsWith PackageInfo.implClassSuffix
 
   /** The implementation class corresponding to this trait */
   private var _implClass: ClassInfo = NoClass
@@ -262,7 +260,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
     ClassfileParser.isInterface(flags)
   }
 
-  def isObject: Boolean = name.endsWith("$")
+  def isObject: Boolean = bytecodeName.endsWith("$")
 
   /** Is this class public? */
   /*
@@ -271,7 +269,7 @@ abstract class ClassInfo(val owner: PackageInfo) extends WithAccessFlags {
     !ClassfileParser.isPrivate(flags)
   }*/
 
-  override def toString = "class " + name
+  override def toString = "class " + bytecodeName
 
   def shortDescription = {
     // using 'description' because elsewhere objects' name are not correctly translated.

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

@@ -287,7 +287,7 @@ abstract class ClassfileParser(definitions: Definitions) {
 
   /** Return true iff TraitSetter annotation found among attributes */
   def parseAttributes(m: MemberInfo) {
-    val maybeTraitSetter = MemberInfo.maybeSetter(m.name)
+    val maybeTraitSetter = MemberInfo.maybeSetter(m.bytecodeName)
     val attrCount = in.nextChar
     for (i <- 0 until attrCount) {
       val attrIndex = in.nextChar

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

@@ -0,0 +1,11 @@
+package com.typesafe.tools.mima.core
+
+import scala.reflect.NameTransformer
+
+trait HasDeclarationName {
+  /** The name as found in the bytecode. */
+  def bytecodeName: String
+
+  /** The name as found in the original Scala source. */
+  final def decodedName: String = NameTransformer.decode(bytecodeName)
+}

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

@@ -13,10 +13,8 @@ object MemberInfo {
   def maybeSetter(name: String) = name.endsWith(setterSuffix)
 }
 
-class MemberInfo(val owner: ClassInfo, val name: String, override val flags: Int, val sig: String) extends WithAccessFlags {
-  override def toString = "def "+name+": "+ sig
-
-  def decodedName = NameTransformer.decode(name)
+class MemberInfo(val owner: ClassInfo, val bytecodeName: String, override val flags: Int, val sig: String) extends HasDeclarationName with WithAccessFlags {
+  override def toString = "def " + bytecodeName + ": "+ sig
 
   def fieldString = "field "+decodedName+" in "+owner.classString
   def shortMethodString = (if(hasSyntheticName) "synthetic " else "") + (if(isDeprecated) "deprecated " else "") + "method "+decodedName + tpe
@@ -54,13 +52,13 @@ class MemberInfo(val owner: ClassInfo, val name: String, override val flags: Int
 
   var codeOpt: Option[(Int, Int)] = None
 
-  def isClassConstructor = name == "<init>"
+  def isClassConstructor = bytecodeName == "<init>"
 
   def needCode = isClassConstructor
 
   import MemberInfo._
 
-  var isTraitSetter = maybeSetter(name) && setterIdx(name) >= 0
+  var isTraitSetter = maybeSetter(bytecodeName) && setterIdx(bytecodeName) >= 0
 
   var isDeprecated = false
 
@@ -70,9 +68,9 @@ class MemberInfo(val owner: ClassInfo, val name: String, override val flags: Int
 
   /** The name of the getter corresponding to this setter */
   private def getterName: String = {
-    val sidx = setterIdx(name)
+    val sidx = setterIdx(bytecodeName)
     val start = if (sidx >= 0) sidx + setterTag.length else 0
-    name.substring(start, name.length - setterSuffix.length)
+    bytecodeName.substring(start, bytecodeName.length - setterSuffix.length)
   }
 
   /** The getter that corresponds to this setter */
@@ -81,6 +79,6 @@ class MemberInfo(val owner: ClassInfo, val name: String, override val flags: Int
     owner.methods.get(getterName) find (_.sig == argsig) get
   }
 
-  def description: String = name+": "+sig+" from "+owner.description
+  def description: String = bytecodeName + ": " + sig + " from " + owner.description
 }
 

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

@@ -8,7 +8,7 @@ class Members(val members: TraversableOnce[MemberInfo]) {
   private val bindings = new mutable.HashMap[String, List[MemberInfo]] {
     override def default(key: String) = List()
   }
-  for (m <- members) bindings += m.name -> (m :: bindings(m.name))
+  for (m <- members) bindings += m.bytecodeName -> (m :: bindings(m.bytecodeName))
 
   def iterator: Iterator[MemberInfo] =
     for (ms <- bindings.valuesIterator; m <- ms.iterator) yield m

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

@@ -72,10 +72,10 @@ abstract class PackageInfo(val owner: PackageInfo) {
     }
 
     def isAccessible(clazz: ClassInfo, prefix: Set[ClassInfo]) = {
-      val idx = clazz.name.lastIndexOf("$")
+      val idx = clazz.decodedName.lastIndexOf("$")
       lazy val isReachable =
       	if (idx < 0) prefix.isEmpty // class name contains no $
-      	else (prefix exists (_.name == clazz.name.substring(0, idx))) // prefix before dollar is an accessible class detected previously
+      	else (prefix exists (_.decodedName == clazz.decodedName.substring(0, idx))) // prefix before dollar is an accessible class detected previously
       clazz.isPublic && isReachable
     }
 

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

@@ -117,7 +117,7 @@ case class MissingTypesProblem(newclazz: ClassInfo, missing: Iterable[ClassInfo]
 
 case class CyclicTypeReferenceProblem(clz: ClassInfo) extends TemplateProblem(clz) {
   def description = {
-    "the type hierarchy of " + clz.description + " has changed in new version. Type " + clz.name + " appears to be a subtype of itself"
+    "the type hierarchy of " + clz.description + " has changed in new version. Type " + clz.bytecodeName + " appears to be a subtype of itself"
   }
 }
 

reporter/functional-tests/src/test/removing-method-from-class-with-special-characters-nok/problems.txt

@@ -0,0 +1 @@
+method ++()Int in class :: does not have a correspondent in new version

reporter/functional-tests/src/test/removing-method-from-class-with-special-characters-nok/v1/List.scala

@@ -0,0 +1,4 @@
+// old version
+class :: {
+  def ++ = 42
+}

reporter/functional-tests/src/test/removing-method-from-class-with-special-characters-nok/v2/List.scala

@@ -0,0 +1,2 @@
+// new version
+class ::

reporter/functional-tests/src/test/removing-method-with-special-characters-from-class-nok/problems.txt

@@ -0,0 +1 @@
+method ++()Int in class SpecialChars does not have a correspondent in new version

reporter/functional-tests/src/test/removing-method-with-special-characters-from-class-nok/v1/SpecialChars.scala

@@ -0,0 +1,4 @@
+// old version
+class SpecialChars {
+  def ++ = 42
+}

reporter/functional-tests/src/test/removing-method-with-special-characters-from-class-nok/v2/SpecialChars.scala

@@ -0,0 +1,2 @@
+// new version
+class SpecialChars

reporter/src/main/scala/com/typesafe/tools/mima/lib/MiMaLib.scala

@@ -45,8 +45,8 @@ class MiMaLib(classpath: JavaClassPath, val log: Logging = ConsoleLogging) {
   private def comparePackages(oldpkg: PackageInfo, newpkg: PackageInfo) {
     val traits = newpkg.traits // determine traits of new package first
     for (oldclazz <- oldpkg.accessibleClasses) {
-      log.info("Analyzing class "+oldclazz.name)
-      newpkg.classes get oldclazz.name match {
+      log.info("Analyzing class "+oldclazz.bytecodeName)
+      newpkg.classes get oldclazz.bytecodeName match {
         case None if oldclazz.isImplClass =>
           // if it is missing a trait implementation class, then no error should be reported
           // since there should be already errors, i.e., missing methods...

reporter/src/main/scala/com/typesafe/tools/mima/lib/analyze/Analyzer.scala

@@ -29,7 +29,7 @@ private[analyze] trait Analyzer extends Function2[ClassInfo, ClassInfo, List[Pro
     analyze(oldclazz, newclazz)
 
   def analyze(oldclazz: ClassInfo, newclazz: ClassInfo): List[Problem] = {
-    assert(oldclazz.name == newclazz.name)
+    assert(oldclazz.bytecodeName == newclazz.bytecodeName)
     val templateProblems = analyzeTemplateDecl(oldclazz, newclazz)
 
     if (templateProblems.exists(p => p.isInstanceOf[IncompatibleTemplateDefProblem] ||
@@ -77,7 +77,7 @@ private[analyze] class ClassAnalyzer extends Analyzer {
   /** Analyze incompatibilities that may derive from methods in the `newclazz` */
   override def analyzeNewClassMethods(oldclazz: ClassInfo, newclazz: ClassInfo): List[Problem] = {
     for (newAbstrMeth <- newclazz.deferredMethods) yield {
-      oldclazz.lookupMethods(newAbstrMeth.name).find(_.sig == newAbstrMeth.sig) match {
+      oldclazz.lookupMethods(newAbstrMeth.bytecodeName).find(_.sig == newAbstrMeth.sig) match {
         case None =>
           val p = MissingMethodProblem(newAbstrMeth)
           p.affectedVersion = Problem.ClassVersion.Old
@@ -106,7 +106,7 @@ private[analyze] class TraitAnalyzer extends Analyzer {
     val res = collection.mutable.ListBuffer.empty[Problem]
 
     for (newmeth <- newclazz.concreteMethods if !oldclazz.hasStaticImpl(newmeth)) {
-      if (!oldclazz.lookupMethods(newmeth.name).exists(_.sig == newmeth.sig)) {
+      if (!oldclazz.lookupMethods(newmeth.bytecodeName).exists(_.sig == newmeth.sig)) {
         // this means that the method is brand new and therefore the implementation
         // has to be injected
         val problem = MissingMethodProblem(newmeth)
@@ -120,7 +120,7 @@ private[analyze] class TraitAnalyzer extends Analyzer {
     }
 
     for (newmeth <- newclazz.deferredMethods) {
-      val oldmeths = oldclazz.lookupMethods(newmeth.name)
+      val oldmeths = oldclazz.lookupMethods(newmeth.bytecodeName)
       oldmeths find (_.sig == newmeth.sig) match {
         case Some(oldmeth) => ()
         case _ =>

reporter/src/main/scala/com/typesafe/tools/mima/lib/analyze/field/FieldChecker.scala

@@ -8,7 +8,7 @@ private[analyze] abstract class BaseFieldChecker extends Checker[MemberInfo, Cla
 
   def check(field: MemberInfo, in: ClassInfo): Option[Problem] = {
     if (field.isAccessible) {
-      val newflds = in.lookupClassFields(field.name)
+      val newflds = in.lookupClassFields(field.bytecodeName)
       if (newflds.hasNext) {
         val newfld = newflds.next
         if (!newfld.isPublic)

reporter/src/main/scala/com/typesafe/tools/mima/lib/analyze/method/MethodChecker.scala

@@ -38,9 +38,9 @@ private[analyze] abstract class BaseMethodChecker extends Checker[MemberInfo, Cl
 private[analyze] class ClassMethodChecker extends BaseMethodChecker {
   def check(method: MemberInfo, inclazz: ClassInfo): Option[Problem] = {
     if (method.isDeferred)
-      super.check(method, inclazz.lookupMethods(method.name))
+      super.check(method, inclazz.lookupMethods(method.bytecodeName))
     else
-      super.check(method, inclazz.lookupClassMethods(method.name))
+      super.check(method, inclazz.lookupClassMethods(method.bytecodeName))
   }
 }
 
@@ -49,7 +49,7 @@ private[analyze] class TraitMethodChecker extends BaseMethodChecker {
     if (method.owner.hasStaticImpl(method)) {
       checkStaticImplMethod(method, inclazz)
     } else {
-      super.check(method, inclazz.lookupMethods(method.name))
+      super.check(method, inclazz.lookupMethods(method.bytecodeName))
     }
   }
 
@@ -68,7 +68,7 @@ private[analyze] class TraitMethodChecker extends BaseMethodChecker {
         // otherwise we check the all concrete trait methods and report
         // either that the method is missing or that no method with the
         // same signature exists. Either way, we expect that a problem is reported!
-        val prob = super.check(method, inclazz.lookupConcreteTraitMethods(method.name))
+        val prob = super.check(method, inclazz.lookupConcreteTraitMethods(method.bytecodeName))
         assert(prob.isDefined)
         prob
       }