diff --git a/compile/api/src/main/scala/sbt/ClassToAPI.scala b/compile/api/src/main/scala/sbt/ClassToAPI.scala index 9589865ec..1eb91f44b 100644 --- a/compile/api/src/main/scala/sbt/ClassToAPI.scala +++ b/compile/api/src/main/scala/sbt/ClassToAPI.scala @@ -261,7 +261,7 @@ object ClassToAPI { def modifiers(i: Int): api.Modifiers = { import Modifier.{ isAbstract, isFinal } - new api.Modifiers(isAbstract(i), false, isFinal(i), false, false, false, false) + new api.Modifiers(isAbstract(i), false, isFinal(i), false, false, false, false, false) } def access(i: Int, pkg: Option[String]): api.Access = { diff --git a/compile/api/src/main/scala/xsbt/api/APIUtil.scala b/compile/api/src/main/scala/xsbt/api/APIUtil.scala index d8c4e88cc..577d2cb8f 100644 --- a/compile/api/src/main/scala/xsbt/api/APIUtil.scala +++ b/compile/api/src/main/scala/xsbt/api/APIUtil.scala @@ -12,7 +12,7 @@ object APIUtil { } val byteToModifiers = (b: Byte) => { def x(bit: Int) = (b & (1 << bit)) != 0 - new Modifiers(x(0), x(1), x(2), x(3), x(4), x(5), x(6)) + new Modifiers(x(0), x(1), x(2), x(3), x(4), x(5), x(6), x(7)) } def isScalaSourceName(name: String): Boolean = name.endsWith(".scala") diff --git a/compile/api/src/main/scala/xsbt/api/HashAPI.scala b/compile/api/src/main/scala/xsbt/api/HashAPI.scala index 6ed4054e0..f600d5416 100644 --- a/compile/api/src/main/scala/xsbt/api/HashAPI.scala +++ b/compile/api/src/main/scala/xsbt/api/HashAPI.scala @@ -139,16 +139,37 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include { hash = startHash(0) hashSymmetric(s.packages, hashPackage) - hashDefinitions(s.definitions, true) + hashDefinitions(s.definitions, topLevel = true, isTrait = false) finalizeHash } def hashPackage(p: Package) = hashString(p.name) + @deprecated("Use the overload that indicates if the enclosing definition is a trait.", "0.14") def hashDefinitions(ds: Seq[Definition], topLevel: Boolean): Unit = + hashDefinitions(ds, topLevel, isTrait = false) + + def hashDefinitions(ds: Seq[Definition], topLevel: Boolean, isTrait: Boolean): Unit = { + def isPublic(d: Definition): Boolean = d.access match { case _: xsbti.api.Public => true; case _ => false } + def isTraitBreaker(d: Definition): Boolean = d match { + // Vars and vals in traits introduce getters, setters and fields in the implementing classes. + // See test `source-dependencies/trait-private-var + case _: FieldLike => true + // Objects in traits introduce fields in the implementing classes. + // See test `source-dependencies/trait-private-object` + case cl: ClassLike => cl.definitionType == DefinitionType.Module + // super calls introduce accessors that are not part of the public API + case d: Def => d.modifiers.isSuperAccessor + case _ => false + } + val includedPrivateDefinitions = + if (!includePrivate && !topLevel && isTrait) { + ds filter (x => isTraitBreaker(x) && !isPublic(x)) + } else Seq.empty + val defs = SameAPI.filterDefinitions(ds, topLevel, includePrivate) - hashSymmetric(defs, hashDefinition) + hashSymmetric(includedPrivateDefinitions ++ defs, hashDefinition) } /** @@ -356,9 +377,9 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include def hashStructure0(structure: Structure, includeDefinitions: Boolean, isTrait: Boolean = false): Unit = { extend(StructureHash) hashTypes(structure.parents, includeDefinitions) - if (includeDefinitions || isTrait) { - hashDefinitions(structure.declared, isTrait) - hashDefinitions(structure.inherited, isTrait) + if (includeDefinitions) { + hashDefinitions(structure.declared, topLevel = false, isTrait) + hashDefinitions(structure.inherited, topLevel = false, isTrait) } } def hashParameters(parameters: Seq[TypeParameter], base: Type): Unit = diff --git a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala index 4849838b2..1ec910058 100644 --- a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala +++ b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala @@ -165,20 +165,39 @@ class NameHashingSpecification extends Specification { } /** - * 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. + * Checks that private vars are included in the hash of the public API of traits. + * Including the private vars of traits is required because classes that implement a trait + * have to define getters and setters for these vars. * For instance, if trait Foo is initially defined as: - * trait Foo { private val x = new A } + * trait Foo { private var x = new A } * changing it to - * trait Foo { private val x = new B } + * trait Foo { private var 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, + * for the private vars 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 { + "private var in traits are included in API hash" in { + /* trait Foo { private var x } */ + val fooTrait1 = + simpleTrait("Foo", + simpleStructure(new Var(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) + + } + + "private vals in traits are included in API hash" in { /* trait Foo { private val x } */ val fooTrait1 = simpleTrait("Foo", @@ -198,15 +217,97 @@ class NameHashingSpecification extends Specification { } + "private objects in traits are included in API hash" in { + /* trait Foo { private object x } */ + val fooTrait1 = + simpleTrait("Foo", + simpleStructure( + new ClassLike(DefinitionType.Module, lzy(emptyType), lzy(simpleStructure()), Array.empty, Array.empty, "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) + + } + + "private non-synthetic def in traits are not included in API hash" in { + /* trait Foo { private def x } */ + val fooTrait1 = + simpleTrait("Foo", + simpleStructure(new Def(Array.empty, emptyType, Array.empty, "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) + + } + + "private synthetic def in traits are included in API hash" in { + /* trait Foo { private def x } */ + val modifiers = new xsbti.api.Modifiers(false, false, false, false, false, false, false, true) + val fooTrait1 = + simpleTrait("Foo", + simpleStructure(new Def(Array.empty, emptyType, Array.empty, "x", privateAccess, modifiers, 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) + + } + + "private types in traits are included not in API hash" in { + /* trait Foo { private type x } */ + val fooTrait1 = + simpleTrait("Foo", + simpleStructure(new TypeAlias(emptyType, Array.empty, "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. + * Checks that private vars in non-top-level traits are included as well. */ - "private members in nested traits" in { - /* class A { trait Foo { private val x } } */ + "private variables in nested traits are include in the API hash" in { + /* class A { trait Foo { private var x } } */ val classA1 = simpleClass("A", simpleTrait("Foo", - simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)), + simpleStructure(new Var(emptyType, "x", privateAccess, defaultModifiers, Array.empty)), publicAccess)) /* class A { trait Foo } */ @@ -226,12 +327,12 @@ class NameHashingSpecification extends Specification { /** * Checks that private traits are NOT included in the hash. */ - "private traits" in { - /* class Foo { private trait T { private val x } } */ + "private inner traits are not included in the API hash" in { + /* class Foo { private trait T { private var x } } */ val classFoo1 = simpleClass("Foo", simpleTrait("T", - simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)), + simpleStructure(new Var(emptyType, "x", privateAccess, defaultModifiers, Array.empty)), privateAccess)) /** class Foo { private trait T } */ @@ -256,10 +357,10 @@ class NameHashingSpecification extends Specification { * Checks that private members are NOT included in the hash of the public API of classes. */ "private members in classes are not included in the api hash" in { - /* class Foo { private val x } */ + /* class Foo { private var x } */ val classFoo1 = simpleClass("Foo", - simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty))) + simpleStructure(new Var(emptyType, "x", privateAccess, defaultModifiers, Array.empty))) /* class Foo */ val classFoo2 = @@ -274,9 +375,9 @@ class NameHashingSpecification extends Specification { } /** - * Checks that private members do NOT contribute to name hashes. - * Test for https://github.com/sbt/sbt/issues/2324 - */ + * Checks that private members do NOT contribute to name hashes. + * Test for https://github.com/sbt/sbt/issues/2324 + */ "private members in classes do not contribute to name hashes" in { /* class Foo { private val x } */ val classFoo = @@ -336,6 +437,6 @@ class NameHashingSpecification extends Specification { 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) + private val defaultModifiers = new Modifiers(false, false, false, false, false, false, false, false) } diff --git a/compile/inc/src/test/scala/sbt/inc/TestCaseGenerators.scala b/compile/inc/src/test/scala/sbt/inc/TestCaseGenerators.scala index 7591183b1..3379ced55 100644 --- a/compile/inc/src/test/scala/sbt/inc/TestCaseGenerators.scala +++ b/compile/inc/src/test/scala/sbt/inc/TestCaseGenerators.scala @@ -76,7 +76,7 @@ object TestCaseGenerators { private[this] def makeDefinition(name: String): Definition = new ClassLike(DefinitionType.ClassDef, lzy(new EmptyType()), lzy(new Structure(lzy(Array()), lzy(Array()), lzy(Array()))), Array(), Array(), - name, new Public(), new Modifiers(false, false, false, false, false, false, false), Array()) + name, new Public(), new Modifiers(false, false, false, false, false, false, false, false), Array()) private[this] def lzy[T <: AnyRef](x: T) = SafeLazy.strict(x) diff --git a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala index 011479be6..07336d31b 100644 --- a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala +++ b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala @@ -438,7 +438,7 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType, val absOver = s.hasFlag(ABSOVERRIDE) val abs = s.hasFlag(ABSTRACT) || s.hasFlag(DEFERRED) || absOver val over = s.hasFlag(OVERRIDE) || absOver - new xsbti.api.Modifiers(abs, over, s.isFinal, s.hasFlag(SEALED), isImplicit(s), s.hasFlag(LAZY), hasMacro(s)) + new xsbti.api.Modifiers(abs, over, s.isFinal, s.hasFlag(SEALED), isImplicit(s), s.hasFlag(LAZY), hasMacro(s), s.hasFlag(SUPERACCESSOR)) } private def isImplicit(s: Symbol) = s.hasFlag(Flags.IMPLICIT) diff --git a/interface/src/main/java/xsbti/api/Modifiers.java b/interface/src/main/java/xsbti/api/Modifiers.java index 5e103c7ec..c91c3ec28 100644 --- a/interface/src/main/java/xsbti/api/Modifiers.java +++ b/interface/src/main/java/xsbti/api/Modifiers.java @@ -9,13 +9,14 @@ public final class Modifiers implements java.io.Serializable private static final int ImplicitBit = 4; private static final int LazyBit = 5; private static final int MacroBit = 6; + private static final int SuperAccessorBit = 7; private static int flag(boolean set, int bit) { return set ? (1 << bit) : 0; } - public Modifiers(boolean isAbstract, boolean isOverride, boolean isFinal, boolean isSealed, boolean isImplicit, boolean isLazy, boolean isMacro) + public Modifiers(boolean isAbstract, boolean isOverride, boolean isFinal, boolean isSealed, boolean isImplicit, boolean isLazy, boolean isMacro, boolean isSuperAccessor) { this.flags = (byte)( flag(isAbstract, AbstractBit) | @@ -24,7 +25,8 @@ public final class Modifiers implements java.io.Serializable flag(isSealed, SealedBit) | flag(isImplicit, ImplicitBit) | flag(isLazy, LazyBit) | - flag(isMacro, MacroBit) + flag(isMacro, MacroBit) | + flag(isSuperAccessor, SuperAccessorBit) ); } @@ -68,6 +70,10 @@ public final class Modifiers implements java.io.Serializable { return flag(MacroBit); } + public final boolean isSuperAccessor() + { + return flag(SuperAccessorBit); + } public boolean equals(Object o) { return (o instanceof Modifiers) && flags == ((Modifiers)o).flags; @@ -78,6 +84,6 @@ public final class Modifiers implements java.io.Serializable } public String toString() { - return "Modifiers(" + "isAbstract: " + isAbstract() + ", " + "isOverride: " + isOverride() + ", " + "isFinal: " + isFinal() + ", " + "isSealed: " + isSealed() + ", " + "isImplicit: " + isImplicit() + ", " + "isLazy: " + isLazy() + ", " + "isMacro: " + isMacro()+ ")"; + return "Modifiers(" + "isAbstract: " + isAbstract() + ", " + "isOverride: " + isOverride() + ", " + "isFinal: " + isFinal() + ", " + "isSealed: " + isSealed() + ", " + "isImplicit: " + isImplicit() + ", " + "isLazy: " + isLazy() + ", " + "isMacro: " + isMacro()+ ", isSuperAccessor:" + isSuperAccessor() + ")"; } } diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt b/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt new file mode 100644 index 000000000..949d78231 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt @@ -0,0 +1,27 @@ +/* Performs checks related to compilations: + * a) checks in which compilation given set of files was recompiled + * b) checks overall number of compilations performed + */ +TaskKey[Unit]("check-compilations") := { + val analysis = (compile in Compile).value + val srcDir = (scalaSource in Compile).value + def relative(f: java.io.File): java.io.File = f.relativeTo(srcDir) getOrElse f + val allCompilations = analysis.compilations.allCompilations + val recompiledFiles: Seq[Set[java.io.File]] = allCompilations map { c => + val recompiledFiles = analysis.apis.internal.collect { + case (file, api) if api.compilation.startTime == c.startTime => relative(file) + } + recompiledFiles.toSet + } + def recompiledFilesInIteration(iteration: Int, fileNames: Set[String]) = { + val files = fileNames.map(new java.io.File(_)) + assert(recompiledFiles(iteration) == files, "%s != %s".format(recompiledFiles(iteration), files)) + } + assert(allCompilations.size == 2) + // B.scala is just compiled at the beginning + recompiledFilesInIteration(0, Set("B.scala")) + // A.scala is changed and recompiled + recompiledFilesInIteration(1, Set("A.scala")) +} + +logLevel := Level.Debug diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/changes/A1.scala b/sbt/src/sbt-test/source-dependencies/trait-member-modified/changes/A1.scala new file mode 100644 index 000000000..57a1f34c6 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/changes/A1.scala @@ -0,0 +1,3 @@ +trait A { + def foo: Int = 12 +} diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/A.scala b/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/A.scala new file mode 100644 index 000000000..0eab80adc --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/A.scala @@ -0,0 +1 @@ +trait A diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/B.scala b/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/B.scala new file mode 100644 index 000000000..c4d3f7e97 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/src/main/scala/B.scala @@ -0,0 +1 @@ +class B(a: A) diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/test b/sbt/src/sbt-test/source-dependencies/trait-member-modified/test new file mode 100644 index 000000000..f8f7cb076 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/test @@ -0,0 +1,9 @@ +# Test if adding a member to a trait affects classes that refer to that trait +# by a member reference +> compile +# add `foo` method to `A` +$ copy-file changes/A1.scala src/main/scala/A.scala +# only A.scala should be recompiled +> compile +# check if there are only two compile iterations performed +> check-compilations diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-object/A.scala b/sbt/src/sbt-test/source-dependencies/trait-private-object/A.scala new file mode 100644 index 000000000..cbcda3176 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-private-object/A.scala @@ -0,0 +1,3 @@ +trait A { + val foo = 0 +} \ No newline at end of file diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-object/B.scala b/sbt/src/sbt-test/source-dependencies/trait-private-object/B.scala new file mode 100644 index 000000000..5da0f8a71 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-private-object/B.scala @@ -0,0 +1,5 @@ +object B extends A { + def main(args: Array[String]): Unit = { + println(foo) + } +} diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-object/changes/A.scala b/sbt/src/sbt-test/source-dependencies/trait-private-object/changes/A.scala new file mode 100644 index 000000000..b5600153b --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-private-object/changes/A.scala @@ -0,0 +1,4 @@ +trait A { + val foo = 0 + X.a + private object X { val a = 1 } +} \ No newline at end of file diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-object/test b/sbt/src/sbt-test/source-dependencies/trait-private-object/test new file mode 100644 index 000000000..5aab7a143 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-private-object/test @@ -0,0 +1,5 @@ +> run + +$ copy-file changes/A.scala A.scala + +> run \ No newline at end of file diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-var/A.scala b/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A0.scala similarity index 100% rename from sbt/src/sbt-test/source-dependencies/trait-private-var/A.scala rename to sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A0.scala diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A.scala b/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A1.scala similarity index 100% rename from sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A.scala rename to sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A1.scala diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A2.scala b/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A2.scala new file mode 100644 index 000000000..60641457d --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/trait-private-var/changes/A2.scala @@ -0,0 +1,5 @@ +trait A { + private val foo = 12 + // we need to access foo to trigger AbstractMethodError + def bar: Int = foo +} diff --git a/sbt/src/sbt-test/source-dependencies/trait-private-var/test b/sbt/src/sbt-test/source-dependencies/trait-private-var/test index d928246a9..c120697d8 100644 --- a/sbt/src/sbt-test/source-dependencies/trait-private-var/test +++ b/sbt/src/sbt-test/source-dependencies/trait-private-var/test @@ -1,14 +1,28 @@ +$ copy-file changes/A0.scala A.scala + # compile and run for the first time to verify that everything works > run # introduce private var and refer to it in a trait that we inherit from # there'll be pair of getters and setters generated for private var that # has to be implemented by a class (where you can declare corresponding field) -$ copy-file changes/A.scala A.scala +$ copy-file changes/A1.scala A.scala -# this fails with AbstractMethodError because getters and setters for -# a private var are not generated because introduction of a private var -# does not trigger recompilation of B -# B is not recompiled because incremental compiler tracks only public -# interace (members visible from outside of given trait/class) +# If the introduction of a private var did not trigger the recompilation of B, +# then this will fail with AbstractMethodError because the getters and setters +# for the private var have not been generated. +> run + +# Try again with a private val +> clean + +$ copy-file changes/A0.scala A.scala + +# compile and run a clean project to verify that everything works +> run + +# introduce a private val in the trait +$ copy-file changes/A2.scala A.scala + +# Verify that B has been recompiled and that everything runs fine. > run