Skip to content

Commit 051fc70

Browse files
committed
Added better support for new statements
1 parent 04ba5b0 commit 051fc70

File tree

3 files changed

+98
-66
lines changed

3 files changed

+98
-66
lines changed

scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class ScoverageOptions {
5050
var dataDir: String = File.createTempFile("scoverage_datadir_not_defined", ".tmp").getParent
5151
}
5252

53+
5354
class ScoverageInstrumentationComponent(val global: Global)
5455
extends PluginComponent
5556
with TypingTransformers
@@ -250,12 +251,12 @@ class ScoverageInstrumentationComponent(val global: Global)
250251
// ignore macro expanded code, do not send to super as we don't want any children to be instrumented
251252
case t if t.attachments.all.toString().contains("MacroExpansionAttachment") => t
252253

253-
/**
254-
* Object creation from new.
255-
* Ignoring creation calls to anon functions
256-
*/
257-
case a: GenericApply if a.symbol.isConstructor && a.symbol.enclClass.isAnonymousFunction => tree
258-
case a: GenericApply if a.symbol.isConstructor => instrument(a)
254+
// /**
255+
// * Object creation from new.
256+
// * Ignoring creation calls to anon functions
257+
// */
258+
// case a: GenericApply if a.symbol.isConstructor && a.symbol.enclClass.isAnonymousFunction => tree
259+
// case a: GenericApply if a.symbol.isConstructor => instrument(a)
259260

260261
/**
261262
* When an apply has no parameters, or is an application of purely literals or idents
@@ -271,6 +272,10 @@ class ScoverageInstrumentationComponent(val global: Global)
271272
//todo remove once scala merges into Apply proper
272273
case a: ApplyToImplicitArgs =>
273274
instrument(treeCopy.Apply(a, traverseApplication(a.fun), transformStatements(a.args)))
275+
276+
// handle 'new' keywords, instrumenting parameter lists
277+
case a@Apply(s@Select(New(tpt), name), args) =>
278+
instrument(treeCopy.Apply(a, s, transformStatements(args)))
274279
case a: Apply =>
275280
instrument(treeCopy.Apply(a, traverseApplication(a.fun), transformStatements(a.args)))
276281
case a: TypeApply =>
@@ -331,13 +336,8 @@ class ScoverageInstrumentationComponent(val global: Global)
331336
case d: DefDef if d.symbol != null && d.tpt.symbol.fullNameString == "scala.reflect.api.Exprs.Expr" =>
332337
tree
333338

334-
case d: DefDef if d.symbol.isPrimaryConstructor =>
335-
updateLocation(d)
336-
tree
337-
338-
case d: DefDef if tree.symbol.isConstructor =>
339-
updateLocation(d)
340-
tree
339+
// we can ignore primary constructors because they are just empty at this stage, the body is added later.
340+
case d: DefDef if d.symbol.isPrimaryConstructor => tree
341341

342342
/**
343343
* Case class accessors for vals
@@ -464,9 +464,9 @@ class ScoverageInstrumentationComponent(val global: Global)
464464
* List(Literal(Constant(2)))),
465465
* List(Literal(Constant(3))))
466466
*
467-
* We can ignore New as they should be profiled by the outer select.
468467
*/
469-
case n: New => super.transform(n)
468+
case s@Select(n@New(tpt), name) =>
469+
instrument(treeCopy.Select(s, n, name))
470470

