Merge pull request #1282 from dwijnand/macro-under-compilation

internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala

@@ -178,7 +178,7 @@ private[inc] abstract class IncrementalCommon(
           if (isFullCompilation) Set.empty[String]
           else
             invalidateAfterInternalCompilation(
-              analysis.relations,
+              analysis,
               newApiChanges,
               recompiledClasses,
               cycleNum >= options.transitiveStep,
@@ -443,7 +443,7 @@ private[inc] abstract class IncrementalCommon(
   /**
    * Invalidates classes internally to a project after an incremental compiler run.
    *
-   * @param relations The relations produced by the immediate previous incremental compiler cycle.
+   * @param analysis The analysis produced by the immediate previous incremental compiler cycle.
    * @param changes The changes produced by the immediate previous incremental compiler cycle.
    * @param recompiledClasses The immediately recompiled class names.
    * @param invalidateTransitively A flag that tells whether transitive invalidations should be
@@ -453,12 +453,13 @@ private[inc] abstract class IncrementalCommon(
    * @return A list of invalidated class names for the next incremental compiler run.
    */
   def invalidateAfterInternalCompilation(
-      relations: Relations,
+      analysis: Analysis,
       changes: APIChanges,
       recompiledClasses: Set[String],
       invalidateTransitively: Boolean,
       isScalaClass: String => Boolean
   ): Set[String] = {
+    val relations = analysis.relations
     val initial = changes.allModified.toSet
     val dependsOnClass = findClassDependencies(_, relations)
     val firstClassInvalidation: Set[String] = {
@@ -479,15 +480,35 @@ private[inc] abstract class IncrementalCommon(
     if (secondClassInvalidation.nonEmpty)
       log.debug(s"Invalidated due to generated class file collision: ${secondClassInvalidation}")
 
-    val newInvalidations = (firstClassInvalidation -- recompiledClasses) ++ secondClassInvalidation
+    // Invalidate macro classes that transitively depend on any of the recompiled classes
+    //
+    // The macro expansion tree can depend on the behavioural change of any upstream code change,
+    // not just API changes, so correctness requires aggressive recompilation of downstream classes.
+    //
+    // Technically the macro doesn't need to be recompiled - it's the classes downstream of the macro
+    // that need to be recompiled, so that the macros can be re-expanded.  But recompiling is the most
+    // straightforward way to signal any classes downstream of the _macro_ that they need to recompile.
+    //
+    // Also, note, that this solution only works for behavioural changes in sources within the same
+    // subproject as the macro.  Changes in behaviour in upstream subprojects don't cause downstream
+    // macro classes to recompile - because downstream projects only have visibility of the upstream
+    // API, and if it changed, which is insufficient, and upstream projects have no other way than
+    // their API to signal to downstream.
+    val thirdClassInvalidation = {
+      val transitive = IncrementalCommon.transitiveDeps(recompiledClasses, log)(dependsOnClass)
+      (transitive -- recompiledClasses).filter(analysis.apis.internalAPI(_).hasMacro)
+    }
+
+    val newInvalidations =
+      (firstClassInvalidation -- recompiledClasses) ++ secondClassInvalidation ++ thirdClassInvalidation
     if (newInvalidations.isEmpty) {
       log.debug("No classes were invalidated.")
       Set.empty
     } else {
       if (invalidateTransitively) {
         newInvalidations ++ recompiledClasses
       } else {
-        firstClassInvalidation ++ secondClassInvalidation
+        firstClassInvalidation ++ secondClassInvalidation ++ thirdClassInvalidation
       }
     }
   }

zinc/src/sbt-test/macros/macro-use/app/App.scala

@@ -0,0 +1,13 @@
+package app
+
+object App {
+  def main(args: Array[String]): Unit = {
+    val res = lib.Macro.append("ABC")
+    val exp = args(0)
+    if (res != exp) {
+      val e = new Exception(s"assertion failed: expected $exp, obtained $res")
+      e.setStackTrace(Array())
+      throw e
+    }
+  }
+}

zinc/src/sbt-test/macros/macro-use/build.json

@@ -0,0 +1,6 @@
+{
+  "projects": [
+    { "name": "lib", "scalaVersion": "2.13.12" },
+    { "name": "app", "scalaVersion": "2.13.12", "dependsOn": [ "lib" ] }
+  ]
+}

zinc/src/sbt-test/macros/macro-use/lib/Access.scala

@@ -0,0 +1,5 @@
+package lib
+
+class Access {
+  def give = new Data().suffix
+}

zinc/src/sbt-test/macros/macro-use/lib/Data.scala

@@ -0,0 +1,5 @@
+package lib
+
+class Data {
+  def suffix = "_" + new InternalApi().value
+}

zinc/src/sbt-test/macros/macro-use/lib/InternalApi.scala

@@ -0,0 +1,5 @@
+package lib
+
+class InternalApi {
+  def value: Int = 1
+}

zinc/src/sbt-test/macros/macro-use/lib/Macro.scala

@@ -0,0 +1,15 @@
+package lib
+
+import scala.language.experimental.macros
+
+import scala.reflect.macros.blackbox.Context
+
+object Macro {
+  def append(s: String): String = macro impl
+
+  def impl(c: Context)(s: c.Tree): c.Tree = {
+    import c.universe._
+    val suffix = new Access().give
+    q"""$s + $suffix"""
+  }
+}

zinc/src/sbt-test/macros/macro-use/lib/changes/InternalApi.scala

@@ -0,0 +1,5 @@
+package lib
+
+class InternalApi {
+  def value: Int = 2
+}

zinc/src/sbt-test/macros/macro-use/test

@@ -0,0 +1,3 @@
+> app/run ABC_1
+$ copy-file lib/changes/InternalApi.scala lib/InternalApi.scala
+> app/run ABC_2