Fix #97: Avoid spurious recompilations when unrelated constructor cha…

…nges

The name hashing algorithm is designed to take implicit conversions into
account: when a name "foo" is changed somewhere in a dependency of X,
you have to recompile X if it uses the name "foo", even if the usage of
"foo" in X is completely unrelated, just because this might have an
effect on available implicit conversions. However, there is one case
where we can be sure that implicit conversions will not kick in: when we
call a constructor. A constructor name is always "<init>", this PR now
replaces this name by "pkgA;pkgB;className;init;", this mean that we no
longer recompile classes when an unrelated constructor in a used class
changed (see the new test `constructors-unrelated` for an example).

internal/compiler-bridge/src/main/scala/xsbt/ClassName.scala

@@ -32,6 +32,37 @@ trait ClassName extends Compat {
    */
   protected def classNameAsString(s: Symbol): String = pickledNameAsString(s)
 
+  /**
+   * Given a class symbol `cls`, construct a name representing this constructor.
+   * For a class:
+   *
+   *     a.b.Foo
+   *
+   * this is:
+   *
+   *     a;b;Foo;init;
+   *
+   * The prefix is important to avoid name hashing all constructors together
+   * (see #97), the weird format is necessary to avoid scalac or zinc trying to
+   * interpret this name (in particular we should not use '.' and we should not
+   * use '<init>'), we use ';' because it is one of the few characters that
+   * cannot appear in a valid JVM name.
+   */
+  protected def constructorName(cls: Symbol): Name =
+    newTermName(constructorNameAsString(cls))
+
+  protected def constructorNameAsString(cls: Symbol): String =
+    cls.fullName(';') ++ ";init;"
+
+  /**
+   * Mangle a JVM symbol name in a format better suited for internal uses by sbt.
+   */
+  protected def mangledName(s: Symbol): Name =
+    if (s.name == nme.CONSTRUCTOR)
+      constructorName(s.enclClass)
+    else
+      s.name
+
   /**
    * Create a (source) name for the class symbol `s` with a prefix determined by the class symbol `in`.
    *

internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala

@@ -613,8 +613,12 @@ class ExtractAPI[GlobalType <: Global](
   private def simpleName(s: Symbol): String =
     {
       val n = s.unexpandedName
-      val n2 = if (n.toString == "<init>") n else n.decode
-      n2.toString.trim
+      val n2 =
+        if (n == nme.CONSTRUCTOR)
+          constructorNameAsString(s.enclClass)
+        else
+          n.decode.toString
+      n2.trim
     }
 
   private def staticAnnotations(annotations: List[AnnotationInfo]): List[AnnotationInfo] = {

internal/compiler-bridge/src/main/scala/xsbt/ExtractUsedNames.scala

@@ -170,11 +170,9 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
 
     val addSymbol: (JavaSet[Name], Symbol) => Unit = {
       (names: JavaSet[Name], symbol: Symbol) =>
-        if (!ignoredSymbol(symbol)) {
-          val name = symbol.name
-          // Synthetic names are no longer included. See https://github.com/sbt/sbt/issues/2537
-          if (!isEmptyName(name))
-            names.add(name)
+        // Synthetic names are no longer included. See https://github.com/sbt/sbt/issues/2537
+        if (!ignoredSymbol(symbol) && !isEmptyName(symbol.name)) {
+          names.add(mangledName(symbol))
           ()
         }
     }
@@ -209,7 +207,7 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
     private object PatMatDependencyTraverser extends TypeDependencyTraverser {
       override def addDependency(symbol: global.Symbol): Unit = {
         if (!ignoredSymbol(symbol) && symbol.isSealed) {
-          val name = symbol.name
+          val name = mangledName(symbol)
           if (!isEmptyName(name)) {
             val existingScopes = _currentScopedNamesCache.get(name)
             if (existingScopes == null)

internal/compiler-bridge/src/test/scala/xsbt/ExtractUsedNamesPerformanceSpecification.scala

@@ -41,12 +41,12 @@ class ExtractUsedNamesPerformanceSpecification extends UnitSpec {
       val compilerForTesting = new ScalaCompilerForUnitTesting
       compilerForTesting.extractUsedNamesFromSrc(src)
     }
-    val expectedNamesForTupler = Set("<init>", "Object", "scala", "tupler", "TuplerInstances", "DepFn1", "HNil", "$anon", "Out", "Out0", "Tupler", "hnilTupler", "acme", "L", "Aux", "HList", "Serializable", "Unit")
-    val expectedNamesForTuplerInstances = Set("E", "Tuple4", "e", "case7", "Tuple15", "s", "case19", "T7", "x", "TuplerInstances", "matchEnd19", "T20", "Tuple11", "HNil", "matchEnd6", "p16", "$anon", "T19", "p20", "T2", "p10", "case22", "p19", "n", "Tuple12", "case11", "Tuple22", "p12", "matchEnd7", "N", "p4", "T13", "case26", "Tuple19", "p7", "p5", "j", "Out", "T", "p23", "case15", "matchEnd20", "t", "p21", "matchEnd15", "J", "head", "case13", "u", "matchEnd18", "U", "Tupler", "f", "T8", "T16", "F", "Tuple3", "case8", "case18", "case24", "Boolean", "matchEnd21", "A", "matchEnd26", "a", "Tuple14", "T1", "::", "Nothing", "p18", "case20", "m", "matchEnd10", "M", "matchEnd25", "tail", "Tuple2", "matchEnd5", "p15", "matchEnd23", "I", "i", "matchEnd14", "AnyRef", "Tuple8", "matchEnd8", "case25", "T12", "p3", "case14", "case23", "T5", "matchEnd22", "T17", "v", "p22", "Tuple18", "G", "Tuple13", "matchEnd12", "<init>", "V", "q", "p11", "Q", "case12", "L", "b", "apply", "Object", "g", "B", "l", "==", "Out0", "Tuple1", "matchEnd9", "P", "p2", "T15", "Aux", "matchEnd24", "p", "scala", "matchEnd11", "Tuple20", "HList", "case17", "T9", "p14", "Tuple7", "matchEnd17", "T4", "case28", "T22", "p17", "C", "Tuple6", "MatchError", "T11", "x1", "H", "case16", "matchEnd13", "c", "Tuple9", "h", "T6", "T18", "r", "K", "Tuple17", "p9", "R", "ne", "T14", "case21", "k", "case10", "Tuple21", "O", "case9", "Tuple10", "Any", "T10", "case27", "Tuple5", "D", "p13", "o", "p6", "p8", "matchEnd16", "S", "T21", "Tuple16", "d", "T3")
+    val expectedNamesForTupler = Set("java;lang;Object;init;", "Object", "acme;Tupler;$anon;init;", "scala", "tupler", "TuplerInstances", "DepFn1", "HNil", "$anon", "Out", "Out0", "Tupler", "hnilTupler", "acme", "L", "Aux", "HList", "Serializable", "Unit")
+    val expectedNamesForTuplerInstances = Set("E", "Tuple4", "e", "case7", "Tuple15", "s", "case19", "T7", "x", "acme;TuplerInstances;$anon;init;", "TuplerInstances", "matchEnd19", "T20", "Tuple11", "HNil", "matchEnd6", "p16", "$anon", "T19", "p20", "T2", "p10", "case22", "p19", "n", "Tuple12", "case11", "Tuple22", "p12", "matchEnd7", "N", "p4", "T13", "case26", "Tuple19", "p7", "p5", "j", "Out", "T", "p23", "case15", "matchEnd20", "t", "p21", "matchEnd15", "J", "head", "case13", "u", "matchEnd18", "U", "Tupler", "f", "T8", "T16", "F", "Tuple3", "case8", "case18", "case24", "Boolean", "matchEnd21", "A", "matchEnd26", "a", "Tuple14", "T1", "::", "Nothing", "p18", "case20", "m", "matchEnd10", "M", "matchEnd25", "tail", "Tuple2", "matchEnd5", "p15", "matchEnd23", "I", "i", "matchEnd14", "AnyRef", "Tuple8", "matchEnd8", "case25", "T12", "p3", "case14", "case23", "T5", "matchEnd22", "T17", "v", "p22", "Tuple18", "G", "Tuple13", "matchEnd12", "java;lang;Object;init;", "V", "q", "p11", "Q", "case12", "L", "b", "apply", "Object", "g", "B", "l", "==", "Out0", "Tuple1", "matchEnd9", "P", "p2", "T15", "Aux", "matchEnd24", "p", "scala", "matchEnd11", "Tuple20", "HList", "case17", "T9", "p14", "Tuple7", "matchEnd17", "T4", "case28", "T22", "p17", "C", "Tuple6", "MatchError", "scala;MatchError;init;", "T11", "x1", "H", "case16", "matchEnd13", "c", "Tuple9", "h", "T6", "T18", "r", "K", "Tuple17", "p9", "R", "ne", "T14", "case21", "k", "case10", "Tuple21", "O", "case9", "Tuple10", "Any", "T10", "case27", "Tuple5", "D", "p13", "o", "p6", "p8", "matchEnd16", "S", "T21", "Tuple16", "d", "T3")
     val expectedNamesForRefinement = Set("Out0")
-    val `expectedNamesFor::` = Set("x", "T2", "ScalaRunTime", "Iterator", "T", "head", "asInstanceOf", "Boolean", "A", "$" + "isInstanceOf", "T1", "||", "::", "Nothing", "x$1", "any2stringadd", "acme", "typedProductIterator", "tail", "Tuple2", "AnyRef", "isInstanceOf", "Int", "<init>", "_hashCode", "apply", "Object", "x$0", "==", "Some", "IndexOutOfBoundsException", "T0", "Predef", "scala", "matchEnd4", "HList", "None", "x1", "toString", "H", "+", "&&", "Serializable", "Product", "case6", "::$1", "eq", "Any", "runtime", "String")
+    val `expectedNamesFor::` = Set("x", "T2", "ScalaRunTime", "Iterator", "T", "head", "asInstanceOf", "Boolean", "A", "$" + "isInstanceOf", "T1", "||", "acme;::;init;", "::", "Nothing", "x$1", "any2stringadd", "acme", "typedProductIterator", "tail", "Tuple2", "AnyRef", "isInstanceOf", "Int", "java;lang;Object;init;", "_hashCode", "apply", "Object", "x$0", "==", "Some", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "T0", "Predef", "scala", "matchEnd4", "HList", "None", "x1", "toString", "H", "+", "&&", "Serializable", "Product", "case6", "::$1", "eq", "Any", "runtime", "String")
     val expectedNamesForDepFn1 = Set("DepFn1", "Out", "T", "AnyRef", "Object", "scala")
-    val expectedNamesForHNil = Set("x", "HNil", "ScalaRunTime", "Iterator", "Boolean", "A", "T", "$" + "isInstanceOf", "::", "Nothing", "x$1", "acme", "typedProductIterator", "Int", "<init>", "apply", "Object", "IndexOutOfBoundsException", "scala", "HList", "toString", "H", "Serializable", "h", "Product", "Any", "runtime", "matchEnd3", "String", "T0")
+    val expectedNamesForHNil = Set("x", "HNil", "ScalaRunTime", "Iterator", "Boolean", "A", "T", "$" + "isInstanceOf", "::", "Nothing", "x$1", "acme", "typedProductIterator", "Int", "java;lang;Object;init;", "apply", "Object", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "scala", "HList", "toString", "H", "Serializable", "h", "Product", "Any", "runtime", "matchEnd3", "String", "T0")
     val expectedNamesForHList = Set("Tupler", "acme", "scala", "Serializable", "Product")
     assert(usedNames("acme.Tupler") -- scalaDiff === expectedNamesForTupler -- scalaDiff)
     assert(usedNames("acme.TuplerInstances") -- scalaDiff === expectedNamesForTuplerInstances -- scalaDiff)
@@ -96,9 +96,9 @@ class ExtractUsedNamesPerformanceSpecification extends UnitSpec {
     val (_, analysis) = compilerForTesting.compileSrcs(List(List(ext), List(cod)), reuseCompilerInstance = true)
     val usedNames = analysis.usedNames.toMap
 
-    val expectedNamesForFoo = Set("TypeApplyExtractor", "mkIdent", "package", "<repeated>", "tpe", "in", "$u", "internal", "reify", "WeakTypeTag", "Name", "empty", "collection", "ThisType", "staticModule", "staticPackage", "Singleton", "T", "asInstanceOf", "ReificationSupportApi", "U", "Expr", "Universe", "TypeApply", "A", "Tree", "Nothing", "acme", "ClassSymbol", "blackbox", "AnyRef", "Context", "mkTypeTree", "immutable", "SelectExtractor", "<init>", "$treecreator1", "apply", "Object", "macros", "moduleClass", "Foo", "T0", "Symbol", "Predef", "scala", "asModule", "Internal", "$m", "TypeCreator", "TermNameExtractor", "ModuleSymbol", "staticClass", "universe", "c", "<refinement>", "TypeTree", "List", "Select", "TermName", "Mirror", "atag", "reificationSupport", "rootMirror", "reflect", "TypeRef", "Ident", "Any", "TreeCreator", "$typecreator2", "$m$untyped", "String", "Type")
-    val expectedNamesForBar = Set("experimental", "package", "WeakTypeTag", "Out", "foo_impl", "Expr", "A", "Nothing", "acme", "AnyRef", "Context", "<init>", "language", "Object", "macros", "Bar", "Foo", "scala", "List", "Any")
+    val expectedNamesForFoo = Set("TypeApplyExtractor", "mkIdent", "package", "<repeated>", "tpe", "in", "$u", "internal", "reify", "WeakTypeTag", "Name", "empty", "collection", "ThisType", "staticModule", "staticPackage", "Singleton", "T", "asInstanceOf", "ReificationSupportApi", "U", "Expr", "Universe", "TypeApply", "A", "Tree", "Nothing", "acme", "ClassSymbol", "blackbox", "AnyRef", "Context", "mkTypeTree", "immutable", "SelectExtractor", "java.lang.Object.init;", "$treecreator1", "apply", "Object", "macros", "moduleClass", "Foo", "T0", "Symbol", "Predef", "scala", "asModule", "Internal", "$m", "TypeCreator", "TermNameExtractor", "ModuleSymbol", "staticClass", "universe", "c", "<refinement>", "TypeTree", "List", "Select", "TermName", "Mirror", "atag", "reificationSupport", "rootMirror", "reflect", "TypeRef", "Ident", "Any", "TreeCreator", "$typecreator2", "$m$untyped", "String", "Type")
+    val expectedNamesForBar = Set("experimental", "package", "WeakTypeTag", "Out", "foo_impl", "Expr", "A", "Nothing", "acme", "AnyRef", "Context", "java;lang;Object;init;", "language", "Object", "macros", "Bar", "Foo", "scala", "List", "Any")
     assert(usedNames("acme.Foo") === expectedNamesForFoo)
     assert(usedNames("acme.Bar") === expectedNamesForBar)
   }
-}
+}

internal/compiler-bridge/src/test/scala/xsbt/ExtractUsedNamesSpecification.scala

@@ -78,8 +78,8 @@ class ExtractUsedNamesSpecification extends UnitSpec {
       if (scalaVersion.contains("2.10")) Set("Nothing", "Any") else Set()
     val namesA = standardNames ++ Set("A") ++ versionDependentNames
     val namesAX = standardNames ++ Set("X", "x", "T", "A")
-    val namesB = Set("B", "A", "Int", "<init>", "scala")
-    val namesC = Set("<init>", "C", "B")
+    val namesB = Set("B", "A", "Int", "A;init;", "scala")
+    val namesC = Set("B;init;", "C", "B")
     val namesD = standardNames ++ Set("D", "C", "X", "foo", "Int", "T")
     assert(usedNames("A") === namesA)
     assert(usedNames("A.X") === namesAX)
@@ -270,9 +270,7 @@ class ExtractUsedNamesSpecification extends UnitSpec {
   private val standardNames = Set(
     "scala",
     // The default parent of a class is "AnyRef" which is an alias for "Object"
-    "AnyRef", "Object",
-    // class receives a default constructor which is internally called "<init>"
-    "<init>"
+    "AnyRef", "Object", "java;lang;Object;init;"
   )
 
 }

internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala

@@ -102,7 +102,7 @@ object ClassToAPI {
     lazy val cf = classFileForClass(c)
     val methods = mergeMap(c, c.getDeclaredMethods, c.getMethods, methodToDef(enclPkg))
     val fields = mergeMap(c, c.getDeclaredFields, c.getFields, fieldToDef(c, cf, enclPkg))
-    val constructors = mergeMap(c, c.getDeclaredConstructors, c.getConstructors, constructorToDef(enclPkg))
+    val constructors = mergeMap(c, c.getDeclaredConstructors, c.getConstructors, constructorToDef(enclPkg, c))
     val classes = merge[Class[_]](c, c.getDeclaredClasses, c.getClasses, toDefinitions(cmap), (_: Seq[Class[_]]).partition(isStatic), _.getEnclosingClass != c)
     val all = methods ++ fields ++ constructors ++ classes
     val parentJavaTypes = allSuperTypes(c)
@@ -218,8 +218,11 @@ object ClassToAPI {
   def methodToDef(enclPkg: Option[String])(m: Method): api.Def =
     defLike(m.getName, m.getModifiers, m.getDeclaredAnnotations, typeParameterTypes(m), m.getParameterAnnotations, parameterTypes(m), Option(returnType(m)), exceptionTypes(m), m.isVarArgs, enclPkg)
 
-  def constructorToDef(enclPkg: Option[String])(c: Constructor[_]): api.Def =
-    defLike("<init>", c.getModifiers, c.getDeclaredAnnotations, typeParameterTypes(c), c.getParameterAnnotations, parameterTypes(c), None, exceptionTypes(c), c.isVarArgs, enclPkg)
+  def constructorToDef(enclPkg: Option[String], enclClass: Class[_])(c: Constructor[_]): api.Def =
+    defLike(
+      s"${name(c).replace('.', ';')};init;", // Use the format defined in xsbt.ClassName.constructorName documentation
+      c.getModifiers, c.getDeclaredAnnotations, typeParameterTypes(c), c.getParameterAnnotations, parameterTypes(c), None, exceptionTypes(c), c.isVarArgs, enclPkg
+    )
 
   def defLike[T <: GenericDeclaration](name: String, mods: Int, annots: Array[Annotation], tps: Array[TypeVariable[T]], paramAnnots: Array[Array[Annotation]], paramTypes: Array[Type], retType: Option[Type], exceptions: Array[Type], varArgs: Boolean, enclPkg: Option[String]): api.Def =
     {

zinc/src/sbt-test/source-dependencies/constructors-unrelated/A.scala

@@ -0,0 +1,2 @@
+class A(a: Int)
+object A { val x = 3 }

zinc/src/sbt-test/source-dependencies/constructors-unrelated/B.scala

@@ -0,0 +1 @@
+class B { val y = A.x }

zinc/src/sbt-test/source-dependencies/constructors-unrelated/changes/A2.scala

@@ -0,0 +1,2 @@
+class A(a: String)
+object A { val x = 3 }

zinc/src/sbt-test/source-dependencies/constructors-unrelated/test

@@ -0,0 +1,4 @@
+> compile # First compilation round
+$ copy-file changes/A2.scala A.scala
+> compile # Second compilation round, there should be no third round (we don't need to recompile B.scala)
+> checkIterations 2 # Check that there were only two rounds of compilation

zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala

@@ -1,5 +1,6 @@
 object C {
   def main(args: Array[String]): Unit = {
     val x = B.foo
+    println("x: " + x) // Need to use x in an expression to see if it crashes or not
   }
 }