Skip to content

Commit da43ab4

Browse files
committed
Make coverage more similar to Scala 2
* remove coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * add coverage for Literals (ones directly in Apply are omitted) * remove coverage of `throw` contents * if apply node is tagged, do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allow instrumenting synthetic method calls (like apply of a case class) Also fixes issue with certain generated Block nodes not having assigned the correct type
1 parent 55ab8d0 commit da43ab4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+2817
-1202
lines changed

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

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,29 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
145145
val span = pos.span.toSynthetic
146146
invokeCall(statementId, span)
147147

148+
private def transformApplyArgs(trees: List[Tree])(using Context): List[Tree] =
149+
if (allConstArgs(trees)) trees else transform(trees)
150+
151+
private def transformInnerApply(tree: Tree)(using Context): Tree = tree match
152+
case a: Apply if a.fun.symbol == defn.StringContextModule_apply =>
153+
a
154+
case a: Apply =>
155+
cpy.Apply(a)(
156+
transformInnerApply(a.fun),
157+
transformApplyArgs(a.args)
158+
)
159+
case a: TypeApply =>
160+
cpy.TypeApply(a)(
161+
transformInnerApply(a.fun),
162+
transformApplyArgs(a.args)
163+
)
164+
case s: Select =>
165+
cpy.Select(s)(transformInnerApply(s.qualifier), s.name)
166+
case i: (Ident | This) => i
167+
case other => transform(other)
168+
169+
private def allConstArgs(args: List[Tree]) =
170+
args.forall(arg => arg.isInstanceOf[Literal] || arg.isInstanceOf[Ident])
148171
/**
149172
* Tries to instrument an `Apply`.
150173
* These "tryInstrument" methods are useful to tweak the generation of coverage instrumentation,
@@ -158,10 +181,12 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
158181
// Create a call to Invoker.invoked(coverageDirectory, newStatementId)
159182
val coverageCall = createInvokeCall(tree, tree.sourcePos)
160183

161-
if needsLift(tree) then
162-
// Transform args and fun, i.e. instrument them if needed (and if possible)
163-
val app = cpy.Apply(tree)(transform(tree.fun), tree.args.map(transform))
184+
// Transform args and fun, i.e. instrument them if needed (and if possible)
185+
val app =
186+
if (tree.fun.symbol eq defn.throwMethod) tree
187+
else cpy.Apply(tree)(transformInnerApply(tree.fun), transformApplyArgs(tree.args))
164188

189+
if needsLift(tree) then
165190
// Lifts the arguments. Note that if only one argument needs to be lifted, we lift them all.
166191
// Also, tree.fun can be lifted too.
167192
// See LiftCoverage for the internal working of this lifting.
@@ -171,11 +196,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
171196
InstrumentedParts(liftedDefs.toList, coverageCall, liftedApp)
172197
else
173198
// Instrument without lifting
174-
val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
175-
InstrumentedParts.singleExpr(coverageCall, transformed)
199+
InstrumentedParts.singleExpr(coverageCall, app)
176200
else
177201
// Transform recursively but don't instrument the tree itself
178-
val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
202+
val transformed = cpy.Apply(tree)(transformInnerApply(tree.fun), transform(tree.args))
179203
InstrumentedParts.notCovered(transformed)
180204

181205
private def tryInstrument(tree: Ident)(using Context): InstrumentedParts =
@@ -187,9 +211,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
187211
else
188212
InstrumentedParts.notCovered(tree)
189213

214+
private def tryInstrument(tree: Literal)(using Context): InstrumentedParts =
215+
val coverageCall = createInvokeCall(tree, tree.sourcePos)
216+
InstrumentedParts.singleExpr(coverageCall, tree)
217+
190218
private def tryInstrument(tree: Select)(using Context): InstrumentedParts =
191219
val sym = tree.symbol
192-
val transformed = cpy.Select(tree)(transform(tree.qualifier), tree.name)
220+
val qual = transform(tree.qualifier).ensureConforms(tree.qualifier.tpe)
221+
val transformed = cpy.Select(tree)(qual, tree.name)
193222
if canInstrumentParameterless(sym) then
194223
// call to a parameterless method
195224
val coverageCall = createInvokeCall(tree, tree.sourcePos)
@@ -202,6 +231,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
202231
tree match
203232
case t: Apply => tryInstrument(t)
204233
case t: Ident => tryInstrument(t)
234+
case t: Literal => tryInstrument(t)
205235
case t: Select => tryInstrument(t)
206236
case _ => InstrumentedParts.notCovered(transform(tree))
207237

@@ -223,10 +253,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
223253
inContext(transformCtx(tree)) { // necessary to position inlined code properly
224254
tree match
225255
// simple cases
226-
case tree: (Import | Export | Literal | This | Super | New) => tree
256+
case tree: (Import | Export | This | Super | New) => tree
227257
case tree if tree.isEmpty || tree.isType => tree // empty Thicket, Ident (referring to a type), TypeTree, ...
228258
case tree if !tree.span.exists || tree.span.isZeroExtent => tree // no meaningful position
229259

260+
case tree: Literal =>
261+
val rest = tryInstrument(tree).toTree
262+
rest
263+
230264
// identifier
231265
case tree: Ident =>
232266
tryInstrument(tree).toTree
@@ -280,6 +314,9 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
280314
case tree: CaseDef =>
281315
transformCaseDef(tree)
282316

317+
case tree: ValDef if tree.symbol.is(Inline) =>
318+
tree // transforming inline vals will result in `inline value must be pure` errors
319+
283320
case tree: ValDef =>
284321
// only transform the rhs
285322
val rhs = transform(tree.rhs)
@@ -323,13 +360,13 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
323360
)
324361

325362
case tree: Inlined =>
326-
// Ideally, tree.call would provide precise information about the inlined call,
327-
// and we would use this information for the coverage report.
328-
// But PostTyper simplifies tree.call, so we can't report the actual method that was inlined.
329-
// In any case, the subtrees need to be repositioned right now, otherwise the
330-
// coverage statement will point to a potentially unreachable source file.
331-
val dropped = Inlines.dropInlined(tree) // drop and reposition
332-
transform(dropped) // transform the content of the Inlined
363+
// Inlined code contents might come from another file (or project),
364+
// which means that we cannot clearly designate which part of the inlined code
365+
// was run using the API we are given.
366+
// At best, we can show that the Inlined tree itself was reached.
367+
// Additionally, Scala 2's coverage ignores macro calls entirely,
368+
// so let's do that here too, also for regular inlined calls.
369+
tree
333370

334371
// For everything else just recurse and transform
335372
case _ =>
@@ -559,15 +596,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
559596
private def isCompilerIntrinsicMethod(sym: Symbol)(using Context): Boolean =
560597
val owner = sym.maybeOwner
561598
owner.exists && (
562-
owner.eq(defn.AnyClass) ||
563-
owner.isPrimitiveValueClass ||
599+
(owner.eq(defn.AnyClass) && (sym == defn.Any_asInstanceOf || sym == defn.Any_isInstanceOf)) ||
564600
owner.maybeOwner == defn.CompiletimePackageClass
565601
)
566602

567603
object InstrumentCoverage:
568604
val name: String = "instrumentCoverage"
569605
val description: String = "instrument code for coverage checking"
570-
val ExcludeMethodFlags: FlagSet = Synthetic | Artifact | Erased
606+
val ExcludeMethodFlags: FlagSet = Artifact | Erased
571607

572608
/**
573609
* An instrumented Tree, in 3 parts.

tests/coverage/pos/Constructor.scoverage.check

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,23 @@ C
110110
Class
111111
covtest.C
112112
x
113+
161
114+
162
115+
13
116+
<none>
117+
Literal
118+
false
119+
0
120+
false
121+
1
122+
123+
6
124+
Constructor.scala
125+
covtest
126+
C
127+
Class
128+
covtest.C
129+
x
113130
153
114131
158
115132
13
@@ -120,7 +137,7 @@ false
120137
false
121138
def x
122139

123-
6
140+
7
124141
Constructor.scala
125142
covtest
126143
C
@@ -137,7 +154,7 @@ false
137154
false
138155
f(x)
139156

140-
7
157+
8
141158
Constructor.scala
142159
covtest
143160
C
@@ -154,7 +171,24 @@ false
154171
false
155172
x
156173

157-
8
174+
9
175+
Constructor.scala
176+
covtest
177+
C
178+
Class
179+
covtest.C
180+
g
181+
188
182+
189
183+
16
184+
<none>
185+
Literal
186+
false
187+
0
188+
false
189+
2
190+
191+
10
158192
Constructor.scala
159193
covtest
160194
C
@@ -171,7 +205,7 @@ false
171205
false
172206
def g
173207

174-
9
208+
11
175209
Constructor.scala
176210
covtest
177211
O
@@ -188,7 +222,24 @@ false
188222
false
189223
def g
190224

191-
10
225+
12
226+
Constructor.scala
227+
covtest
228+
O
229+
Object
230+
covtest.O
231+
y
232+
231
233+
232
234+
20
235+
<none>
236+
Literal
237+
false
238+
0
239+
false
240+
1
241+
242+
13
192243
Constructor.scala
193244
covtest
194245
O
@@ -205,7 +256,7 @@ false
205256
false
206257
def y
207258

208-
11
259+
14
209260
Constructor.scala
210261
covtest
211262
O
@@ -222,20 +273,3 @@ false
222273
false
223274
g(y)
224275

225-
12
226-
Constructor.scala
227-
covtest
228-
O
229-
Object
230-
covtest.O
231-
<init>
232-
237
233-
238
234-
21
235-
y
236-
Ident
237-
false
238-
0
239-
false
240-
y
241-

tests/coverage/pos/ContextFunctions.scoverage.check

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,23 @@ covtest
4141
Imperative
4242
Class
4343
covtest.Imperative
44+
$anonfun
45+
178
46+
184
47+
9
48+
<none>
49+
Literal
50+
false
51+
0
52+
false
53+
"name"
54+
55+
2
56+
ContextFunctions.scala
57+
covtest
58+
Imperative
59+
Class
60+
covtest.Imperative
4461
readName2
4562
121
4663
134
@@ -52,41 +69,41 @@ false
5269
false
5370
def readName2
5471

55-
2
72+
3
5673
ContextFunctions.scala
5774
covtest
5875
Imperative
5976
Class
6077
covtest.Imperative
6178
readPerson
62-
252
63-
309
64-
14
65-
onError
66-
Apply
79+
243
80+
247
81+
13
82+
<none>
83+
Literal
6784
false
6885
0
6986
false
70-
OnError((e) => readName2(using e)(using s)).onError(None)
87+
null
7188

72-
3
89+
4
7390
ContextFunctions.scala
7491
covtest
7592
Imperative
7693
Class
7794
covtest.Imperative
7895
readPerson
7996
252
80-
295
97+
309
8198
14
82-
<init>
99+
onError
83100
Apply
84101
false
85102
0
86103
false
87-
OnError((e) => readName2(using e)(using s))
104+
OnError((e) => readName2(using e)(using s)).onError(None)
88105

89-
4
106+
5
90107
ContextFunctions.scala
91108
covtest
92109
Imperative
@@ -103,7 +120,7 @@ false
103120
false
104121
readName2(using e)(using s)
105122

106-
5
123+
6
107124
ContextFunctions.scala
108125
covtest
109126
Imperative

0 commit comments

Comments
 (0)