From 8382a621c1eee67816fdc884a6201299a8e5ce35 Mon Sep 17 00:00:00 2001 From: Daniil Leontiev Date: Sun, 5 Mar 2023 20:46:11 +0400 Subject: [PATCH] fix: ignore lazy definition generated from by-name implicits Preamble: by-name implicit parameters used for shapeless magic and were introduced in SIP-31 (available from 2.13+): https://docs.scala-lang.org/sips/byname-implicits.html Using scoverage with such code may result in the following warning from the plugin: [warn] Could not instrument [Select/value rec$1]. Originally discovered by using scoverage with the code that derives typeclass instances using kittens library: case class Bar() case class Foo(bars: List[Bar]) object Foo { implicit eq: cats.Eq[Foo] = cats.derived.semiauto.eq } Solution: Ignore LazyDefns$1.rec$1 from instrumentation like other synthetic code Other changes: It was necessary to disable position validation phase for the test case that reproduces the issue because the code generated by scala compiler fails such validation. It could be also checked outside testing environment by manually setting -Yvalidate-pos:typer option to the compiler. Not sure whether it's expected compiler behavior or not --- build.sbt | 9 ++++- .../scala/scoverage/ScoveragePlugin.scala | 8 +++++ .../Scala213PluginCoverageTest.scala | 33 +++++++++++++++++++ .../scala/scoverage/ScoverageCompiler.scala | 15 ++++++--- 4 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 plugin/src/test/scala-2.13+/scoverage/Scala213PluginCoverageTest.scala diff --git a/build.sbt b/build.sbt index 2ad28e0d..12c510d9 100644 --- a/build.sbt +++ b/build.sbt @@ -151,7 +151,14 @@ lazy val plugin = sharedSettings ) .settings( - Test / unmanagedSourceDirectories += (Test / sourceDirectory).value / "scala-2.12+" + Test / unmanagedSourceDirectories += (Test / sourceDirectory).value / "scala-2.12+", + Test / unmanagedSourceDirectories ++= { + val sourceDir = (Test / sourceDirectory).value + CrossVersion.partialVersion(scalaVersion.value) match { + case Some((2, n)) if n >= 13 => Seq(sourceDir / "scala-2.13+") + case _ => Seq.empty + } + } ) .dependsOn(domain, reporter % "test->compile", serializer, buildInfo % Test) diff --git a/plugin/src/main/scala/scoverage/ScoveragePlugin.scala b/plugin/src/main/scala/scoverage/ScoveragePlugin.scala index d772e07b..6a144e9c 100644 --- a/plugin/src/main/scala/scoverage/ScoveragePlugin.scala +++ b/plugin/src/main/scala/scoverage/ScoveragePlugin.scala @@ -790,6 +790,14 @@ class ScoverageInstrumentationComponent( */ case s: Select if s.symbol.isLazy => tree + // Generated by compiler for lazy definitions involving + // by-name implicit parameters. More on that here: + // https://docs.scala-lang.org/sips/byname-implicits.html + // + // final val lazyDefns$1: LazyDefns$1 = new LazyDefns$1(); + // lazyDefns$1.rec$1() + case s: Select if s.symbol.isSynthetic => tree + case s: Select => instrument( treeCopy.Select(s, traverseApplication(s.qualifier), s.name), diff --git a/plugin/src/test/scala-2.13+/scoverage/Scala213PluginCoverageTest.scala b/plugin/src/test/scala-2.13+/scoverage/Scala213PluginCoverageTest.scala new file mode 100644 index 00000000..364b09ca --- /dev/null +++ b/plugin/src/test/scala-2.13+/scoverage/Scala213PluginCoverageTest.scala @@ -0,0 +1,33 @@ +package scoverage + +import munit.FunSuite + +class Scala213PluginCoverageTest extends FunSuite with MacroSupport { + + test( + "scoverage should ignore synthetic lazy definitions generated by compiler from by-name implicits" + ) { + val compiler = ScoverageCompiler.noPositionValidation + compiler.compileCodeSnippet( + """ + |object test { + | + | trait Foo { + | def next: Foo + | } + | + | object Foo { + | implicit def foo(implicit rec: => Foo): Foo = + | new Foo { def next = rec } + | } + | + | val foo = implicitly[Foo] + | + |} + | + """.stripMargin + ) + assert(!compiler.reporter.hasErrors) + assert(!compiler.reporter.hasWarnings) + } +} diff --git a/plugin/src/test/scala/scoverage/ScoverageCompiler.scala b/plugin/src/test/scala/scoverage/ScoverageCompiler.scala index f06aba2c..09b41e85 100644 --- a/plugin/src/test/scala/scoverage/ScoverageCompiler.scala +++ b/plugin/src/test/scala/scoverage/ScoverageCompiler.scala @@ -57,12 +57,17 @@ private[scoverage] object ScoverageCompiler { def default: ScoverageCompiler = { val reporter = new scala.tools.nsc.reporters.ConsoleReporter(settings) - new ScoverageCompiler(settings, reporter) + new ScoverageCompiler(settings, reporter, validatePositions = true) + } + + def noPositionValidation: ScoverageCompiler = { + val reporter = new scala.tools.nsc.reporters.ConsoleReporter(settings) + new ScoverageCompiler(settings, reporter, validatePositions = false) } def defaultJS: ScoverageCompiler = { val reporter = new scala.tools.nsc.reporters.ConsoleReporter(jsSettings) - new ScoverageCompiler(jsSettings, reporter) + new ScoverageCompiler(jsSettings, reporter, validatePositions = true) } def locationCompiler: LocationCompiler = { @@ -152,7 +157,8 @@ private[scoverage] object ScoverageCompiler { class ScoverageCompiler( settings: scala.tools.nsc.Settings, - rep: scala.tools.nsc.reporters.Reporter + rep: scala.tools.nsc.reporters.Reporter, + validatePositions: Boolean ) extends scala.tools.nsc.Global(settings, rep) { def addToClassPath(file: File): Unit = { @@ -268,7 +274,8 @@ class ScoverageCompiler( override def computeInternalPhases(): Unit = { super.computeInternalPhases() - addToPhasesSet(validator, "scoverage validator") + if (validatePositions) + addToPhasesSet(validator, "scoverage validator") addToPhasesSet( instrumentationComponent, "scoverage instrumentationComponent"