Include private members in API hash of traits.

During compilation, scalac generates getters and setters for the fields
of traits, regardless of their access modifiers. Therefore, when a
field of a trait is modified, all its implementors must be recompiled to
take these changes into account.

Private fields of traits were not included in the API hash of traits,
and their implementors were thus not recompiled when modified.

This commit changes the way the API hash is computed for traits only, so
that the generated hash includes the private members of traits.

Fixes sbt/sbt#2155

compile/api/src/main/scala/xsbt/api/HashAPI.scala

@@ -60,6 +60,8 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include
     }
   }
 
+  private[this] def isTrait(cl: ClassLike) = cl.definitionType == DefinitionType.Trait
+
   private[this] final val ValHash = 1
   private[this] final val VarHash = 2
   private[this] final val DefHash = 3
@@ -184,7 +186,7 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include
     extend(ClassHash)
     hashParameterizedDefinition(c)
     hashType(c.selfType)
-    hashStructure(c.structure, includeDefinitions)
+    hashStructure(c.structure, includeDefinitions, isTrait(c))
   }
   def hashField(f: FieldLike): Unit = {
     f match {
@@ -343,14 +345,14 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include
     hashType(a.baseType)
     hashAnnotations(a.annotations)
   }
-  final def hashStructure(structure: Structure, includeDefinitions: Boolean) =
-    visit(visitedStructures, structure)(structure => hashStructure0(structure, includeDefinitions))
-  def hashStructure0(structure: Structure, includeDefinitions: Boolean): Unit = {
+  final def hashStructure(structure: Structure, includeDefinitions: Boolean, isTrait: Boolean = false) =
+    visit(visitedStructures, structure)(structure => hashStructure0(structure, includeDefinitions, isTrait))
+  def hashStructure0(structure: Structure, includeDefinitions: Boolean, isTrait: Boolean = false): Unit = {
     extend(StructureHash)
     hashTypes(structure.parents, includeDefinitions)
-    if (includeDefinitions) {
-      hashDefinitions(structure.declared, false)
-      hashDefinitions(structure.inherited, false)
+    if (includeDefinitions || isTrait) {
+      hashDefinitions(structure.declared, isTrait)
+      hashDefinitions(structure.inherited, isTrait)
     }
   }
   def hashParameters(parameters: Seq[TypeParameter], base: Type): Unit =

compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala

@@ -164,6 +164,115 @@ class NameHashingSpecification extends Specification {
     assertNameHashNotEqualForRegularName("bar", nameHashes1, nameHashes2)
   }
 
+  /**
+   * Checks that private members are included in the hash of the public API of traits.
+   * Including the private members of traits is required because classes that implement a trait
+   * have to define the private members of the trait. Therefore, if a private member of a trait is added,
+   * modified or removed we need to recompile the classes that implement this trait.
+   * For instance, if trait Foo is initially defined as:
+   *     trait Foo { private val x = new A }
+   * changing it to
+   *     trait Foo { private val x = new B }
+   * requires us to recompile all implementors of trait Foo, because scalac generates setters and getters
+   * for the private fields of trait Foo in its implementor. If the clients of trait Foo are not recompiled,
+   * we get abstract method errors at runtime, because the types expected by the setter (for instance) does not
+   * match.
+   */
+  "private members in traits" in {
+    /* trait Foo { private val x } */
+    val fooTrait1 =
+      simpleTrait("Foo",
+        simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)),
+        publicAccess)
+
+    /* trait Foo */
+    val fooTrait2 =
+      simpleTrait("Foo",
+        simpleStructure(),
+        publicAccess)
+
+    val api1 = new SourceAPI(Array.empty, Array(fooTrait1))
+    val api2 = new SourceAPI(Array.empty, Array(fooTrait2))
+
+    HashAPI(api1) !== HashAPI(api2)
+
+  }
+
+  /**
+   * Checks that private members in non-top-level traits are included as well.
+   */
+  "private members in nested traits" in {
+    /* class A { trait Foo { private val x } } */
+    val classA1 =
+      simpleClass("A",
+        simpleTrait("Foo",
+          simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)),
+          publicAccess))
+
+    /* class A { trait Foo } */
+    val classA2 =
+      simpleClass("A",
+        simpleTrait("Foo",
+          simpleStructure(),
+          publicAccess))
+
+    val api1 = new SourceAPI(Array.empty, Array(classA1))
+    val api2 = new SourceAPI(Array.empty, Array(classA2))
+
+    HashAPI(api1) !== HashAPI(api2)
+
+  }
+
+  /**
+   * Checks that private traits are NOT included in the hash.
+   */
+  "private traits" in {
+    /* class Foo { private trait T { private val x } } */
+    val classFoo1 =
+      simpleClass("Foo",
+        simpleTrait("T",
+          simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)),
+          privateAccess))
+
+    /** class Foo { private trait T } */
+    val classFoo2 =
+      simpleClass("Foo",
+        simpleTrait("T",
+          simpleStructure(),
+          privateAccess))
+
+    /** class Foo */
+    val classFoo3 =
+      simpleClass("Foo")
+
+    val api1 = new SourceAPI(Array.empty, Array(classFoo1))
+    val api2 = new SourceAPI(Array.empty, Array(classFoo2))
+    val api3 = new SourceAPI(Array.empty, Array(classFoo3))
+
+    HashAPI(api1) === HashAPI(api2) && HashAPI(api2) === HashAPI(api3)
+  }
+
+  /**
+   * Checks that private members are NOT included in the hash of the public API of classes.
+   */
+  "private members in classes" in {
+    /* class Foo { private val x } */
+    val classFoo1 =
+      simpleClass("Foo",
+        simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)))
+
+    /* class Foo */
+    val classFoo2 =
+      simpleClass("Foo",
+        simpleStructure())
+
+    val api1 = new SourceAPI(Array.empty, Array(classFoo1))
+    val api2 = new SourceAPI(Array.empty, Array(classFoo2))
+
+    HashAPI(api1) === HashAPI(api2)
+
+  }
+
   private def assertNameHashEqualForRegularName(name: String, nameHashes1: _internalOnly_NameHashes,
     nameHashes2: _internalOnly_NameHashes) = {
     val nameHash1 = nameHashForRegularName(nameHashes1, name)
@@ -204,10 +313,15 @@ class NameHashingSpecification extends Specification {
     new ClassLike(DefinitionType.ClassDef, lzy(emptyType), lzy(structure), Array.empty, Array.empty, name, publicAccess, defaultModifiers, Array.empty)
   }
 
+  private def simpleTrait(name: String, structure: Structure, access: Access): ClassLike = {
+    new ClassLike(DefinitionType.Trait, lzy(emptyType), lzy(structure), Array.empty, Array.empty, name, access, defaultModifiers, Array.empty)
+  }
+
   private val emptyType = new EmptyType
   private val intTpe = new Projection(emptyType, "Int")
   private val strTpe = new Projection(emptyType, "String")
   private val publicAccess = new Public
+  private val privateAccess = new Private(new Unqualified)
   private val defaultModifiers = new Modifiers(false, false, false, false, false, false, false)
 
 }