Fix coverage for macro suspension after InstrumentCoverage

Author: jchyb

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

@@ -41,6 +41,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 {
 
@@ -982,10 +983,11 @@ object Contexts {
     /** Was best effort file used during compilation? */
     private[core] var usedBestEffortTasty = false
 
-    /** Counter to assign a unique id to each statement (for coverage) */
-    private[dotc] var coverageStatementId = 0
-    /** A variable denoting if the coverage file already written to (for coverage) */
-    private[dotc] var coverageStartedWriting = false
+    /** 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 = Coverage()
 
     // Types state
     /** A table for hash consing unique types */

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/coverage/Serializer.scala

@@ -1,11 +1,10 @@
 package dotty.tools.dotc
 package coverage
 
-import java.nio.file.{Path, Paths, Files, StandardOpenOption}
+import java.nio.file.{Path, Paths, Files}
 import java.io.Writer
 import scala.language.unsafeNulls
 import scala.collection.mutable.StringBuilder
-import dotty.tools.dotc.core.Contexts.Context
 
 /**
  * Serializes scoverage data.
@@ -17,20 +16,20 @@ object Serializer:
   private val CoverageDataFormatVersion = "3.0"
 
   /** Write out coverage data to the given data directory, using the default coverage filename */
-  def serialize(coverage: Coverage, dataDir: String, sourceRoot: String)(using Context): Unit =
+  def serialize(coverage: Coverage, dataDir: String, sourceRoot: String): Unit =
     serialize(coverage, Paths.get(dataDir, CoverageFileName).toAbsolutePath, Paths.get(sourceRoot).toAbsolutePath)
 
   /** Write out coverage data to a file. */
-  def serialize(coverage: Coverage, file: Path, sourceRoot: Path)(using Context): Unit =
-    val writer = Files.newBufferedWriter(file, StandardOpenOption.CREATE, StandardOpenOption.APPEND)
+  def serialize(coverage: Coverage, file: Path, sourceRoot: Path): Unit =
+    val writer = Files.newBufferedWriter(file)
     try
       serialize(coverage, writer, sourceRoot)
     finally
       writer.close()
 
   /** Write out coverage data (info about each statement that can be covered) to a writer.
    */
-  def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path)(using ctx: Context): Unit =
+  def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path): Unit =
 
     def getRelativePath(filePath: Path): String =
       // We need to normalize the path here because the relativizing paths containing '.' or '..' differs between Java versions
@@ -82,8 +81,7 @@ object Serializer:
                       |\f
                       |""".stripMargin)
 
-    if (!ctx.base.coverageStartedWriting) writeHeader(writer)
-    ctx.base.coverageStartedWriting = true
+    writeHeader(writer)
     coverage.statements.toSeq
       .sortBy(_.id)
       .foreach(stmt => writeStatement(stmt, writer))

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

@@ -38,9 +38,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
   override def isEnabled(using ctx: Context) =
     ctx.settings.coverageOutputDir.value.nonEmpty
 
-  // stores all instrumented statements
-  private val coverage = Coverage()
-
   private var coverageExcludeClasslikePatterns: List[Pattern] = Nil
   private var coverageExcludeFilePatterns: List[Pattern] = Nil
 
@@ -51,7 +48,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
     val dataDir = File(outputPath)
     val newlyCreated = dataDir.mkdirs()
 
-    if !newlyCreated && !ctx.base.coverageStartedWriting then
+    if !newlyCreated then
       // If the directory existed before, let's clean it up.
       dataDir.listFiles.nn
         .filter(_.nn.getName.nn.startsWith("scoverage"))
@@ -61,9 +58,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
     coverageExcludeClasslikePatterns = ctx.settings.coverageExcludeClasslikes.value.map(_.r.pattern)
     coverageExcludeFilePatterns = ctx.settings.coverageExcludeFiles.value.map(_.r.pattern)
 
+    ctx.base.coverage.removeStatementsFromFile(ctx.compilationUnit.source.file.absolute.jpath)
     super.run
 
-    Serializer.serialize(coverage, outputPath, ctx.settings.sourceroot.value)
+    Serializer.serialize(ctx.base.coverage, outputPath, ctx.settings.sourceroot.value)
 
   private def isClassIncluded(sym: Symbol)(using Context): Boolean =
     val fqn = sym.fullName.toText(ctx.printerFn(ctx)).show
@@ -107,8 +105,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 = ctx.base.coverageStatementId
-      ctx.base.coverageStatementId += 1
+      val id = ctx.base.coverage.nextStatementId()
 
       val sourceFile = pos.source
       val statement = Statement(
@@ -124,7 +121,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         treeName = tree.getClass.getSimpleName.nn,
         branch
       )
-      coverage.addStatement(statement)
+      ctx.base.coverage.addStatement(statement)
       id
 
     /**

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

@@ -65,7 +65,7 @@ class CoverageTests:
       val (targetDir, expectFile, expectMeasurementFile) =
         if Files.isDirectory(path) then
           val dirName = path.getFileName().toString
-          assert(!Files.walk(path).filter(scalaFile.matches(_)).toList.isEmpty, s"No scala files found in test directory: ${path}")
+          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

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
+
+