From 0fed29b34abad581989efa1df587bf65d9beb039 Mon Sep 17 00:00:00 2001 From: Richard Bradley Date: Wed, 19 Mar 2014 18:09:00 +0000 Subject: [PATCH 1/3] Add support for coverage exclusion comments --- README.md | 20 +++ build.sbt | 2 +- src/main/scala/scoverage/CoverageFilter.scala | 76 ++++++++++- src/main/scala/scoverage/plugin.scala | 123 ++++++++++++------ .../scala/scoverage/CoverageFilterTest.scala | 96 ++++++++++++-- 5 files changed, 256 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 2ece786b..0be93348 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,26 @@ project you will need to use one of the build plugins: If you want to write a tool that uses this code coverage library then it is available on maven central. Search for scalac-scoverage-plugin. +#### Excluding code from coverage stats + +You can exclude whole classes or packages by name. Pass a semicolon separated +list of regexes to the 'excludedPackages' option. + +For example: + -P:scoverage:excludedPackages:.*\.utils\..*;.*\.SomeClass;org\.apache\..* + +The regular expressions are matched against the fully qualified class name, and must match the entire string to take effect. + +Any matched classes will not be instrumented or included in the coverage report. + +You can also mark sections of code with comments like: + + // $COVERAGE:OFF$ + ... + // $COVERAGE:ON$ + +Any code between two such comments will not be instrumented or included in the coverage report. + ### Alternatives There are still only a few code coverage tools for Scala. Here are two that we know of: diff --git a/build.sbt b/build.sbt index eeffd2de..a3f5086c 100644 --- a/build.sbt +++ b/build.sbt @@ -2,7 +2,7 @@ name := "scalac-scoverage-plugin" organization := "org.scoverage" -version := "0.97.0" +version := "0.98.0" scalacOptions := Seq("-unchecked", "-deprecation", "-feature", "-encoding", "utf8") diff --git a/src/main/scala/scoverage/CoverageFilter.scala b/src/main/scala/scoverage/CoverageFilter.scala index 50646631..d88ded30 100644 --- a/src/main/scala/scoverage/CoverageFilter.scala +++ b/src/main/scala/scoverage/CoverageFilter.scala @@ -1,8 +1,76 @@ package scoverage -/** @author Stephen Samuel */ +import scala.collection.mutable +import scala.reflect.internal.util.SourceFile +import scala.reflect.internal.util.Position + +/** + * Methods related to filtering the instrumentation and coverage. + * + * @author Stephen Samuel + */ class CoverageFilter(excludedPackages: Seq[String]) { - def isIncluded(className: String): Boolean = { - excludedPackages.isEmpty || !excludedPackages.exists(_.r.pattern.matcher(className).matches) + + val excludedClassNamePatterns = excludedPackages.map(_.r.pattern) + /** + * We cache the excluded ranges to avoid scanning the source code files + * repeatedly. For a large project there might be a lot of source code + * data, so we only hold a weak reference. + */ + val linesExcludedByScoverageCommentsCache: mutable.Map[SourceFile, List[Range]] = + mutable.WeakHashMap.empty + + final val scoverageExclusionCommentsRegex = + """(?ms)^\s*//\s*(\$COVERAGE-OFF\$)\s*$.*?(^\s*//\s*\$COVERAGE-ON\$\s*$|\Z)""".r + + /** + * True if the given className has not been excluded by the + * `excludedPackages` option. + */ + def isClassIncluded(className: String): Boolean = { + excludedClassNamePatterns.isEmpty || + !excludedClassNamePatterns.exists(_.matcher(className).matches) + } + + /** + * True if the line containing `position` has not been excluded by a magic comment. + */ + def isLineIncluded(position: Position): Boolean = { + if (position.isDefined) { + val excludedLineNumbers = getExcludedLineNumbers(position.source) + val lineNumber = position.line + !excludedLineNumbers.exists(_.contains(lineNumber)) + } else { + true + } + } + + /** + * Checks the given sourceFile for any magic comments which exclude lines + * from coverage. Returns a list of Ranges of lines that should be excluded. + * + * The line numbers returned are conventional 1-based line numbers (i.e. the + * first line is line number 1) + */ + def getExcludedLineNumbers(sourceFile: SourceFile): List[Range] = { + linesExcludedByScoverageCommentsCache.get(sourceFile) match { + case Some(lineNumbers) => lineNumbers + case None => { + val lineNumbers = scoverageExclusionCommentsRegex.findAllIn(sourceFile.content).matchData.map { m => + // Asking a SourceFile for the line number of the char after + // the end of the file gives an exception + val endChar = math.min(m.end(2), sourceFile.content.length - 1) + // Most of the compiler API appears to use conventional + // 1-based line numbers (e.g. "Position.line"), but it appears + // that the "offsetToLine" method in SourceFile uses 0-based + // line numbers + Range( + 1 + sourceFile.offsetToLine(m.start(1)), + 1 + sourceFile.offsetToLine(endChar)) + }.toList + linesExcludedByScoverageCommentsCache.put(sourceFile, lineNumbers) + lineNumbers + } + } } -} \ No newline at end of file +} diff --git a/src/main/scala/scoverage/plugin.scala b/src/main/scala/scoverage/plugin.scala index 35296392..b2ca984e 100644 --- a/src/main/scala/scoverage/plugin.scala +++ b/src/main/scala/scoverage/plugin.scala @@ -10,26 +10,30 @@ import java.util.concurrent.atomic.AtomicInteger /** @author Stephen Samuel */ class ScoveragePlugin(val global: Global) extends Plugin { - val name: String = "scoverage" - val description: String = "scoverage code coverage compiler plugin" - val opts = new ScoverageOptions - val components: List[PluginComponent] = List(new ScoverageComponent(global, opts)) - - override def processOptions(options: List[String], error: String => Unit) { - for ( opt <- options ) { + override val name: String = "scoverage" + override val description: String = "scoverage code coverage compiler plugin" + val component = new ScoverageComponent(global) + override val components: List[PluginComponent] = List(component) + + override def processOptions(opts: List[String], error: String => Unit) { + val options = new ScoverageOptions + for ( opt <- opts ) { if (opt.startsWith("excludedPackages:")) { - opts.excludedPackages = opt.substring("excludedPackages:".length).split(";").map(_.trim).filterNot(_.isEmpty) + options.excludedPackages = opt.substring("excludedPackages:".length).split(";").map(_.trim).filterNot(_.isEmpty) } else if (opt.startsWith("dataDir:")) { - opts.dataDir = opt.substring("dataDir:".length) + options.dataDir = opt.substring("dataDir:".length) } else { error("Unknown option: " + opt) } } + component.setOptions(options) } override val optionsHelp: Option[String] = Some(Seq( "-P:scoverage:dataDir: where the coverage files should be written\n", - "-P:scoverage:excludedPackages:; semicolon separated list of regexs for packages to exclude\n" + "-P:scoverage:excludedPackages:; semicolon separated list of regexs for packages to exclude", + " Any classes whose fully qualified name matches the regex will", + " be excluded from coverage." ).mkString("\n")) } @@ -38,16 +42,38 @@ class ScoverageOptions { var dataDir: String = _ } -class ScoverageComponent(val global: Global, options: ScoverageOptions) - extends PluginComponent with TypingTransformers with Transform with TreeDSL { +class ScoverageComponent( + val global: Global) + extends PluginComponent + with TypingTransformers + with Transform + with TreeDSL { import global._ val statementIds = new AtomicInteger(0) val coverage = new Coverage - val phaseName: String = "scoverage" - val runsAfter: List[String] = List("typer") + override val phaseName: String = "scoverage" + override val runsAfter: List[String] = List("typer") override val runsBefore = List[String]("patmat") + /** + * Our options are not provided at construction time, but shortly after, + * so they start as None. + * You must call "setOptions" before running any commands that rely on + * the options. + */ + private var _options: Option[ScoverageOptions] = None + private var coverageFilter: Option[CoverageFilter] = None + + private def options: ScoverageOptions = { + require(_options.nonEmpty, "You must first call \"setOptions\"") + _options.get + } + + def setOptions(options: ScoverageOptions): Unit = { + _options = Some(options) + coverageFilter = Some(new CoverageFilter(options.excludedPackages)) + } override def newPhase(prev: scala.tools.nsc.Phase): Phase = new Phase(prev) { @@ -70,8 +96,14 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) var location: Location = null - def safeStart(tree: Tree): Int = if (tree.pos.isDefined) tree.pos.start else -1 - def safeEnd(tree: Tree): Int = if (tree.pos.isDefined) tree.pos.end else -1 + /** + * The 'start' of the position, if it is available, else -1 + * We cannot use 'isDefined' to test whether pos.start will work, as some + * classes (e.g. [[scala.reflect.internal.util.OffsetPosition]] have + * isDefined true, but throw on `start` + */ + def safeStart(tree: Tree): Int = scala.util.Try(tree.pos.start).getOrElse(-1) + def safeEnd(tree: Tree): Int = scala.util.Try(tree.pos.end).getOrElse(-1) def safeLine(tree: Tree): Int = if (tree.pos.isDefined) tree.pos.line else -1 def safeSource(tree: Tree): Option[SourceFile] = if (tree.pos.isDefined) Some(tree.pos.source) else None @@ -117,25 +149,28 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) println(s"[warn] Could not instrument [${tree.getClass.getSimpleName}/${tree.symbol}]. No position.") tree case Some(source) => - - val id = statementIds.incrementAndGet - val statement = MeasuredStatement( - source.path, - location, - id, - safeStart(tree), - safeEnd(tree), - safeLine(tree), - tree.toString(), - Option(tree.symbol).map(_.fullNameString).getOrElse(""), - tree.getClass.getSimpleName, - branch - ) - coverage.add(statement) - - val apply = invokeCall(id) - val block = Block(List(apply), tree) - localTyper.typed(atPos(tree.pos)(block)) + if (tree.pos.isDefined && !isStatementIncluded(tree.pos)) { + tree + } else { + val id = statementIds.incrementAndGet + val statement = MeasuredStatement( + source.path, + location, + id, + safeStart(tree), + safeEnd(tree), + safeLine(tree), + tree.toString(), + Option(tree.symbol).map(_.fullNameString).getOrElse(""), + tree.getClass.getSimpleName, + branch + ) + coverage.add(statement) + + val apply = invokeCall(id) + val block = Block(List(apply), tree) + localTyper.typed(atPos(tree.pos)(block)) + } } } @@ -153,8 +188,12 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) dir.getPath } - def isIncluded(t: Tree): Boolean = { - new CoverageFilter(options.excludedPackages).isIncluded(t.symbol.fullNameString) + def isClassIncluded(symbol: Symbol): Boolean = { + coverageFilter.get.isClassIncluded(symbol.fullNameString) + } + + def isStatementIncluded(pos: Position): Boolean = { + coverageFilter.get.isLineIncluded(pos) } def className(s: Symbol): String = { @@ -275,7 +314,7 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) // special support to handle partial functions case c: ClassDef if c.symbol.isAnonymousFunction && c.symbol.enclClass.superClass.nameString.contains("AbstractPartialFunction") => - if (isIncluded(c)) + if (isClassIncluded(c.symbol)) transformPartial(c) else c @@ -283,13 +322,13 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) // scalac generated classes, we just instrument the enclosed methods/statments // the location would stay as the source class case c: ClassDef if c.symbol.isAnonymousClass || c.symbol.isAnonymousFunction => - if (isIncluded(c)) + if (isClassIncluded(c.symbol)) super.transform(tree) else c case c: ClassDef => - if (isIncluded(c)) { + if (isClassIncluded(c.symbol)) { updateLocation(c.symbol) super.transform(tree) } @@ -386,7 +425,7 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) // user defined objects case m: ModuleDef => - if (isIncluded(m)) { + if (isClassIncluded(m.symbol)) { updateLocation(m.symbol) super.transform(tree) } @@ -422,7 +461,7 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) case n: New => super.transform(n) case p: PackageDef => - if (isIncluded(p)) treeCopy.PackageDef(p, p.pid, transformStatements(p.stats)) + if (isClassIncluded(p.symbol)) treeCopy.PackageDef(p, p.pid, transformStatements(p.stats)) else p // This AST node corresponds to the following Scala code: `return` expr diff --git a/src/test/scala/scoverage/CoverageFilterTest.scala b/src/test/scala/scoverage/CoverageFilterTest.scala index fe6c9fc3..63da0d85 100644 --- a/src/test/scala/scoverage/CoverageFilterTest.scala +++ b/src/test/scala/scoverage/CoverageFilterTest.scala @@ -1,34 +1,102 @@ package scoverage import org.scalatest.FlatSpec +import scala.reflect.internal.util.{NoFile, BatchSourceFile, SourceFile} class CoverageFilterTest extends FlatSpec { - "isIncluded" should "return true for empty excludes" in { - assert(new CoverageFilter(Nil).isIncluded("x")) + "isClassIncluded" should "return true for empty excludes" in { + assert(new CoverageFilter(Nil).isClassIncluded("x")) } - "isIncluded" should "not crash for empty input" in { - assert(new CoverageFilter(Nil).isIncluded("")) + "isClassIncluded" should "not crash for empty input" in { + assert(new CoverageFilter(Nil).isClassIncluded("")) } - "isIncluded" should "exclude scoverage -> scoverage" in { - assert(!new CoverageFilter(Seq("scoverage")).isIncluded("scoverage")) + "isClassIncluded" should "exclude scoverage -> scoverage" in { + assert(!new CoverageFilter(Seq("scoverage")).isClassIncluded("scoverage")) } - "isIncluded" should "include scoverage -> scoverageeee" in { - assert(new CoverageFilter(Seq("scoverage")).isIncluded("scoverageeee")) + "isClassIncluded" should "include scoverage -> scoverageeee" in { + assert(new CoverageFilter(Seq("scoverage")).isClassIncluded("scoverageeee")) } - "isIncluded" should "exclude scoverage* -> scoverageeee" in { - assert(!new CoverageFilter(Seq("scoverage*")).isIncluded("scoverageeee")) + "isClassIncluded" should "exclude scoverage* -> scoverageeee" in { + assert(!new CoverageFilter(Seq("scoverage*")).isClassIncluded("scoverageeee")) } - "isIncluded" should "include eee -> scoverageeee" in { - assert(new CoverageFilter(Seq("eee")).isIncluded("scoverageeee")) + "isClassIncluded" should "include eee -> scoverageeee" in { + assert(new CoverageFilter(Seq("eee")).isClassIncluded("scoverageeee")) } - "isIncluded" should "exclude .*eee -> scoverageeee" in { - assert(!new CoverageFilter(Seq(".*eee")).isIncluded("scoverageeee")) + "isClassIncluded" should "exclude .*eee -> scoverageeee" in { + assert(!new CoverageFilter(Seq(".*eee")).isClassIncluded("scoverageeee")) + } + + "getExcludedLineNumbers" should "exclude no lines if no magic comments are found" in { + val file = + """1 + |2 + |3 + |4 + |5 + |6 + |7 + |8 + """.stripMargin + + val numbers = new CoverageFilter(Nil).getExcludedLineNumbers(mockSourceFile(file)) + numbers === List.empty + } + + "getExcludedLineNumbers" should "exclude lines between magic comments" in { + val file = + """1 + |2 + |3 + | // $COVERAGE-OFF$ + |5 + |6 + |7 + |8 + | // $COVERAGE-ON$ + |10 + |11 + | // $COVERAGE-OFF$ + |13 + | // $COVERAGE-ON$ + |15 + |16 + """.stripMargin + + val numbers = new CoverageFilter(Nil).getExcludedLineNumbers(mockSourceFile(file)) + numbers === List(Range(4,9), Range(12,14)) + } + + "getExcludedLineNumbers" should "exclude all lines after an upaired magic comment" in { + val file = + """1 + |2 + |3 + | // $COVERAGE-OFF$ + |5 + |6 + |7 + |8 + | // $COVERAGE-ON$ + |10 + |11 + | // $COVERAGE-OFF$ + |13 + |14 + |15 + """.stripMargin + + val numbers = new CoverageFilter(Nil).getExcludedLineNumbers(mockSourceFile(file)) + numbers === List(Range(4,9), Range(12,16)) + } + + private def mockSourceFile(contents: String): SourceFile = { + new BatchSourceFile(NoFile, contents.toCharArray) } } From 81c90fba9035f41f3e37572895850e052eb8de85 Mon Sep 17 00:00:00 2001 From: Richard Bradley Date: Thu, 20 Mar 2014 17:24:47 +0000 Subject: [PATCH 2/3] Fix comment examples in README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0be93348..12bd1a95 100644 --- a/README.md +++ b/README.md @@ -108,9 +108,9 @@ Any matched classes will not be instrumented or included in the coverage report. You can also mark sections of code with comments like: - // $COVERAGE:OFF$ + // $COVERAGE-OFF$ ... - // $COVERAGE:ON$ + // $COVERAGE-ON$ Any code between two such comments will not be instrumented or included in the coverage report. From 5396129f07ee182d5dcca06f16fd461f599beda4 Mon Sep 17 00:00:00 2001 From: Richard Bradley Date: Thu, 20 Mar 2014 19:38:12 +0000 Subject: [PATCH 3/3] Allow text comments on the same line as the marker comments --- src/main/scala/scoverage/CoverageFilter.scala | 2 +- .../scala/scoverage/CoverageFilterTest.scala | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/main/scala/scoverage/CoverageFilter.scala b/src/main/scala/scoverage/CoverageFilter.scala index d88ded30..e88a3b93 100644 --- a/src/main/scala/scoverage/CoverageFilter.scala +++ b/src/main/scala/scoverage/CoverageFilter.scala @@ -21,7 +21,7 @@ class CoverageFilter(excludedPackages: Seq[String]) { mutable.WeakHashMap.empty final val scoverageExclusionCommentsRegex = - """(?ms)^\s*//\s*(\$COVERAGE-OFF\$)\s*$.*?(^\s*//\s*\$COVERAGE-ON\$\s*$|\Z)""".r + """(?ms)^\s*//\s*(\$COVERAGE-OFF\$).*?(^\s*//\s*\$COVERAGE-ON\$|\Z)""".r /** * True if the given className has not been excluded by the diff --git a/src/test/scala/scoverage/CoverageFilterTest.scala b/src/test/scala/scoverage/CoverageFilterTest.scala index 63da0d85..1b7ccd16 100644 --- a/src/test/scala/scoverage/CoverageFilterTest.scala +++ b/src/test/scala/scoverage/CoverageFilterTest.scala @@ -96,6 +96,29 @@ class CoverageFilterTest extends FlatSpec { numbers === List(Range(4,9), Range(12,16)) } + "getExcludedLineNumbers" should "allow text comments on the same line as the markers" in { + val file = + """1 + |2 + |3 + | // $COVERAGE-OFF$ because the next lines are boring + |5 + |6 + |7 + |8 + | // $COVERAGE-ON$ resume coverage here + |10 + |11 + | // $COVERAGE-OFF$ but ignore this bit + |13 + |14 + |15 + """.stripMargin + + val numbers = new CoverageFilter(Nil).getExcludedLineNumbers(mockSourceFile(file)) + numbers === List(Range(4,9), Range(12,16)) + } + private def mockSourceFile(contents: String): SourceFile = { new BatchSourceFile(NoFile, contents.toCharArray) }