mirror of https://github.com/sbt/sbt.git
Merge pull request #2325 from gkossakowski/nameHashing/private-members
Do not compute name hashes for private members.
This commit is contained in:
eugene yokota
commit
c5b0c7b733
|
|
@ -60,6 +60,15 @@ object APIUtil {
|
|||
new Structure(lzy(s.parents), filterDefinitions(s.declared, isModule), filterDefinitions(s.inherited, isModule))
|
||||
def filterDefinitions(ds: Array[Definition], isModule: Boolean): Lazy[Array[Definition]] =
|
||||
lzy(if (isModule) ds filter Discovery.isMainMethod else Array())
|
||||
|
||||
def isNonPrivate(d: Definition): Boolean = isNonPrivate(d.access)
|
||||
/** Returns false if the `access` is `Private` and qualified, true otherwise.*/
|
||||
def isNonPrivate(access: Access): Boolean =
|
||||
access match {
|
||||
case p: Private if !p.qualifier.isInstanceOf[IdQualifier] => false
|
||||
case _ => true
|
||||
}
|
||||
|
||||
private[this] def lzy[T <: AnyRef](t: T): Lazy[T] = SafeLazy.strict(t)
|
||||
|
||||
private[this] val emptyType = new EmptyType
|
||||
|
|
|
|||
|
|
@ -78,7 +78,9 @@ class NameHashing {
|
|||
visitDefinition(topLevelDef)
|
||||
}
|
||||
}
|
||||
override def visitDefinition(d: Definition): Unit = {
|
||||
// if the definition is private, we do not visit because we do
|
||||
// not want to include any private members or its children
|
||||
override def visitDefinition(d: Definition): Unit = if (APIUtil.isNonPrivate(d)) {
|
||||
val locatedDef = LocatedDefinition(currentLocation, d)
|
||||
locatedDefs += locatedDef
|
||||
d match {
|
||||
|
|
|
|||
|
|
@ -89,14 +89,9 @@ object SameAPI {
|
|||
* All top-level definitions are always considered: 'private' only means package-private.
|
||||
* Other definitions are considered if they are not qualified with 'private[this]' or 'private'.
|
||||
*/
|
||||
def filterDefinitions(d: Seq[Definition], topLevel: Boolean, includePrivate: Boolean) = if (topLevel || includePrivate) d else d.filter(isNonPrivate)
|
||||
def isNonPrivate(d: Definition): Boolean = isNonPrivate(d.access)
|
||||
/** Returns false if the `access` is `Private` and qualified, true otherwise.*/
|
||||
def isNonPrivate(access: Access): Boolean =
|
||||
access match {
|
||||
case p: Private if !p.qualifier.isInstanceOf[IdQualifier] => false
|
||||
case _ => true
|
||||
}
|
||||
def filterDefinitions(d: Seq[Definition], topLevel: Boolean, includePrivate: Boolean) =
|
||||
if (topLevel || includePrivate) d else d.filter(APIUtil.isNonPrivate)
|
||||
|
||||
}
|
||||
/**
|
||||
* Used to implement API equality.
|
||||
|
|
|
|||
|
|
@ -255,7 +255,7 @@ class NameHashingSpecification extends Specification {
|
|||
/**
|
||||
* Checks that private members are NOT included in the hash of the public API of classes.
|
||||
*/
|
||||
"private members in classes" in {
|
||||
"private members in classes are not included in the api hash" in {
|
||||
/* class Foo { private val x } */
|
||||
val classFoo1 =
|
||||
simpleClass("Foo",
|
||||
|
|
@ -273,6 +273,20 @@ class NameHashingSpecification extends Specification {
|
|||
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 =
|
||||
simpleClass("Foo",
|
||||
simpleStructure(new Val(emptyType, "x", privateAccess, defaultModifiers, Array.empty)))
|
||||
val nameHashes = nameHashesForClass(classFoo)
|
||||
// make sure there's no name hash for the private member "x"
|
||||
Seq("Foo") === nameHashes.regularMembers.map(_.name).toSeq
|
||||
}
|
||||
|
||||
private def assertNameHashEqualForRegularName(name: String, nameHashes1: _internalOnly_NameHashes,
|
||||
nameHashes2: _internalOnly_NameHashes) = {
|
||||
val nameHash1 = nameHashForRegularName(nameHashes1, name)
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import scala.collection.mutable.{ HashMap, HashSet, ListBuffer }
|
|||
import xsbti.api.{ ClassLike, DefinitionType, PathComponent, SimpleType }
|
||||
|
||||
/**
|
||||
* Extracts API representation out of Symbols and Types.
|
||||
* Extracts full (including private members) API representation out of Symbols and Types.
|
||||
*
|
||||
* Each compilation unit should be processed by a fresh instance of this class.
|
||||
*
|
||||
|
|
@ -18,6 +18,12 @@ import xsbti.api.{ ClassLike, DefinitionType, PathComponent, SimpleType }
|
|||
* it has a call to `addInheritedDependencies` method defined in CallbackGlobal. In the future
|
||||
* we should refactor this code so inherited dependencies are just accumulated in a buffer and
|
||||
* exposed to a client that can pass them to an instance of CallbackGlobal it holds.
|
||||
*
|
||||
* NOTE: This class extract *full* API representation. In most of other places in the incremental compiler,
|
||||
* only non-private (accessible from other compilation units) members are relevant. Other parts of the
|
||||
* incremental compiler filter out private definitions before processing API structures. Check SameAPI for
|
||||
* an example.
|
||||
*
|
||||
*/
|
||||
class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
|
||||
// Tracks the source file associated with the CompilationUnit currently being processed by the API phase.
|
||||
|
|
|
|||
|
|
@ -3,11 +3,17 @@
|
|||
*/
|
||||
package xsbti.api;
|
||||
|
||||
import java.io.ObjectStreamException;
|
||||
import java.io.ObjectStreamException;
|
||||
|
||||
public abstract class AbstractLazy<T> implements Lazy<T>, java.io.Serializable
|
||||
{
|
||||
private Object writeReplace() throws ObjectStreamException
|
||||
// `writeReplace` must be `protected`, so that the `Impl` subclass
|
||||
// inherits the serialization override.
|
||||
//
|
||||
// (See source-dependencies/java-analysis-serialization-error, which would
|
||||
// crash trying to serialize an AbstractLazy, because it pulled in an
|
||||
// unserializable type eventually.)
|
||||
protected Object writeReplace() throws ObjectStreamException
|
||||
{
|
||||
return new StrictLazy<T>(get());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,25 @@
|
|||
/* 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"))
|
||||
}
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
object A {
|
||||
private def foo: String = "1"
|
||||
def bar: String = "29"
|
||||
def xyz: Int = 101
|
||||
}
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
object A {
|
||||
private def foo: Int = 1
|
||||
def bar: Int = 29
|
||||
def xyz: Int = 101
|
||||
}
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
class B {
|
||||
private def foo: Int = 1
|
||||
def baz(): Unit = println(foo, A.xyz)
|
||||
}
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
# Test for https://github.com/sbt/sbt/issues/2324
|
||||
# introduces first compile iteration
|
||||
> compile
|
||||
# change signature of a private `A.foo` method
|
||||
$ copy-file changes/A1.scala src/main/scala/A.scala
|
||||
# it should recompile just A.scala because changes to `A.foo` can't affect B
|
||||
> compile
|
||||
# check if there are only two compile iterations performed
|
||||
> check-compilations
|
||||
|
|
@ -0,0 +1 @@
|
|||
public class Outer { private class T extends Thread {} }
|
||||
|
|
@ -0,0 +1 @@
|
|||
incOptions := incOptions.value.withNameHashing(true).withApiDebug(true)
|
||||
|
|
@ -0,0 +1 @@
|
|||
> compile
|
||||
Loading…
Reference in New Issue