471471
case p: PackageDef =>
472472
if (isClassIncluded(p.symbol)) treeCopy.PackageDef(p, p.pid, transformStatements(p.stats))
@@ -475,7 +475,6 @@ class ScoverageInstrumentationComponent(val global: Global)
475475
// This AST node corresponds to the following Scala code: `return` expr
476476
case r: Return =>
477477
treeCopy.Return(r, transform(r.expr))
478-
479478
/** pattern match with syntax `Select(qual, name)`.
480479
* This AST node corresponds to the following Scala code:
481480
*

scalac-scoverage-plugin/src/test/scala/scoverage/PluginCoverageTest.scala

Lines changed: 80 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ class PluginCoverageTest
1313
test("scoverage should instrument default arguments with methods") {
1414
val compiler = ScoverageCompiler.default
1515
compiler.compileCodeSnippet( """ object DefaultArgumentsObject {
16-
| val defaultName = "world"
17-
| def makeGreeting(name: String = defaultName): String = {
18-
| s"Hello, $name"
19-
| }
20-
|} """.stripMargin)
16+
| val defaultName = "world"
17+
| def makeGreeting(name: String = defaultName): String = {
18+
| s"Hello, $name"
19+
| }
20+
|} """.stripMargin)
2121
assert(!compiler.reporter.hasErrors)
2222
// we should have 2 statements - initialising the val and executing string sub in the def
2323
compiler.assertNMeasuredStatements(2)
@@ -43,14 +43,14 @@ class PluginCoverageTest
4343
test("scoverage should instrument final vals") {
4444
val compiler = ScoverageCompiler.default
4545
compiler.compileCodeSnippet( """ object FinalVals {
46-
| final val name = {
47-
| val name = "sammy"
48-
| if (System.currentTimeMillis() > 0) {
49-
| println(name)
50-
| }
51-
| }
52-
| println(name)
53-
|} """.stripMargin)
46+
| final val name = {
47+
| val name = "sammy"
48+
| if (System.currentTimeMillis() > 0) {
49+
| println(name)
50+
| }
51+
| }
52+
| println(name)
53+
|} """.stripMargin)
5454
assert(!compiler.reporter.hasErrors)
5555
// we should have 3 statements - initialising the val, executing println, and executing the parameter
5656
compiler.assertNMeasuredStatements(8)
@@ -59,10 +59,10 @@ class PluginCoverageTest
5959
test("scoverage should instrument selectors in match") {
6060
val compiler = ScoverageCompiler.default
6161
compiler.compileCodeSnippet( """ trait A {
62-
| def foo(a:String) = (if (a == "hello") 1 else 2) match {
63-
| case any => "yes"
64-
| }
65-
|} """.stripMargin)
62+
| def foo(a:String) = (if (a == "hello") 1 else 2) match {
63+
| case any => "yes"
64+
| }
65+
|} """.stripMargin)
6666
assert(!compiler.reporter.hasErrors)
6767
// should instrument the method call, the if clause, thenp, thenp literal, elsep, elsep literal, case block,
6868
// case block literal
@@ -73,20 +73,53 @@ class PluginCoverageTest
7373
test("scoverage should instrument for-loops but not the generated default case") {
7474
val compiler = ScoverageCompiler.default
7575
compiler.compileCodeSnippet( """ trait A {
76-
| def print1(list: List[String]) = for (string: String <- list) println(string)
77-
|} """.stripMargin)
76+
| def print1(list: List[String]) = for (string: String <- list) println(string)
77+
|} """.stripMargin)
7878
assert(!compiler.reporter.hasErrors)
7979
assert(!compiler.reporter.hasWarnings)
8080
// should have one statement for the withFilter invoke, one of the match selector,
8181
// one of the case block, one for the case string RHS value, one for the foreach block.
8282
compiler.assertNMeasuredStatements(5)
8383
}
8484

85+
test("scoverage should correctly handle new with args (apply with list of args)") {
86+
val compiler = ScoverageCompiler.default
87+
compiler.compileCodeSnippet( """ object A {
88+
| new String(new String(new String))
89+
| } """.stripMargin)
90+
assert(!compiler.reporter.hasErrors)
91+
assert(!compiler.reporter.hasWarnings)
92+
// should have 3 statements, one for each of the nested strings
93+
compiler.assertNMeasuredStatements(3)
94+
}
95+
96+
test("scoverage should correctly handle no args new (apply, empty list of args)") {
97+
val compiler = ScoverageCompiler.default
98+
compiler.compileCodeSnippet( """ object A {
99+
| new String
100+
| } """.stripMargin)
101+
assert(!compiler.reporter.hasErrors)
102+
assert(!compiler.reporter.hasWarnings)
103+
// should have 1. the apply that wraps the select.
104+
compiler.assertNMeasuredStatements(1)
105+
}
106+
107+
test("scoverage should correctly handle new that invokes nested statements") {
108+
val compiler = ScoverageCompiler.default
109+
compiler.compileCodeSnippet( """ object A {
110+
| new String(if (System.currentTimeMillis > 1) "yes" else "no")
111+
| } """.stripMargin)
112+
assert(!compiler.reporter.hasErrors)
113+
assert(!compiler.reporter.hasWarnings)
114+
// should have 6 statements - the apply/new statement, two literals, the if cond, if elsep, if thenp
115+
compiler.assertNMeasuredStatements(6)
116+
}
117+
85118
test("scoverage should instrument val RHS") {
86119
val compiler = ScoverageCompiler.default
87120
compiler.compileCodeSnippet( """object A {
88-
| val name = BigDecimal(50.0)
89-
|} """.stripMargin)
121+
| val name = BigDecimal(50.0)
122+
|} """.stripMargin)
90123
assert(!compiler.reporter.hasErrors)
91124
assert(!compiler.reporter.hasWarnings)
92125
compiler.assertNMeasuredStatements(1)
@@ -95,12 +128,12 @@ class PluginCoverageTest
95128
test("scoverage should instrument all case statements in an explicit match") {
96129
val compiler = ScoverageCompiler.default
97130
compiler.compileCodeSnippet( """ trait A {
98-
| def foo(name: Any) = name match {
99-
| case i : Int => 1
100-
| case b : Boolean => 2
101-
| case _ => 3
102-
| }
103-
|} """.stripMargin)
131+
| def foo(name: Any) = name match {
132+
| case i : Int => 1
133+
| case b : Boolean => 2
134+
| case _ => 3
135+
| }
136+
|} """.stripMargin)
104137
assert(!compiler.reporter.hasErrors)
105138
assert(!compiler.reporter.hasWarnings)
106139
// should have one statement for each literal, one for each case block, and one for the selector.
@@ -110,12 +143,12 @@ class PluginCoverageTest
110143
test("scoverage should support yields") {
111144
val compiler = ScoverageCompiler.default
112145
compiler.compileCodeSnippet( """
113-
| object Yielder {
114-
| val holidays = for ( name <- Seq("sammy", "clint", "lee");
115-
| place <- Seq("london", "philly", "iowa") ) yield {
116-
| name + " has been to " + place
117-
| }
118-
| }""".stripMargin)
146+
| object Yielder {
147+
| val holidays = for ( name <- Seq("sammy", "clint", "lee");
148+
| place <- Seq("london", "philly", "iowa") ) yield {
149+
| name + " has been to " + place
150+
| }
151+
| }""".stripMargin)
119152
assert(!compiler.reporter.hasErrors)
120153
// 2 statements for the two applies in Seq, one for each literal which is 6, one for the flat map,
121154
// one for the map, one for the yield op.
@@ -125,17 +158,17 @@ class PluginCoverageTest
125158
test("scoverage should not instrument local macro implementation") {
126159
val compiler = ScoverageCompiler.default
127160
compiler.compileCodeSnippet( """
128-
| object MyMacro {
129-
| import scala.language.experimental.macros
130-
| import scala.reflect.macros.Context
131-
| def test = macro testImpl
132-
| def testImpl(c: Context): c.Expr[Unit] = {
133-
| import c.universe._
134-
| reify {
135-
| println("macro test")
136-
| }
137-
| }
138-
|} """.stripMargin)
161+
| object MyMacro {
162+
| import scala.language.experimental.macros
163+
| import scala.reflect.macros.Context
164+
| def test = macro testImpl
165+
| def testImpl(c: Context): c.Expr[Unit] = {
166+
| import c.universe._
167+
| reify {
168+
| println("macro test")
169+
| }
170+
| }
171+
|} """.stripMargin)
139172
assert(!compiler.reporter.hasErrors)
140173
compiler.assertNoCoverage()
141174
}
@@ -153,9 +186,9 @@ class PluginCoverageTest
153186
"scala-logging-slf4j_" + ScoverageCompiler.ShortScalaVersion,
154187
"2.1.2")
155188
compiler.compileCodeSnippet( """import com.typesafe.scalalogging.slf4j.StrictLogging
156-
|class MacroTest extends StrictLogging {
157-
| logger.info("will break")
158-
|} """.stripMargin)
189+
|class MacroTest extends StrictLogging {
190+
| logger.info("will break")
191+
|} """.stripMargin)
159192
assert(!compiler.reporter.hasErrors)
160193
assert(!compiler.reporter.hasWarnings)
161194
compiler.assertNoCoverage()

scalac-scoverage-plugin/src/test/scala/scoverage/compiler.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ object ScoverageCompiler {
1818

1919
def settings: Settings = {
2020
val s = new scala.tools.nsc.Settings
21-
//s.Xprint.value = List("all")
21+
s.Xprint.value = List("all")
2222
s.Yrangepos.value = true
23-
//s.Yposdebug.value = true
23+
s.Yposdebug.value = true
2424
s.classpath.value = classPath.mkString(":")
2525
s
2626
}
@@ -99,7 +99,7 @@ class ScoverageCompiler(settings: scala.tools.nsc.Settings, reporter: scala.tool
9999
def assertNMeasuredStatements(n: Int): Unit = {
100100
for ( k <- 1 to n ) {
101101
assert(testStore.sources.mkString(" ").contains(s"scoverage.Invoker.invoked($k,"),
102-
s"Should be $n invoked statements but missing $k")
102+
s"Should be $n invoked statements but missing #$k")
103103
}
104104
assert(!testStore.sources.mkString(" ").contains(s"scoverage.Invoker.invoked(${n + 1},"),
105105
s"Found statement ${n + 1} but only expected $n")

0 commit comments

Comments
 (0)