Merge pull request #2160 from Duhemm/fix-2155

Include private members in API hash of traits.

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 {
@@ -276,7 +278,7 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include
     hashSeq(ts, (t: Type) => hashType(t, includeDefinitions))
   def hashType(t: Type, includeDefinitions: Boolean = true): Unit =
     t match {
-      case s: Structure     => hashStructure(s, includeDefinitions)
+      case s: Structure     => hashStructure(s, includeDefinitions, isTrait = false)
       case e: Existential   => hashExistential(e)
       case c: Constant      => hashConstant(c)
       case p: Polymorphic   => hashPolymorphic(p)
@@ -343,14 +345,20 @@ 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 = {
+  @deprecated("Use the overload that indicates if the definition is a trait.", "0.14")
+  final def hashStructure(structure: Structure, includeDefinitions: Boolean): Unit =
+    hashStructure(structure, includeDefinitions, isTrait = false)
+  final def hashStructure(structure: Structure, includeDefinitions: Boolean, isTrait: Boolean = false): Unit =
+    visit(visitedStructures, structure)(structure => hashStructure0(structure, includeDefinitions, isTrait))
+  @deprecated("Use the overload that indicates if the definition is a trait.", "0.14")
+  def hashStructure0(structure: Structure, includeDefinitions: Boolean): Unit =
+    hashStructure0(structure, includeDefinitions, isTrait = false)
+  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)
 
 }