Fixed all tests. Updated handling of for loops

Author: sksamuel

project/Scoverage.scala

@@ -7,7 +7,7 @@ object Scoverage extends Build {
   val Version = "1.0.3-SNAPSHOT"
   val Scala = "2.11.4"
   val MockitoVersion = "1.9.5"
-  val ScalatestVersion = "2.2.2"
+  val ScalatestVersion = "2.2.3"
 
   lazy val LocalTest = config("local") extend Test
 
@@ -77,8 +77,8 @@ object Scoverage extends Build {
     .settings(name := "scalac-scoverage-plugin")
     .settings(appSettings: _*)
     .settings(libraryDependencies ++= Seq(
-    "org.scala-lang" % "scala-reflect" % scalaVersion.value % "provided",
-    "org.scala-lang" % "scala-compiler" % scalaVersion.value % "provided",
+    "org.scala-lang" % "scala-reflect" % Scala % "provided",
+    "org.scala-lang" % "scala-compiler" % Scala % "provided",
     "org.joda" % "joda-convert" % "1.6" % "test",
     "joda-time" % "joda-time" % "2.3" % "test",
     "com.typesafe.scala-logging" %% "scala-logging-slf4j" % "2.1.2" % "test"

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

@@ -5,8 +5,8 @@ import java.util.concurrent.atomic.AtomicInteger
 
 import scala.reflect.internal.ModifierFlags
 import scala.reflect.internal.util.SourceFile
-import scala.tools.nsc._
-import scala.tools.nsc.plugins.{Plugin, PluginComponent}
+import scala.tools.nsc.Global
+import scala.tools.nsc.plugins.{PluginComponent, Plugin}
 import scala.tools.nsc.transform.{Transform, TypingTransformers}
 
 /** @author Stephen Samuel */
@@ -142,6 +142,16 @@ class ScoverageInstrumentationComponent(val global: Global)
 
     def transformStatements(trees: List[Tree]): List[Tree] = trees.map(process)
 
+    def transformForCases(cases: List[CaseDef]): List[CaseDef] = {
+      // we don't instrument the synthetic case _ => false clause
+      cases.dropRight(1).map(c => {
+        treeCopy.CaseDef(
+          // in a for-loop we don't care about instrumenting the guards, as they are synthetically generated
+          c, c.pat, process(c.guard), process(c.body)
+        )
+      }) ++ cases.takeRight(1)
+    }
+
     def transformCases(cases: List[CaseDef]): List[CaseDef] = {
       cases.map(c => {
         treeCopy.CaseDef(
@@ -167,7 +177,7 @@ class ScoverageInstrumentationComponent(val global: Global)
               safeStart(tree),
               safeEnd(tree),
               safeLine(tree),
-              original.toString(),
+              original.toString,
               Option(original.symbol).fold("<nosymbol>")(_.fullNameString),
               tree.getClass.getSimpleName,
               branch
@@ -225,6 +235,8 @@ class ScoverageInstrumentationComponent(val global: Global)
     def traverseApplication(t: Tree): Tree = {
       t match {
         case a: ApplyToImplicitArgs => treeCopy.Apply(a, traverseApplication(a.fun), transformStatements(a.args))
+        case Apply(Select(_, TermName("withFilter")), List(fun@Function(params, body)))
+          if fun.symbol.isSynthetic && fun.toString.contains("check$ifrefutable$1") => t
         case a: Apply => treeCopy.Apply(a, traverseApplication(a.fun), transformStatements(a.args))
         case a: TypeApply => treeCopy.TypeApply(a, traverseApplication(a.fun), transformStatements(a.args))
         case s: Select => treeCopy.Select(s, traverseApplication(s.qualifier), s.name)
@@ -323,11 +335,6 @@ class ScoverageInstrumentationComponent(val global: Global)
         // ignore macro definitions in 2.11
         case DefDef(mods, _, _, _, _, _) if mods.isMacro => tree
 
-        case d: DefDef =>
-          println(d)
-          val e = d
-          d
-
         // this will catch methods defined as macros, eg def test = macro testImpl
         // it will not catch macro implemenations
         case d: DefDef if d.symbol != null
@@ -428,16 +435,17 @@ class ScoverageInstrumentationComponent(val global: Global)
         // pattern match clauses will be instrumented per case
         case m@Match(selector: Tree, cases: List[CaseDef]) =>
           // we can be fairly sure this was generated as part of a for loop
-          if (selector.toString().contains("check$")
+          if (selector.toString.contains("check$")
             && selector.tpe.annotations.mkString == "unchecked"
             && m.cases.last.toString == "case _ => false") {
-            // todo check these assumptions for 2.11
-            treeCopy
-              .Match(tree, instrument(selector, selector), transformCases(cases.dropRight(1)) ++ cases.takeRight(1))
+            treeCopy.Match(tree, process(selector), transformForCases(cases))
           } else {
-            if (selector.symbol.isSynthetic)
+            // if the selector was added by compiler, we don't want to instrument it....
+            // that usually means some construct is being transformed into a match
+            if (Option(selector.symbol).exists(_.isSynthetic))
               treeCopy.Match(tree, selector, transformCases(cases))
             else
+            // .. but we will if it was a user match
               treeCopy.Match(tree, process(selector), transformCases(cases))
           }
 

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

@@ -19,7 +19,9 @@ class PluginCoverageTest
                                    |  }
                                    |} """.stripMargin)
     assert(!compiler.reporter.hasErrors)
-    // we should have 2 statements - initialising the val and executing string sub in the def
+    // we expect:
+    // instrumenting the default-param which becomes a method call invocation
+    // the method makeGreeting is entered.
     compiler.assertNMeasuredStatements(2)
   }
 
@@ -89,30 +91,69 @@ class PluginCoverageTest
     compiler.assertNMeasuredStatements(6)
   }
 
-  test("scoverage should instrument selectors in match") {
+  test("scoverage should instrument non basic selector") {
+    val compiler = ScoverageCompiler.default
+    compiler.compileCodeSnippet( """ trait A {
+                                   |  def someValue = "sammy"
+                                   |  def foo(a:String) = someValue match {
+                                   |    case any => "yes"
+                                   |  }
+                                   |} """.stripMargin)
+    assert(!compiler.reporter.hasErrors)
+    // should instrument:
+    // the someValue method entry
+    // the selector call
+    // case block "yes" literal
+    compiler.assertNMeasuredStatements(3)
+  }
+
+  test("scoverage should instrument conditional selectors in a match") {
     val compiler = ScoverageCompiler.default
     compiler.compileCodeSnippet( """ trait A {
                                    |  def foo(a:String) = (if (a == "hello") 1 else 2) match {
                                    |    case any => "yes"
                                    |  }
                                    |} """.stripMargin)
     assert(!compiler.reporter.hasErrors)
-    // should instrument the method call, the if clause, thenp, thenp literal, elsep, elsep literal, case block,
-    // case block literal
-    compiler.assertNMeasuredStatements(8)
+    // should instrument:
+    // the if clause,
+    // thenp block,
+    // thenp literal "1",
+    // elsep block,
+    // elsep literal "2",
+    // case block "yes" literal
+    compiler.assertNMeasuredStatements(6)
   }
 
   // https://github.com/scoverage/sbt-scoverage/issues/16
-  test("scoverage should instrument for-loops but not the generated default case") {
+  test("scoverage should instrument for-loops but not the generated scaffolding") {
     val compiler = ScoverageCompiler.default
     compiler.compileCodeSnippet( """ trait A {
                                    |  def print1(list: List[String]) = for (string: String <- list) println(string)
                                    |} """.stripMargin)
     assert(!compiler.reporter.hasErrors)
     assert(!compiler.reporter.hasWarnings)
-    // should have one statement for the withFilter invoke, one of the match selector,
-    // one of the case block, one for the case string RHS value, one for the foreach block.
-    compiler.assertNMeasuredStatements(5)
+    // should instrument:
+    // the def method entry
+    // foreach body
+    // specifically we want to avoid the withFilter partial function added by the compiler
+    compiler.assertNMeasuredStatements(2)
+  }
+
+  test("scoverage should instrument for-loop guards") {
+    val compiler = ScoverageCompiler.default
+
+    compiler.compileCodeSnippet( """object A {
+                                   |  def foo(list: List[String]) = for (string: String <- list if string.length > 5)
+                                   |    println(string)
+                                   |} """.stripMargin)
+    assert(!compiler.reporter.hasErrors)
+    assert(!compiler.reporter.hasWarnings)
+    // should instrument:
+    // foreach body
+    // the guard
+    // but we want to avoid the withFilter partial function added by the compiler
+    compiler.assertNMeasuredStatements(3)
   }
 
   test("scoverage should correctly handle new with args (apply with list of args)") {
@@ -188,14 +229,15 @@ class PluginCoverageTest
     compiler.compileCodeSnippet( """ trait A {
                                    |  def foo(name: Any) = name match {
                                    |    case i : Int => 1
-                                   |    case b : Boolean => 2
+                                   |    case b : Boolean => println("boo")
                                    |    case _ => 3
                                    |  }
                                    |} """.stripMargin)
     assert(!compiler.reporter.hasErrors)
     assert(!compiler.reporter.hasWarnings)
-    // should have one statement for each literal, one for each case block, and one for the selector.
-    compiler.assertNMeasuredStatements(7)
+    // should have one statement for each case body
+    // selector is a constant so would be ignored.
+    compiler.assertNMeasuredStatements(3)
   }
 
   test("scoverage should support yields") {