Merge pull request #9694 from dotty-staging/fix-incremental-inner

Author: smarter

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -250,6 +250,8 @@ object Symbols {
 
     /** The source or class file from which this class or
      *  the class containing this symbol was generated, null if not applicable.
+     *  Note that this the returned classfile might be the top-level class
+     *  containing this symbol instead of the directly enclosing class.
      *  Overridden in ClassSymbol
      */
     def associatedFile(using Context): AbstractFile =

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

@@ -112,32 +112,22 @@ class ExtractDependencies extends Phase {
     def binaryDependency(file: File, binaryClassName: String) =
       ctx.sbtCallback.binaryDependency(file, binaryClassName, fromClassName, sourceFile, dep.context)
 
-    def processExternalDependency(depFile: AbstractFile) = {
-      def binaryClassName(classSegments: List[String]) =
-        classSegments.mkString(".").stripSuffix(".class")
-
+    def processExternalDependency(depFile: AbstractFile, binaryClassName: String) = {
       depFile match {
         case ze: ZipArchive#Entry => // The dependency comes from a JAR
-          for (zip <- ze.underlyingSource; zipFile <- Option(zip.file)) {
-            val classSegments = io.File(ze.path).segments
-            binaryDependency(zipFile, binaryClassName(classSegments))
-          }
-
+          ze.underlyingSource match
+            case Some(zip) if zip.file != null =>
+              binaryDependency(zip.file, binaryClassName)
+            case _ =>
         case pf: PlainFile => // The dependency comes from a class file
-          val packages = dep.to.ownersIterator
-            .count(x => x.is(PackageClass) && !x.isEffectiveRoot)
-          // We can recover the fully qualified name of a classfile from
-          // its path
-          val classSegments = pf.givenPath.segments.takeRight(packages + 1)
           // FIXME: pf.file is null for classfiles coming from the modulepath
           // (handled by JrtClassPath) because they cannot be represented as
           // java.io.File, since the `binaryDependency` callback must take a
           // java.io.File, this means that we cannot record dependencies coming
           // from the modulepath. For now this isn't a big deal since we only
           // support having the standard Java library on the modulepath.
-          if (pf.file != null)
-            binaryDependency(pf.file, binaryClassName(classSegments))
-
+          if pf.file != null then
+            binaryDependency(pf.file, binaryClassName)
         case _ =>
           report.warning(s"sbt-deps: Ignoring dependency $depFile of class ${depFile.getClass}}")
       }
@@ -149,7 +139,25 @@ class ExtractDependencies extends Phase {
       def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance
       if (depFile.extension == "class") {
         // Dependency is external -- source is undefined
-        processExternalDependency(depFile)
+
+        // The fully qualified name on the JVM of the class corresponding to `dep.to`
+        val binaryClassName = {
+          val builder = new StringBuilder
+          val pkg = dep.to.enclosingPackageClass
+          if (!pkg.isEffectiveRoot) {
+            builder.append(pkg.fullName.mangledString)
+            builder.append(".")
+          }
+          val flatName = dep.to.flatName
+          // We create fake companion object symbols to hold the static members
+          // of Java classes, make sure to use the name of the actual Java class
+          // here.
+          val clsFlatName = if (dep.to.is(JavaDefined)) flatName.stripModuleClassSuffix else flatName
+          builder.append(clsFlatName.mangledString)
+          builder.toString
+        }
+
+        processExternalDependency(depFile, binaryClassName)
       } else if (allowLocal || depFile.file != sourceFile) {
         // We cannot ignore dependencies coming from the same source file because
         // the dependency info needs to propagate. See source-dependencies/trait-trait-211.

sbt-dotty/sbt-test/source-dependencies/inner-class/A.scala

@@ -0,0 +1,5 @@
+object A {
+  class InnerClass {
+    def foo: Int = 1
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-class/B.scala

@@ -0,0 +1,7 @@
+object B {
+  def main(args: Array[String]): Unit = {
+    val innerClass = new A.InnerClass
+    val x = innerClass.foo
+    println(x)
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-class/build.sbt

@@ -0,0 +1,12 @@
+import complete.DefaultParsers._
+
+val checkIterations = inputKey[Unit]("Verifies the accumlated number of iterations of incremental compilation.")
+
+checkIterations := {
+  val analysis = (compile in Compile).value.asInstanceOf[sbt.internal.inc.Analysis]
+
+  val expected: Int = (Space ~> NatBasic).parsed
+  val actual: Int = analysis.compilations.allCompilations.size
+  assert(expected == actual, s"Expected $expected compilations, got $actual")
+}
+

sbt-dotty/sbt-test/source-dependencies/inner-class/changes/A2.scala

@@ -0,0 +1,5 @@
+object A {
+  class InnerClass {
+    def foo: String = "foo"
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-class/changes/B2.scala

@@ -0,0 +1,8 @@
+object B {
+  def main(args: Array[String]): Unit = {
+    val innerClass = new A.InnerClass
+    val x = innerClass.foo
+    println(x)
+  }
+  val forceRecompileDummy = ""
+}

sbt-dotty/sbt-test/source-dependencies/inner-class/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion")
+  )
+}

sbt-dotty/sbt-test/source-dependencies/inner-class/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))

sbt-dotty/sbt-test/source-dependencies/inner-class/test

@@ -0,0 +1,12 @@
+> run
+
+# Recompile B (no meaningful change, this is just so that the dependencies on A.InnerClass are
+# recorded using the `binaryDependency` sbt callback, which will only work correctly
+# if we call it with the inner class binary name and not the top-level class name).
+$ copy-file changes/B2.scala B.scala
+> compile
+
+# Change the signature of A.Inner#foo, this requires B to be recompiled,
+# otherwise run will fail:
+$ copy-file changes/A2.scala A.scala
+> run

sbt-dotty/sbt-test/source-dependencies/inner-object/A.scala

@@ -0,0 +1,5 @@
+object A {
+  object InnerObject {
+    def bla: Int = 2
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-object/B.scala

@@ -0,0 +1,6 @@
+object B {
+  def main(args: Array[String]): Unit = {
+    val o = A.InnerObject.bla
+    println(o)
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-object/build.sbt

@@ -0,0 +1,12 @@
+import complete.DefaultParsers._
+
+val checkIterations = inputKey[Unit]("Verifies the accumlated number of iterations of incremental compilation.")
+
+checkIterations := {
+  val analysis = (compile in Compile).value.asInstanceOf[sbt.internal.inc.Analysis]
+
+  val expected: Int = (Space ~> NatBasic).parsed
+  val actual: Int = analysis.compilations.allCompilations.size
+  assert(expected == actual, s"Expected $expected compilations, got $actual")
+}
+

sbt-dotty/sbt-test/source-dependencies/inner-object/changes/A2.scala

@@ -0,0 +1,5 @@
+object A {
+  object InnerObject {
+    def bla: String = "bla"
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inner-object/changes/B2.scala

@@ -0,0 +1,7 @@
+object B {
+  def main(args: Array[String]): Unit = {
+    val o = A.InnerObject.bla
+    println(o)
+  }
+  val forceRecompileDummy = ""
+}

sbt-dotty/sbt-test/source-dependencies/inner-object/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion")
+  )
+}

sbt-dotty/sbt-test/source-dependencies/inner-object/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))

sbt-dotty/sbt-test/source-dependencies/inner-object/test

@@ -0,0 +1,12 @@
+> run
+
+# Recompile B (no meaningful change, this is just so that the dependency on A.InnerObject are
+# recorded using the `binaryDependency` sbt callback, which will only work correctly
+# if we call it with the inner object binary name and not the top-level class name).
+$ copy-file changes/B2.scala B.scala
+> compile
+
+# Change the signature of A.Inner#foo, this requires B to be recompiled,
+# otherwise run will fail:
+$ copy-file changes/A2.scala A.scala
+> run