Merge pull request #175 from scala/backport-lts-3.3-22303

Backport "Fix coverage serialization when encountering macro suspension " to 3.3 LTS

Author: tgodzik

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

@@ -40,6 +40,7 @@ import util.Store
 import plugins.*
 import java.util.concurrent.atomic.AtomicInteger
 import java.nio.file.InvalidPathException
+import dotty.tools.dotc.coverage.Coverage
 
 object Contexts {
 
@@ -958,6 +959,11 @@ object Contexts {
     val sources: util.HashMap[AbstractFile, SourceFile] = util.HashMap[AbstractFile, SourceFile]()
     val files: util.HashMap[TermName, AbstractFile] = util.HashMap()
 
+    /** If coverage option is used, it stores all instrumented statements (for InstrumentCoverage).
+     * We need this information to be persisted across different runs, so it's stored here.
+     */
+    private[dotc] var coverage: Coverage | Null = null
+
     // Types state
     /** A table for hash consing unique types */
     private[core] val uniques: Uniques = Uniques()

compiler/src/dotty/tools/dotc/coverage/Coverage.scala

@@ -2,15 +2,27 @@ package dotty.tools.dotc
 package coverage
 
 import scala.collection.mutable
+import java.nio.file.Path
 
 /** Holds a list of statements to include in the coverage reports. */
 class Coverage:
   private val statementsById = new mutable.LongMap[Statement](256)
 
+  private var statementId: Int = 0
+
+  def nextStatementId(): Int =
+    statementId += 1
+    statementId - 1
+
+
   def statements: Iterable[Statement] = statementsById.values
 
   def addStatement(stmt: Statement): Unit = statementsById(stmt.id) = stmt
 
+  def removeStatementsFromFile(sourcePath: Path) =
+    val removedIds = statements.filter(_.location.sourcePath == sourcePath).map(_.id.toLong)
+    removedIds.foreach(statementsById.remove)
+
 
 /**
   * A statement that can be invoked, and thus counted as "covered" by code coverage tools.

compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala

@@ -38,12 +38,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
   override def isEnabled(using ctx: Context) =
     ctx.settings.coverageOutputDir.value.nonEmpty
 
-  // counter to assign a unique id to each statement
-  private var statementId = 0
-
-  // stores all instrumented statements
-  private val coverage = Coverage()
-
   private var coverageExcludeClasslikePatterns: List[Pattern] = Nil
   private var coverageExcludeFilePatterns: List[Pattern] = Nil
 
@@ -61,12 +55,17 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         .foreach(_.nn.delete())
     end if
 
+    // Initialise a coverage object if it does not exist yet
+    if ctx.base.coverage == null then
+      ctx.base.coverage = Coverage()
+
     coverageExcludeClasslikePatterns = ctx.settings.coverageExcludeClasslikes.value.map(_.r.pattern)
     coverageExcludeFilePatterns = ctx.settings.coverageExcludeFiles.value.map(_.r.pattern)
 
+    ctx.base.coverage.nn.removeStatementsFromFile(ctx.compilationUnit.source.file.absolute.jpath)
     super.run
 
-    Serializer.serialize(coverage, outputPath, ctx.settings.sourceroot.value)
+    Serializer.serialize(ctx.base.coverage.nn, outputPath, ctx.settings.sourceroot.value)
 
   private def isClassIncluded(sym: Symbol)(using Context): Boolean =
     val fqn = sym.fullName.toText(ctx.printerFn(ctx)).show
@@ -110,8 +109,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
       * @return the statement's id
       */
     private def recordStatement(tree: Tree, pos: SourcePosition, branch: Boolean)(using ctx: Context): Int =
-      val id = statementId
-      statementId += 1
+      val id = ctx.base.coverage.nn.nextStatementId()
 
       val sourceFile = pos.source
       val statement = Statement(
@@ -127,7 +125,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         treeName = tree.getClass.getSimpleName.nn,
         branch
       )
-      coverage.addStatement(statement)
+      ctx.base.coverage.nn.addStatement(statement)
       id
 
     /**

compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala

@@ -45,15 +45,27 @@ class CoverageTests:
         lines
     end fixWindowsPaths
 
-    def runOnFile(p: Path): Boolean =
-      scalaFile.matches(p) &&
-      (Properties.testsFilter.isEmpty || Properties.testsFilter.exists(p.toString.contains))
+    def runOnFileOrDir(p: Path): Boolean =
+      (scalaFile.matches(p) || Files.isDirectory(p))
+      && (p != dir)
+      && (Properties.testsFilter.isEmpty || Properties.testsFilter.exists(p.toString.contains))
+
+    Files.walk(dir, 1).filter(runOnFileOrDir).forEach(path => {
+      // measurement files only exist in the "run" category
+      // as these are generated at runtime by the scala.runtime.coverage.Invoker
+      val (targetDir, expectFile, expectMeasurementFile) =
+        if Files.isDirectory(path) then
+          val dirName = path.getFileName().toString
+          assert(!Files.walk(path).filter(scalaFile.matches(_)).toArray.isEmpty, s"No scala files found in test directory: ${path}")
+          val targetDir = computeCoverageInTmp(path, isDirectory = true, dir, run)
+          (targetDir, path.resolve(s"test.scoverage.check"), path.resolve(s"test.measurement.check"))
+        else
+          val fileName = path.getFileName.toString.stripSuffix(".scala")
+          val targetDir = computeCoverageInTmp(path, isDirectory = false, dir, run)
+          (targetDir, path.resolveSibling(s"${fileName}.scoverage.check"), path.resolveSibling(s"${fileName}.measurement.check"))
 
-    Files.walk(dir).filter(runOnFile).forEach(path => {
-      val fileName = path.getFileName.toString.stripSuffix(".scala")
-      val targetDir = computeCoverageInTmp(path, dir, run)
       val targetFile = targetDir.resolve(s"scoverage.coverage")
-      val expectFile = path.resolveSibling(s"$fileName.scoverage.check")
+
       if updateCheckFiles then
         Files.copy(targetFile, expectFile, StandardCopyOption.REPLACE_EXISTING)
       else
@@ -63,9 +75,6 @@ class CoverageTests:
           val instructions = FileDiff.diffMessage(expectFile.toString, targetFile.toString)
           fail(s"Coverage report differs from expected data.\n$instructions")
 
-      // measurement files only exist in the "run" category
-      // as these are generated at runtime by the scala.runtime.coverage.Invoker
-      val expectMeasurementFile = path.resolveSibling(s"$fileName.measurement.check")
       if run && Files.exists(expectMeasurementFile) then
 
         // Note that this assumes that the test invoked was single threaded,
@@ -86,14 +95,20 @@ class CoverageTests:
     })
 
   /** Generates the coverage report for the given input file, in a temporary directory. */
-  def computeCoverageInTmp(inputFile: Path, sourceRoot: Path, run: Boolean)(using TestGroup): Path =
+  def computeCoverageInTmp(inputFile: Path, isDirectory: Boolean, sourceRoot: Path, run: Boolean)(using TestGroup): Path =
     val target = Files.createTempDirectory("coverage")
     val options = defaultOptions.and("-Ycheck:instrumentCoverage", "-coverage-out", target.toString, "-sourceroot", sourceRoot.toString)
     if run then
-      val test = compileDir(inputFile.getParent.toString, options)
+      val path = if isDirectory then inputFile.toString else inputFile.getParent.toString
+      val test = compileDir(path, options)
+      test.checkFilePaths.foreach { checkFilePath =>
+        assert(checkFilePath.exists, s"Expected checkfile for $path $checkFilePath does not exist.")
+      }
       test.checkRuns()
     else
-      val test = compileFile(inputFile.toString, options)
+      val test =
+        if isDirectory then compileDir(inputFile.toString, options)
+        else compileFile(inputFile.toString, options)
       test.checkCompile()
     target
 

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

@@ -250,15 +250,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
      * For a given test source, returns a check file against which the result of the test run
      * should be compared. Is used by implementations of this trait.
      */
-    final def checkFile(testSource: TestSource): Option[JFile] = (testSource match {
-      case ts: JointCompilationSource =>
-        ts.files.collectFirst {
-          case f if !f.isDirectory =>
-            new JFile(f.getPath.replaceFirst("\\.(scala|java)$", ".check"))
-        }
-      case ts: SeparateCompilationSource =>
-        Option(new JFile(ts.dir.getPath + ".check"))
-    }).filter(_.exists)
+    final def checkFile(testSource: TestSource): Option[JFile] = (CompilationLogic.checkFilePath(testSource)).filter(_.exists)
 
     /**
      * Checks if the given actual lines are the same as the ones in the check file.
@@ -335,6 +327,18 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     }
   }
 
+  object CompilationLogic {
+    private[ParallelTesting] def checkFilePath(testSource: TestSource) = testSource match {
+      case ts: JointCompilationSource =>
+        ts.files.collectFirst {
+          case f if !f.isDirectory =>
+            new JFile(f.getPath.replaceFirst("\\.(scala|java)$", ".check"))
+        }
+      case ts: SeparateCompilationSource =>
+        Option(new JFile(ts.dir.getPath + ".check"))
+    }
+  }
+
   /** Each `Test` takes the `testSources` and performs the compilation and assertions
    *  according to the implementing class "neg", "run" or "pos".
    */
@@ -1087,6 +1091,8 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     def this(targets: List[TestSource]) =
       this(targets, 1, true, None, false, false)
 
+    def checkFilePaths: List[JFile] = targets.map(CompilationLogic.checkFilePath).flatten
+
     def copy(targets: List[TestSource],
       times: Int = times,
       shouldDelete: Boolean = shouldDelete,

tests/coverage/pos/macro-late-suspend/Test.scala

@@ -0,0 +1,13 @@
+package example
+
+sealed trait Test
+
+object Test {
+  case object Foo extends Test
+
+  val visitorType = mkVisitorType[Test]
+
+  trait Visitor[A] {
+    type V[a] = visitorType.Out[a]
+  }
+}

tests/coverage/pos/macro-late-suspend/UsesTest.scala

@@ -0,0 +1,3 @@
+package example
+
+val _ = Test.Foo

tests/coverage/pos/macro-late-suspend/VisitorMacros.scala

@@ -0,0 +1,12 @@
+package example
+
+import scala.quoted.*
+
+private def mkVisitorTypeImpl[T: Type](using q: Quotes): Expr[VisitorType[T]] =
+  '{new VisitorType[T]{}}
+
+transparent inline def mkVisitorType[T]: VisitorType[T] = ${ mkVisitorTypeImpl[T] }
+
+trait VisitorType[T] {
+  type Out[A]
+}

tests/coverage/pos/macro-late-suspend/test.scoverage.check

@@ -0,0 +1,88 @@
+# Coverage data, format version: 3.0
+# Statement data:
+# - id
+# - source path
+# - package name
+# - class name
+# - class type (Class, Object or Trait)
+# - full class name
+# - method name
+# - start offset
+# - end offset
+# - line number
+# - symbol name
+# - tree name
+# - is branch
+# - invocations count
+# - is ignored
+# - description (can be multi-line)
+# '' sign
+# ------------------------------------------
+1
+macro-late-suspend/VisitorMacros.scala
+example
+VisitorMacros$package
+Object
+example.VisitorMacros$package
+mkVisitorTypeImpl
+124
+144
+6
+unpickleExprV2
+Apply
+false
+0
+false
+new VisitorType[T]{}
+
+2
+macro-late-suspend/VisitorMacros.scala
+example
+VisitorMacros$package
+Object
+example.VisitorMacros$package
+mkVisitorTypeImpl
+40
+69
+5
+mkVisitorTypeImpl
+DefDef
+false
+0
+false
+private def mkVisitorTypeImpl
+
+3
+macro-late-suspend/Test.scala
+example
+Test
+Object
+example.Test
+<init>
+102
+121
+8
+<init>
+Apply
+false
+0
+false
+mkVisitorType[Test]
+
+4
+macro-late-suspend/UsesTest.scala
+example
+UsesTest$package
+Object
+example.UsesTest$package
+<init>
+22
+22
+3
+<none>
+Literal
+true
+0
+false
+
+

tests/coverage/run/i16940/i16940.check

@@ -0,0 +1 @@
+2