[2.x] fix: Register every Def.declareOutput execution, not one per call site (#9492)

The cached-task macro allocated one mutable slot per syntactic
Def.declareOutput / Def.declareOutputDirectory call site and snapshotted the
slots into the task's outputs after the body ran. A call inside a loop or .map
over a runtime-determined list is a single syntactic site executed many times,
so each iteration overwrote the same slot and only the last file was cached and
restored on a cache hit. There was also no way for a conditional call site that
did not execute to stay out of the outputs: its slot remained null.

Declared outputs now accumulate in a per-task ListBuffer: the macro emits one
buffer at the top of the cached body and rewrites each call site to
ActionCache.registerOutput(vf, buffer), which appends and returns the value.
Every execution registers, an unexecuted site contributes nothing, and the
static multi-site shape is unchanged.

Refs #9462 (the declareOutput-in-a-loop half)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: eugene yokota <eed3si9n@gmail.com>

build.sbt

@@ -670,8 +670,12 @@ lazy val coreMacrosProj = (project in file("core-macros"))
     SettingKey[Boolean]("exportPipelining") := false,
     mimaSettings,
     mimaBinaryIssueFilters ++= Seq(
-      exclude[ReversedMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil.*"),
+      // macro-expansion internals; Output's per-call-site var codegen members were removed
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sbt.internal.util.appmacro.ContextUtil#Output.*"
+      ),
       exclude[DirectMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil#Input.*"),
+      exclude[ReversedMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil.*"),
     ),
   )
 

core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala

@@ -192,6 +192,16 @@ trait Cont:
 
       val inputBuf = ListBuffer[Input]()
       val outputBuf = ListBuffer[Output]()
+      lazy val outputAccSym: Symbol =
+        Symbol.newVal(
+          Symbol.spliceOwner,
+          freshName("outputs"),
+          TypeRepr.of[ListBuffer[VirtualFile]],
+          Flags.EmptyFlags,
+          Symbol.noSymbol
+        )
+      def outputAccRef: Expr[ListBuffer[VirtualFile]] =
+        Ref(outputAccSym).asExprOf[ListBuffer[VirtualFile]]
 
       def unitExpr: Expr[Unit] = '{ () }
 
@@ -405,28 +415,33 @@ trait Cont:
           }
 
       // This will generate following code for Def.declareOutput(...):
-      //   var $o1: VirtualFile = null
-      //   ActionCache.ActionResult({
+      //   val $outputs = ListBuffer.empty[VirtualFile]
+      //   ActionCache.InternalActionResult({
       //     body...
-      //     $o1 = out // Def.declareOutput(out)
+      //     ActionCache.registerOutput(out, $outputs) // Def.declareOutput(out)
       //     result
-      //   }, List($o1))
+      //   }, $outputs.toList)
       def letOutput[A1: Type](
           outputs: List[Output],
           cacheConfigExpr: Expr[BuildWideCacheConfiguration],
       )(body: Expr[A1]): Expr[ActionCache.InternalActionResult[A1]] =
-        Block(
-          outputs.map(_.toVarDef),
+        if outputs.isEmpty then
           '{
             ActionCache.InternalActionResult(
               value = $body,
-              outputs = List(${
-                Varargs[VirtualFile](outputs.map: out =>
-                  out.toRef.asExprOf[VirtualFile])
-              }*),
+              outputs = Nil,
             )
-          }.asTerm
-        ).asExprOf[ActionCache.InternalActionResult[A1]]
+          }
+        else
+          Block(
+            ValDef(outputAccSym, Some('{ ListBuffer.empty[VirtualFile] }.asTerm)) :: Nil,
+            '{
+              ActionCache.InternalActionResult(
+                value = $body,
+                outputs = $outputAccRef.toList,
+              )
+            }.asTerm
+          ).asExprOf[ActionCache.InternalActionResult[A1]]
 
       val WrapOutputName = "wrapOutput_\u2603\u2603"
       val WrapOutputDirectoryName = "wrapOutputDirectory_\u2603\u2603"
@@ -442,12 +457,16 @@ trait Cont:
                 val output = Output(
                   tpe = TypeRepr.of[a],
                   term = qual,
-                  name = freshName("o"),
-                  parent = Symbol.spliceOwner,
-                  outputType = OutputType.File
+                  outputType = OutputType.File,
                 )
                 outputBuf += output
-                if cacheConfigExprOpt.isDefined then output.toAssign(output.term)
+                if cacheConfigExprOpt.isDefined then
+                  '{
+                    ActionCache.registerOutput(
+                      ${ output.term.asExprOf[VirtualFile] },
+                      $outputAccRef,
+                    )
+                  }.asTerm
                 else oldTree
               case WrapOutputDirectoryName =>
                 val output = Output(
@@ -455,20 +474,21 @@ trait Cont:
                   // which contains hash.
                   tpe = TypeRepr.of[VirtualFile],
                   term = qual,
-                  name = freshName("o"),
-                  parent = Symbol.spliceOwner,
                   outputType = OutputType.Directory,
                 )
                 outputBuf += output
                 cacheConfigExprOpt match
                   case Some(cacheConfigExpr) =>
