From 65768446986d4e521cdb3f82f807d4886474c216 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 10 Feb 2016 18:15:44 +0100 Subject: [PATCH] Hash of traits: include private fields, objects and super accessors --- .../api/src/main/scala/sbt/ClassToAPI.scala | 2 +- .../api/src/main/scala/xsbt/api/APIUtil.scala | 2 +- .../api/src/main/scala/xsbt/api/HashAPI.scala | 22 +++++--- .../xsbt/api/NameHashingSpecification.scala | 52 ++++++++++++++++--- .../scala/sbt/inc/TestCaseGenerators.scala | 2 +- .../src/main/scala/xsbt/ExtractAPI.scala | 2 +- .../src/main/java/xsbti/api/Modifiers.java | 12 +++-- .../trait-private-object/A.scala | 3 ++ .../trait-private-object/B.scala | 5 ++ .../trait-private-object/changes/A.scala | 4 ++ .../trait-private-object/test | 5 ++ 11 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 sbt/src/sbt-test/source-dependencies/trait-private-object/A.scala create mode 100644 sbt/src/sbt-test/source-dependencies/trait-private-object/B.scala create mode 100644 sbt/src/sbt-test/source-dependencies/trait-private-object/changes/A.scala create mode 100644 sbt/src/sbt-test/source-dependencies/trait-private-object/test 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 0f0baac56..f600d5416 100644 --- a/compile/api/src/main/scala/xsbt/api/HashAPI.scala +++ b/compile/api/src/main/scala/xsbt/api/HashAPI.scala @@ -151,17 +151,25 @@ final class HashAPI(includePrivate: Boolean, includeParamNames: Boolean, include def hashDefinitions(ds: Seq[Definition], topLevel: Boolean, isTrait: Boolean): Unit = { - // If the enclosing definition is a trait, then we must include private vars and vals in the API hash - // of the trait, because scalac will generate setters, getters and fields for them in the traits - // implementors. - val traitPrivateFields = + 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) { - def isPublic(d: Definition): Boolean = d.access match { case _: xsbti.api.Public => true; case _ => false } - ds.collect { case fl: FieldLike if !isPublic(fl) => fl } + ds filter (x => isTraitBreaker(x) && !isPublic(x)) } else Seq.empty val defs = SameAPI.filterDefinitions(ds, topLevel, includePrivate) - hashSymmetric(traitPrivateFields ++ defs, hashDefinition) + hashSymmetric(includedPrivateDefinitions ++ defs, hashDefinition) } /** diff --git a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala index 5d75708dd..1ec910058 100644 --- a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala +++ b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala @@ -176,8 +176,6 @@ class NameHashingSpecification extends Specification { * 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. - * - * NOTE: This logic is important for vars and vals only. No other private member needs to be included. */ "private var in traits are included in API hash" in { /* trait Foo { private var x } */ @@ -219,7 +217,28 @@ class NameHashingSpecification extends Specification { } - "private def in traits are not included in API hash" in { + "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", @@ -239,6 +258,27 @@ class NameHashingSpecification extends Specification { } + "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 = @@ -262,7 +302,7 @@ class NameHashingSpecification extends Specification { /** * Checks that private vars in non-top-level traits are included as well. */ - "private members in nested traits" in { + "private variables in nested traits are include in the API hash" in { /* class A { trait Foo { private var x } } */ val classA1 = simpleClass("A", @@ -287,7 +327,7 @@ class NameHashingSpecification extends Specification { /** * Checks that private traits are NOT included in the hash. */ - "private traits" in { + "private inner traits are not included in the API hash" in { /* class Foo { private trait T { private var x } } */ val classFoo1 = simpleClass("Foo", @@ -397,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-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