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.

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

fix: ignore lazy definition generated from by-name implicits #536

Merged

Conversation

danielleontiev
Copy link
Contributor

@danielleontiev danielleontiev commented Mar 5, 2023

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

@danielleontiev danielleontiev force-pushed the ignore-lazy-defs-from-by-name-implicits branch from 51f4004 to a0588cd Compare March 5, 2023 17:15
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
@danielleontiev danielleontiev force-pushed the ignore-lazy-defs-from-by-name-implicits branch from a0588cd to 8382a62 Compare March 5, 2023 21:11
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Thanks for this @danielleontiev! And thanks for the explanations. Regarding the position validation behavior, to be honest I'm not really sure what's happening there. However all tests are passing so I'm pretty comfortable merging.

@ckipp01 ckipp01 merged commit 8dd2df4 into scoverage:main Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants