Merge pull request #2438 from Duhemm/wip/package-object-filename

Do not rely on filename of package objects

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

@@ -17,6 +17,28 @@ object APIUtil {
 
   def isScalaSourceName(name: String): Boolean = name.endsWith(".scala")
 
+  def hasPackageObject(s: SourceAPI): Boolean =
+    {
+      val check = new HasPackageObject
+      check.visitAPI(s)
+      check.hasPackageObject
+    }
+
+  private[this] class HasPackageObject extends Visit {
+    private var _hasPackageObject = false
+
+    def hasPackageObject = _hasPackageObject
+
+    private def isPackageObject(c: ClassLike): Boolean = {
+      import xsbti.api.DefinitionType.{ Module, PackageModule }
+      c.definitionType == PackageModule || (c.definitionType == Module && c.name.endsWith(".package"))
+    }
+
+    override def visitClass0(c: ClassLike): Unit = {
+      _hasPackageObject ||= isPackageObject(c)
+    }
+  }
+
   def hasMacro(s: SourceAPI): Boolean =
     {
       val check = new HasMacro
@@ -25,7 +47,9 @@ object APIUtil {
     }
 
   private[this] class HasMacro extends Visit {
-    var hasMacro = false
+    private var _hasMacro = false
+
+    def hasMacro = _hasMacro
 
     // Don't visit inherited definitions since we consider that a class
     // that inherits a macro does not have a macro.
@@ -35,7 +59,7 @@ object APIUtil {
     }
 
     override def visitModifiers(m: Modifiers): Unit = {
-      hasMacro ||= m.isMacro
+      _hasMacro ||= m.isMacro
       super.visitModifiers(m)
     }
   }

compile/inc/src/main/scala/sbt/inc/APIs.scala

@@ -38,6 +38,8 @@ trait APIs {
 
   def internal: Map[File, Source]
   def external: Map[String, Source]
+
+  def hasPackageObject(src: File): Boolean
 }
 object APIs {
   def apply(internal: Map[File, Source], external: Map[String, Source]): APIs = new MAPIs(internal, external)
@@ -46,7 +48,7 @@ object APIs {
   val emptyAPI = new xsbti.api.SourceAPI(Array(), Array())
   val emptyCompilation = new xsbti.api.Compilation(-1, Array())
   val emptyNameHashes = new xsbti.api._internalOnly_NameHashes(Array.empty, Array.empty)
-  val emptySource = new xsbti.api.Source(emptyCompilation, Array(), emptyAPI, 0, emptyNameHashes, false)
+  val emptySource = new xsbti.api.Source(emptyCompilation, Array(), emptyAPI, 0, emptyNameHashes, false, false)
   def getAPI[T](map: Map[T, Source], src: T): Source = map.getOrElse(src, emptySource)
 }
 
@@ -71,6 +73,8 @@ private class MAPIs(val internal: Map[File, Source], val external: Map[String, S
   def internalAPI(src: File) = getAPI(internal, src)
   def externalAPI(ext: String) = getAPI(external, ext)
 
+  override def hasPackageObject(src: File): Boolean = internalAPI(src).hasPackageObject
+
   override def equals(other: Any): Boolean = other match {
     case o: MAPIs => {
       def areEqual[T](x: Map[T, Source], y: Map[T, Source])(implicit ord: math.Ordering[T]) = {

compile/inc/src/main/scala/sbt/inc/Compile.scala

@@ -110,6 +110,8 @@ private final class AnalysisCallback(internalMap: File => Option[File], external
   private[this] val binaryClassName = new HashMap[File, String]
   // source files containing a macro def.
   private[this] val macroSources = Set[File]()
+  // source files defining a package object
+  private[this] val packageObjectSources = Set[File]()
 
   private def add[A, B](map: Map[A, Set[B]], a: A, b: B): Unit =
     map.getOrElseUpdate(a, new HashSet[B]) += b
@@ -186,7 +188,10 @@ private final class AnalysisCallback(internalMap: File => Option[File], external
 
   def api(sourceFile: File, source: SourceAPI): Unit = {
     import xsbt.api.{ APIUtil, HashAPI }
-    if (APIUtil.isScalaSourceName(sourceFile.getName) && APIUtil.hasMacro(source)) macroSources += sourceFile
+    if (APIUtil.isScalaSourceName(sourceFile.getName)) {
+      if (APIUtil.hasMacro(source)) macroSources += sourceFile
+      if (APIUtil.hasPackageObject(source)) packageObjectSources += sourceFile
+    }
     publicNameHashes(sourceFile) = {
       if (nameHashing)
         (new xsbt.api.NameHashing).nameHashes(source)
@@ -216,9 +221,11 @@ private final class AnalysisCallback(internalMap: File => Option[File], external
       case (a, (src, api)) =>
         val stamp = current.internalSource(src)
         val hash = stamp match { case h: Hash => h.value; case _ => new Array[Byte](0) }
+        val nameHashes = publicNameHashes(src)
         // TODO store this in Relations, rather than Source.
         val hasMacro: Boolean = macroSources.contains(src)
-        val s = new xsbti.api.Source(compilation, hash, api._2, api._1, publicNameHashes(src), hasMacro)
+        val hasPackageObject = packageObjectSources.contains(src)
+        val s = new xsbti.api.Source(compilation, hash, api._2, api._1, nameHashes, hasMacro, hasPackageObject)
         val info = SourceInfos.makeInfo(getOrNil(reporteds, src), getOrNil(unreporteds, src))
         val binaries = binaryDeps.getOrElse(src, Nil: Iterable[File])
         val prods = classes.getOrElse(src, Nil: Iterable[(File, String)])

compile/inc/src/main/scala/sbt/inc/IncrementalAntStyle.scala

@@ -7,7 +7,7 @@ import xsbti.api.Source
 private final class IncrementalAntStyle(log: Logger, options: IncOptions) extends IncrementalCommon(log, options) {
 
   /** Ant-style mode doesn't do anything special with package objects */
-  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations): Set[File] = Set.empty
+  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations, apis: APIs): Set[File] = Set.empty
 
   /** In Ant-style mode we don't need to compare APIs because we don't perform any invalidation */
   override protected def sameAPI[T](src: T, a: Source, b: Source): Option[APIChange[T]] = None

compile/inc/src/main/scala/sbt/inc/IncrementalCommon.scala

@@ -24,7 +24,7 @@ private[inc] abstract class IncrementalCommon(log: Logger, options: IncOptions)
     else {
       val wrappedLog = new Incremental.PrefixingLogger("[inv] ")(log)
       def debug(s: => String) = if (incDebug(options)) wrappedLog.debug(s) else ()
-      val withPackageObjects = invalidatedRaw ++ invalidatedPackageObjects(invalidatedRaw, previous.relations)
+      val withPackageObjects = invalidatedRaw ++ invalidatedPackageObjects(invalidatedRaw, previous.relations, previous.apis)
       val invalidated = expand(withPackageObjects, allSources)
       val pruned = Incremental.prune(invalidated, previous, classfileManager)
       debug("********* Pruned: \n" + pruned.relations + "\n*********")
@@ -54,7 +54,7 @@ private[inc] abstract class IncrementalCommon(log: Logger, options: IncOptions)
     } else invalidated
   }
 
-  protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations): Set[File]
+  protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations, apis: APIs): Set[File]
 
   /**
    * Logs API changes using debug-level logging. The API are obtained using the APIDiff class.

compile/inc/src/main/scala/sbt/inc/IncrementalDefaultImpl.scala

@@ -9,8 +9,8 @@ private final class IncrementalDefaultImpl(log: Logger, options: IncOptions) ext
 
   // Package objects are fragile: if they inherit from an invalidated source, get "class file needed by package is missing" error
   //  This might be too conservative: we probably only need package objects for packages of invalidated sources.
-  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations): Set[File] =
+  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations, apis: APIs): Set[File] =
-    invalidated flatMap relations.publicInherited.internal.reverse filter { _.getName == "package.scala" }
+    invalidated flatMap relations.publicInherited.internal.reverse filter apis.hasPackageObject
 
   override protected def sameAPI[T](src: T, a: Source, b: Source): Option[SourceAPIChange[T]] = {
     if (SameAPI(a, b))

compile/inc/src/main/scala/sbt/inc/IncrementalNameHashing.scala

@@ -17,10 +17,8 @@ private final class IncrementalNameHashing(log: Logger, options: IncOptions) ext
 
   // Package objects are fragile: if they inherit from an invalidated source, get "class file needed by package is missing" error
   //  This might be too conservative: we probably only need package objects for packages of invalidated sources.
-  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations): Set[File] =
+  override protected def invalidatedPackageObjects(invalidated: Set[File], relations: Relations, apis: APIs): Set[File] =
-    transitiveDeps(invalidated)(relations.inheritance.internal.reverse).filter {
+    transitiveDeps(invalidated)(relations.inheritance.internal.reverse) filter apis.hasPackageObject
-      _.getName == "package.scala"
-    }
 
   override protected def sameAPI[T](src: T, a: Source, b: Source): Option[APIChange[T]] = {
     if (SameAPI(a, b))

compile/inc/src/test/scala/sbt/inc/TestCaseGenerators.scala

@@ -110,8 +110,9 @@ object TestCaseGenerators {
     hash <- Gen.containerOfN[Array, Byte](hashLen, arbitrary[Byte])
     apiHash <- arbitrary[Int]
     hasMacro <- arbitrary[Boolean]
+    hasPackageObject <- arbitrary[Boolean]
     nameHashes <- genNameHashes(defns)
-  } yield new Source(new Compilation(startTime, Array()), hash, new SourceAPI(Array(), Array(defns map makeDefinition: _*)), apiHash, nameHashes, hasMacro)
+  } yield new Source(new Compilation(startTime, Array()), hash, new SourceAPI(Array(), Array(defns map makeDefinition: _*)), apiHash, nameHashes, hasMacro, hasPackageObject)
 
   def genSources(all_defns: Seq[Seq[String]]): Gen[Seq[Source]] = Gen.sequence[List, Source](all_defns.map(genSource))
 

interface/other

@@ -5,6 +5,7 @@ Source
 	apiHash: Int
 	_internalOnly_nameHashes: _internalOnly_NameHashes
 	hasMacro: Boolean
+	hasPackageObject: Boolean
 
 _internalOnly_NameHashes
 	regularMembers: _internalOnly_NameHash*

sbt/src/sbt-test/source-dependencies/package-object-name/A.scala

@@ -0,0 +1,3 @@
+package b
+
+class A

sbt/src/sbt-test/source-dependencies/package-object-name/b.scala

@@ -0,0 +1 @@
+package object b extends A

sbt/src/sbt-test/source-dependencies/package-object-name/changes/A1.scala

@@ -0,0 +1,5 @@
+package b
+
+class A {
+  def foo = 1
+}

sbt/src/sbt-test/source-dependencies/package-object-name/test

@@ -0,0 +1,3 @@
+> compile
+$ copy-file changes/A1.scala A.scala
+> compile