Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: scoverage statement's line number should be 1-base #18932

Merged
merged 1 commit into from
Nov 17, 2023
Merged

fix: scoverage statement's line number should be 1-base #18932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/src/dotty/tools/dotc/coverage/Coverage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ class Coverage:

def addStatement(stmt: Statement): Unit = statementsById(stmt.id) = stmt

/** A statement that can be invoked, and thus counted as "covered" by code coverage tools. */

/**
* A statement that can be invoked, and thus counted as "covered" by code coverage tools.
*
* @param line 1-indexed line number
*/
case class Statement(
location: Location,
id: Int,
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
id = id,
start = pos.start,
end = pos.end,
line = pos.line,
// +1 to account for the line number starting at 1
// the internal line number is 0-base https://github.com/lampepfl/dotty/blob/18ada516a85532524a39a962b2ddecb243c65376/compiler/src/dotty/tools/dotc/util/SourceFile.scala#L173-L176
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add parameter documention in Statement.

https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/coverage/Coverage.scala#L14-L26

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! do you mean, the parameter document that says line in Statement is 1-base ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean add something like this

/** A statement that can be invoked, and thus counted as "covered" by code coverage tools. 
 *
 *  @param location ...
 *  ...
 *  @param line 1-indexed line number
 *  ...
 */
case class Statement(
    location: Location,
    id: Int,
    start: Int,
    end: Int,
    line: Int,
    ...

line = pos.line + 1,
desc = sourceFile.content.slice(pos.start, pos.end).mkString,
symbolName = tree.symbol.name.toSimpleName.toString,
treeName = tree.getClass.getSimpleName.nn,
Expand Down
26 changes: 13 additions & 13 deletions tests/coverage/pos/Constructor.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ covtest.C
<init>
28
36
3
4
<init>
DefDef
false
Expand All @@ -44,7 +44,7 @@ covtest.C
<init>
69
72
5
6
g
Apply
false
Expand All @@ -61,7 +61,7 @@ covtest.C
<init>
80
88
8
9
<init>
DefDef
false
Expand All @@ -78,7 +78,7 @@ covtest.C
<init>
108
128
9
10
+
Apply
false
Expand All @@ -95,7 +95,7 @@ covtest.C
f
133
138
11
12
f
DefDef
false
Expand All @@ -112,7 +112,7 @@ covtest.C
x
153
158
12
13
x
DefDef
false
Expand All @@ -129,7 +129,7 @@ covtest.C
<init>
165
169
13
14
f
Apply
false
Expand All @@ -146,7 +146,7 @@ covtest.C
<init>
167
168
13
14
x
Select
false
Expand All @@ -163,7 +163,7 @@ covtest.C
g
173
178
15
16
g
DefDef
false
Expand All @@ -180,7 +180,7 @@ covtest.O$
g
203
208
18
19
g
DefDef
false
Expand All @@ -197,7 +197,7 @@ covtest.O$
y
223
228
19
20
y
DefDef
false
Expand All @@ -214,7 +214,7 @@ covtest.O$
<init>
235
239
20
21
g
Apply
false
Expand All @@ -231,7 +231,7 @@ covtest.O$
<init>
237
238
20
21
y
Ident
false
Expand Down
12 changes: 6 additions & 6 deletions tests/coverage/pos/ContextFunctions.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ covtest.OnError
onError
56
67
3
4
onError
DefDef
false
Expand All @@ -44,7 +44,7 @@ covtest.Imperative
readName2
121
134
7
8
readName2
DefDef
false
Expand All @@ -61,7 +61,7 @@ covtest.Imperative
readPerson
252
309
13
14
onError
Apply
false
Expand All @@ -78,7 +78,7 @@ covtest.Imperative
readPerson
252
295
13
14
<init>
Apply
false
Expand All @@ -95,7 +95,7 @@ covtest.Imperative
$anonfun
267
294
13
14
apply
Apply
false
Expand All @@ -112,7 +112,7 @@ covtest.Imperative
readPerson
192
206
11
12
readPerson
DefDef
false
Expand Down
34 changes: 17 additions & 17 deletions tests/coverage/pos/Enum.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ covtest.Planet
surfaceGravity
338
356
14
15
surfaceGravity
DefDef
false
Expand All @@ -44,7 +44,7 @@ covtest.Planet
surfaceWeight
444
458
15
16
surfaceGravity
Select
false
Expand All @@ -61,7 +61,7 @@ covtest.Planet
surfaceWeight
392
409
15
16
surfaceWeight
DefDef
false
Expand All @@ -78,7 +78,7 @@ covtest.EnumTypes$
test
1043
1077
30
31
println
Apply
false
Expand All @@ -95,7 +95,7 @@ covtest.EnumTypes$
test
1051
1076
30
31
+
Apply
false
Expand All @@ -112,7 +112,7 @@ covtest.EnumTypes$
test
1082
1103
31
32
println
Apply
false
Expand All @@ -129,7 +129,7 @@ covtest.EnumTypes$
test
1090
1102
31
32
s
Apply
false
Expand All @@ -146,7 +146,7 @@ covtest.EnumTypes$
calculateEarthWeightOnPlanets
1195
1222
34
35
surfaceGravity
Select
false
Expand All @@ -163,7 +163,7 @@ covtest.EnumTypes$
calculateEarthWeightOnPlanets
1229
1320
35
36
foreach
Apply
false
Expand All @@ -180,7 +180,7 @@ covtest.EnumTypes$
calculateEarthWeightOnPlanets
1238
1251
35
36
refArrayOps
Apply
false
Expand All @@ -197,7 +197,7 @@ covtest.EnumTypes$
$anonfun
1263
1320
36
37
println
Apply
false
Expand All @@ -214,7 +214,7 @@ covtest.EnumTypes$
$anonfun
1271
1319
36
37
s
Apply
false
Expand All @@ -231,7 +231,7 @@ covtest.EnumTypes$
$anonfun
1296
1317
36
37
surfaceWeight
Apply
false
Expand All @@ -248,7 +248,7 @@ covtest.EnumTypes$
calculateEarthWeightOnPlanets
1109
1142
33
34
calculateEarthWeightOnPlanets
DefDef
false
Expand All @@ -265,7 +265,7 @@ covtest.EnumTypes$
test
1326
1347
38
39
println
Apply
false
Expand All @@ -282,7 +282,7 @@ covtest.EnumTypes$
test
1352
1385
39
40
calculateEarthWeightOnPlanets
Apply
false
Expand All @@ -299,7 +299,7 @@ covtest.EnumTypes$
test
901
909
27
28
test
DefDef
false
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/pos/Escaping.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ covtest.\n.\r\n\f
\r\n\f
69
80
3
4
length
Apply
false
Expand All @@ -44,7 +44,7 @@ covtest.\n.\r\n\f
\r\n\f
40
48
3
4
\r\n\f
DefDef
false
Expand Down
Loading
Loading