-                    output.toAssign('{
-                      ActionCache.packageDirectory(
-                        dir = ${ output.term.asExprOf[VirtualFileRef] },
-                        conv = $cacheConfigExpr.fileConverter,
-                        outputDirectory = $cacheConfigExpr.outputDirectory,
+                    '{
+                      ActionCache.registerOutput(
+                        ActionCache.packageDirectory(
+                          dir = ${ output.term.asExprOf[VirtualFileRef] },
+                          conv = $cacheConfigExpr.fileConverter,
+                          outputDirectory = $cacheConfigExpr.outputDirectory,
+                        ),
+                        $outputAccRef,
                       )
-                    }.asTerm)
+                    }.asTerm
                   case None => oldTree
               case _ =>
                 inputBuf += Input(

core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala

@@ -157,31 +157,10 @@ trait ContextUtil[C <: Quotes & scala.Singleton](val valStart: Int):
   final class Output(
       val tpe: TypeRepr,
       val term: Term,
-      val name: String,
-      val parent: Symbol,
       val outputType: OutputType,
   ):
     override def toString: String =
-      s"Output($tpe, $term, $name, $outputType)"
-    val placeholder: Symbol =
-      tpe.asType match
-        case '[a] =>
-          Symbol.newVal(
-            parent,
-            name,
-            tpe,
-            Flags.Mutable,
-            Symbol.noSymbol
-          )
-    def toVarDef: ValDef =
-      ValDef(placeholder, rhs = Some('{ null }.asTerm))
-    def toAssign(value: Term): Term =
-      Block(
-        Assign(toRef, value) :: Nil,
-        toRef
-      )
-    def toRef: Ref = Ref(placeholder)
-    def isFile: Boolean = outputType == OutputType.File
+      s"Output($tpe, $term, $outputType)"
   end Output
 
   def applyTuple(tupleTerm: Term, tpe: TypeRepr, idx: Int): Term =

notes/2.0.0/declare-output-in-loop.md

@@ -0,0 +1,13 @@
+### Every `Def.declareOutput` call registers, including inside loops
+
+The cached-task macro allocated one slot per syntactic `Def.declareOutput` (or
+`Def.declareOutputDirectory`) call site, so a call inside a loop or `.map` over a
+runtime-determined list of files overwrote the same slot on every iteration and
+only the last file was cached and restored. Declared outputs now accumulate per
+execution, so a dynamic number of outputs declared from one call site all
+survive a cache hit. A `declareOutput` in a conditional branch that is not taken
+no longer contributes a null entry to the task's outputs either.
+
+This addresses the loop half of [#9462][i9462].
+
+[i9462]: https://github.com/sbt/sbt/issues/9462

sbt-app/src/sbt-test/cache/declare-output-loop/build.sbt

@@ -0,0 +1,66 @@
+import sbt.internal.util.CacheEventSummary
+import xsbti.HashedVirtualFileRef
+
+val declareLoop = taskKey[Seq[HashedVirtualFileRef]]("declares 3 files via .map over a runtime list")
+val checkAll = taskKey[Unit]("asserts all 3 files exist")
+val delFiles = taskKey[Unit]("deletes the 3 files")
+val checkNone = taskKey[Unit]("asserts none of the 3 files exist")
+val checkHit = taskKey[Unit]("asserts previous command was a pure cache hit")
+
+Global / localCacheDirectory := baseDirectory.value / "diskcache"
+
+lazy val declareOutputLoop = project.in(file("."))
+
+declareLoop := {
+  val log = streams.value.log
+  val dir = target.value / "gen-multi"
+  IO.createDirectory(dir)
+  val files = List(dir / "a.txt", dir / "b.txt", dir / "c.txt")
+  IO.write(files(0), "AAA")
+  IO.write(files(1), "BBB")
+  IO.write(files(2), "CCC")
+  log.info(s"COMPUTED declareLoop (cache miss)")
+  if (sys.props.contains("never.set.property")) {
+    val ghost = fileConverter.value.toVirtualFile((dir / "never.txt").toPath)
+    val _ = Def.declareOutput(ghost)
+  }
+  files.map { f =>
+    val vf = fileConverter.value.toVirtualFile(f.toPath)
+    Def.declareOutput(vf)
+  }
+}
+
+def listing(dir: File): String =
+  if (dir.exists) (dir ** "*").get().mkString(", ") else "<dir missing>"
+
+checkAll := Def.uncached {
+  val dir = target.value / "gen-multi"
+  streams.value.log.info(s"gen-multi listing: ${listing(dir)}")
+  assert((dir / "a.txt").exists, s"a.txt missing under $dir")
+  assert((dir / "b.txt").exists, s"b.txt missing under $dir")
+  assert((dir / "c.txt").exists, s"c.txt missing under $dir")
+}
+
+delFiles := Def.uncached {
+  val dir = target.value / "gen-multi"
+  IO.delete(Seq(dir / "a.txt", dir / "b.txt", dir / "c.txt"))
+  streams.value.log.info(s"deleted files under $dir")
+}
+
+checkNone := Def.uncached {
+  val dir = target.value / "gen-multi"
+  assert(
+    !(dir / "a.txt").exists && !(dir / "b.txt").exists && !(dir / "c.txt").exists,
+    s"files still present under $dir"
+  )
+}
+
+checkHit := Def.uncached {
+  val config = Def.cacheConfiguration.value
+  val prev = config.cacheEventLog.previous match
+    case s: CacheEventSummary.Data => s
+    case _                         => sys.error("empty event log")
+  streams.value.log.info(s"prev hitCount=${prev.hitCount} missCount=${prev.missCount}")
+  assert(prev.missCount == 0, s"expected pure hit but missCount=${prev.missCount}")
+  assert(prev.hitCount >= 1, s"expected a hit but hitCount=${prev.hitCount}")
+}

sbt-app/src/sbt-test/cache/declare-output-loop/test

@@ -0,0 +1,10 @@
+# Regression for #9462: every execution of a Def.declareOutput call site must register,
+# including calls inside a .map over a runtime-determined list. The task also contains a
+# declareOutput in a never-taken branch, which must contribute nothing.
+> declareLoop
+> checkAll
+> delFiles
+> checkNone
+> declareLoop
+> checkHit
+> checkAll

util-cache/src/main/scala/sbt/util/ActionCache.scala

@@ -33,6 +33,7 @@ import sbt.nio.file.syntax.*
 import sbt.util.CacheImplicits
 import scala.reflect.ClassTag
 import scala.annotation.{ meta, StaticAnnotation }
+import scala.collection.mutable
 import scala.util.control.NonFatal
 import sjsonnew.{ HashWriter, JsonFormat }
 import sjsonnew.support.murmurhash.Hasher
@@ -374,6 +375,14 @@ object ActionCache:
           Files.move(staging, destZip, StandardCopyOption.REPLACE_EXISTING)
     finally Files.deleteIfExists(staging)
 
+  /** Appends a declared output; called from code generated by the cached-task macro. */
+  def registerOutput(
+      vf: VirtualFile,
+      outputs: mutable.ListBuffer[VirtualFile],
+  ): VirtualFile =
+    outputs += vf
+    vf
+
   def packageDirectory(
       dir: VirtualFileRef,
       conv: FileConverter,