bugfix: scoverage does not instrument pat-mat assignment properly

[mekpavit]

Why

As mentioned in this issue, scoverage doesn't instrument its measure statement(s) into the RHS of pattern-matching assignment. This causes the coverage report to not include any of the user code inside that RHS and make the coverage score differs from the reality.

The root cause of this issue is due to the following match-case in ScoveragePlugin.scala:

case v: ValDef if v.symbol.isSynthetic => tree

Scala compiler compiles the pattern-matching assignment into the synthetic ValDef, so the above match-case causes scoverage to completely ignore that tree. For example:

User code

val (a, b) = {
    if (c) 1 -> 1 else 2 -> 2 // statements which should be instrumented
}

Compiled code

val x1 = (if (c) 1 -> 1 else 2 -> 2) match {
    case scala.Tuple2((a @ _), (b @ _)) => scala.Tuple2(a, b)
} // This is synthetic ValDef! Scoverage completely ignore this.
val a = x1._1
val b= x2._2

When scoverage encounters the val x1 = ..., it completely ignore this and doesn't instrument if (c) 1 -> 1 else 2 -> 2.

What

This PR fixes this by adding an additional match-case specifically to handle this case:

case v: ValDef if v.symbol.isSynthetic && v.rhs.pos.isDefined && containsNonSynthetic(v.rhs) =>
          treeCopy.ValDef(tree, v.mods, v.name, v.tpt, process(v.rhs))

In the example, val x1 = ... will fall into this case since it's a synthetic code whose RHS contains some user code. With this fix, scoverage will now consider if (c) 1 -> 1 else 2 -> 2 and include them in the coverage report.

Note

For the above example, this fix also unintentionally introduce another 2 measure statements inside the case scala.Tuple2((a @ _), (b @ _)) => scala.Tuple2(a, b).

IMO, This is also a synthetic code and should not be included in the coverage report. However, I don't enough confidence to exclude this out since it might accidentally exclude some valid codes out of the report. And since the consequence of having this additional measure statements is just the dilution of the %coverage with 2 always-hit/always-miss statements. It's fine to just ignore them.

[mekpavit]

@tgodzik This is the PR to fix this issue. I'm quite new to scoverage and scala compiler, so feel free to leave any comment if my understanding is wrong!

[tgodzik]

LGTM!

[mekpavit]

@tgodzik Thank you for reviewing/merging this quick. May I know when you will release this fix? Do you want me to bump the version in sbt-scoverage as well?

[tgodzik]

@tgodzik Thank you for reviewing/merging this quick. May I know when you will release this fix? Do you want me to bump the version in sbt-scoverage as well?

We can release soon, I usually try to gather a couple of fixes before releasing, but I guess there is not a lot of things to fix currently. I will do a release tomorrow then