Merge pull request #569 from scala/backport-lts-3.3-23722

Backport "Make coverage more similar to the one in Scala 2" to 3.3 LTS

Author: tgodzik

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

@@ -145,6 +145,31 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
       val span = pos.span.toSynthetic
       invokeCall(statementId, span)
 
+    private def transformApplyArgs(trees: List[Tree])(using Context): List[Tree] =
+      if allConstArgs(trees) then trees else transform(trees)
+
+    private def transformInnerApply(tree: Tree)(using Context): Tree = tree match
+      case a: Apply if a.fun.symbol == defn.StringContextModule_apply =>
+        a
+      case a: Apply =>
+        cpy.Apply(a)(
+          transformInnerApply(a.fun),
+          transformApplyArgs(a.args)
+        )
+      case a: TypeApply =>
+        cpy.TypeApply(a)(
+          transformInnerApply(a.fun),
+          transformApplyArgs(a.args)
+        )
+      case s: Select =>
+        cpy.Select(s)(transformInnerApply(s.qualifier), s.name)
+      case i: (Ident | This) => i
+      case t: Typed =>
+        cpy.Typed(t)(transformInnerApply(t.expr), t.tpt)
+      case other    => transform(other)
+
+    private def allConstArgs(args: List[Tree]) =
+      args.forall(arg => arg.isInstanceOf[Literal] || arg.isInstanceOf[Ident])
     /**
       * Tries to instrument an `Apply`.
       * These "tryInstrument" methods are useful to tweak the generation of coverage instrumentation,
@@ -158,10 +183,12 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         // Create a call to Invoker.invoked(coverageDirectory, newStatementId)
         val coverageCall = createInvokeCall(tree, tree.sourcePos)
 
-        if needsLift(tree) then
-          // Transform args and fun, i.e. instrument them if needed (and if possible)
-          val app = cpy.Apply(tree)(transform(tree.fun), tree.args.map(transform))
+        // Transform args and fun, i.e. instrument them if needed (and if possible)
+        val app =
+          if tree.fun.symbol eq defn.throwMethod then tree
+          else cpy.Apply(tree)(transformInnerApply(tree.fun), transformApplyArgs(tree.args))
 
+        if needsLift(tree) then
           // Lifts the arguments. Note that if only one argument needs to be lifted, we lift them all.
           // Also, tree.fun can be lifted too.
           // See LiftCoverage for the internal working of this lifting.
@@ -171,11 +198,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
           InstrumentedParts(liftedDefs.toList, coverageCall, liftedApp)
         else
           // Instrument without lifting
-          val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
-          InstrumentedParts.singleExpr(coverageCall, transformed)
+          InstrumentedParts.singleExpr(coverageCall, app)
       else
         // Transform recursively but don't instrument the tree itself
-        val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
+        val transformed = cpy.Apply(tree)(transformInnerApply(tree.fun), transform(tree.args))
         InstrumentedParts.notCovered(transformed)
 
     private def tryInstrument(tree: Ident)(using Context): InstrumentedParts =
@@ -187,9 +213,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
       else
         InstrumentedParts.notCovered(tree)
 
+    private def tryInstrument(tree: Literal)(using Context): InstrumentedParts =
+      val coverageCall = createInvokeCall(tree, tree.sourcePos)
+      InstrumentedParts.singleExpr(coverageCall, tree)
+
     private def tryInstrument(tree: Select)(using Context): InstrumentedParts =
       val sym = tree.symbol
-      val transformed = cpy.Select(tree)(transform(tree.qualifier), tree.name)
+      val qual = transform(tree.qualifier).ensureConforms(tree.qualifier.tpe)
+      val transformed = cpy.Select(tree)(qual, tree.name)
       if canInstrumentParameterless(sym) then
         // call to a parameterless method
         val coverageCall = createInvokeCall(tree, tree.sourcePos)
@@ -202,6 +233,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
       tree match
         case t: Apply => tryInstrument(t)
         case t: Ident => tryInstrument(t)
+        case t: Literal => tryInstrument(t)
         case t: Select => tryInstrument(t)
         case _ => InstrumentedParts.notCovered(transform(tree))
 
@@ -223,10 +255,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
       inContext(transformCtx(tree)) { // necessary to position inlined code properly
         tree match
           // simple cases
-          case tree: (Import | Export | Literal | This | Super | New) => tree
+          case tree: (Import | Export | This | Super | New) => tree
           case tree if tree.isEmpty || tree.isType => tree // empty Thicket, Ident (referring to a type), TypeTree, ...
           case tree if !tree.span.exists || tree.span.isZeroExtent => tree // no meaningful position
 
+          case tree: Literal =>
+            val rest = tryInstrument(tree).toTree
+            rest
+
           // identifier
           case tree: Ident =>
             tryInstrument(tree).toTree
@@ -280,6 +316,9 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
           case tree: CaseDef =>
             transformCaseDef(tree)
 
+          case tree: ValDef if tree.symbol.is(Inline) =>
+            tree // transforming inline vals will result in `inline value must be pure` errors
+
           case tree: ValDef =>
             // only transform the rhs
             val rhs = transform(tree.rhs)
@@ -323,13 +362,13 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
             )
 
           case tree: Inlined =>
-            // Ideally, tree.call would provide precise information about the inlined call,
-            // and we would use this information for the coverage report.
-            // But PostTyper simplifies tree.call, so we can't report the actual method that was inlined.
-            // In any case, the subtrees need to be repositioned right now, otherwise the
-            // coverage statement will point to a potentially unreachable source file.
-            val dropped = Inlines.dropInlined(tree) // drop and reposition
-            transform(dropped) // transform the content of the Inlined
+            // Inlined code contents might come from another file (or project),
+            // which means that we cannot clearly designate which part of the inlined code
+            // was run using the API we are given.
+            // At best, we can show that the Inlined tree itself was reached.
+            // Additionally, Scala 2's coverage ignores macro calls entirely,
+            // so let's do that here too, also for regular inlined calls.
+            tree
 
           // For everything else just recurse and transform
           case _ =>
@@ -559,15 +598,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
     private def isCompilerIntrinsicMethod(sym: Symbol)(using Context): Boolean =
       val owner = sym.maybeOwner
       owner.exists && (
-        owner.eq(defn.AnyClass) ||
-        owner.isPrimitiveValueClass ||
+        (owner.eq(defn.AnyClass) && (sym == defn.Any_asInstanceOf || sym == defn.Any_isInstanceOf)) ||
         owner.maybeOwner == defn.CompiletimePackageClass
       )
 
 object InstrumentCoverage:
   val name: String = "instrumentCoverage"
   val description: String = "instrument code for coverage checking"
-  val ExcludeMethodFlags: FlagSet = Synthetic | Artifact | Erased
+  val ExcludeMethodFlags: FlagSet = Artifact | Erased
 
   /**
    * An instrumented Tree, in 3 parts.

presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala

@@ -205,11 +205,11 @@ class Completions(
             case (ident: Ident) :: (app@Apply(_, List(arg))) :: _  =>
               app.symbol.info match
                 case mt@MethodType(termNames) if app.symbol.paramSymss.last.exists(_.is(Given)) &&
-                  !text.substring(app.fun.span.start, arg.span.end).contains("using") =>
+                  !text.substring(app.fun.span.start, arg.span.end).nn.contains("using") =>
                   suffix.withNewPrefix(Affix(PrefixKind.Using))
-                case _ => suffix    
+                case _ => suffix
             case _ => suffix
-        
+
       }
       .chain { suffix => // for () suffix
         if shouldAddSuffix && symbol.is(Flags.Method) then

tests/coverage/pos/Constructor.scoverage.check

@@ -110,6 +110,23 @@ C
 Class
 covtest.C
 x
+161
+162
+13
+<none>
+Literal
+false
+0
+false
+1
+
+6
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+x
 153
 158
 13
@@ -120,7 +137,7 @@ false
 false
 def x
 
-6
+7
 Constructor.scala
 covtest
 C
@@ -137,7 +154,7 @@ false
 false
 f(x)
 
-7
+8
 Constructor.scala
 covtest
 C
@@ -154,7 +171,24 @@ false
 false
 x
 
-8
+9
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+g
+188
+189
+16
+<none>
+Literal
+false
+0
+false
+2
+
+10
 Constructor.scala
 covtest
 C
@@ -171,7 +205,7 @@ false
 false
 def g
 
-9
+11
 Constructor.scala
 covtest
 O
@@ -188,7 +222,24 @@ false
 false
 def g
 
-10
+12
+Constructor.scala
+covtest
+O
+Object
+covtest.O
+y
+231
+232
+20
+<none>
+Literal
+false
+0
+false
+1
+
+13
 Constructor.scala
 covtest
 O
@@ -205,7 +256,7 @@ false
 false
 def y
 
-11
+14
 Constructor.scala
 covtest
 O
@@ -222,20 +273,3 @@ false
 false
 g(y)
 
-12
-Constructor.scala
-covtest
-O
-Object
-covtest.O
-<init>
-237
-238
-21
-y
-Ident
-false
-0
-false
-y
-

tests/coverage/pos/ContextFunctions.scoverage.check

@@ -41,6 +41,23 @@ covtest
 Imperative
 Class
 covtest.Imperative
+$anonfun
+178
+184
+9
+<none>
+Literal
+false
+0
+false
+"name"
+
+2
+ContextFunctions.scala
+covtest
+Imperative
+Class
+covtest.Imperative
 readName2
 121
 134
@@ -52,41 +69,41 @@ false
 false
 def readName2
 
-2
+3
 ContextFunctions.scala
 covtest
 Imperative
 Class
 covtest.Imperative
 readPerson
-252
-309
-14
-onError
-Apply
+243
+247
+13
+<none>
+Literal
 false
 0
 false
-OnError((e) => readName2(using e)(using s)).onError(None)
+null
 
-3
+4
 ContextFunctions.scala
 covtest
 Imperative
 Class
 covtest.Imperative
 readPerson
 252
-295
+309
 14
-<init>
+onError
 Apply
 false
 0
 false
-OnError((e) => readName2(using e)(using s))
+OnError((e) => readName2(using e)(using s)).onError(None)
 
-4
+5
 ContextFunctions.scala
 covtest
 Imperative
@@ -103,7 +120,7 @@ false
 false
 readName2(using e)(using s)
 
-5
+6
 ContextFunctions.scala
 covtest
 Imperative