Add in initial support for code coverage

[ckipp01]

ℹ️ initial description kept for reference, see the follow-up for recent changes

This pr adds in the initial groundwork for code coverage as a compiler phase in response to the conversation here. Much of the work that is here was actually done as a POC by @Qjaquier a few years ago, and this basically continues the great work he started. There is a report that was written that explains much of this approach here as well.

This approach aims to inline the code coverage process up until writing instrumentation to disk, and then the rest of the process from reading up the coverage data, aggregating it, and reporting on it can now all be shared. This logic was de-coupled from the Scala 2 compiler plugin in scalac-scoverage-plugin and is already published as a milestone release to be tested with this. The same goes for the sbt-scoverage sbt plugin. You can read more about the changes that were necessary to make that work here. Most of the work up to this point was actually done there, not in this pr.

The way this works

The give a brief summary of how this will all work together, the initial step would be that the user (through a plugin) would enabled a -coverage flag, and then provide the output directory and also the sourceroot directory (if necessary, most of the time the default works fine). You can see how this is done here in the sbt plugin:

https://github.com/scoverage/sbt-scoverage/blob/d7df279c0a738988b770e529e5b4ba0454170ad6/src/main/scala/scoverage/ScoverageSbtPlugin.scala#L159-L168

Having the -coverage flag will enable the phase defined in CoverageTransformMacro.scala, which takes care of instrumenting the code, which will call out to Invoker.scala which takes care of serializing and writing the coverage data. Don't be alarmed by the diff count here, as 3k lines is actually the expect file, which I'll explain down below since I do have reservations about testing this way. At this point, after the coverage data is written, scoverage will take over and is published for 2.12, 2,13, and 3 (the domain, serializer, and reporter artifacts).

Trying this out

In order to try this out, you'll need to publish this branch locally. After doing that you can utilize the milestone releases via the sbt plugin to test. In order to make this easy I have a couple set up repos to that you can play around with:

• https://github.com/ckipp01/scala3-example-project (forked from the scala3-example-project with tests added)
• https://github.com/ckipp01/pool-party (forked from @tpolecat and hardcoded to the snapshot version of Scala 3 and coverage added)

Stuff left to figure out

At this point, I'm sort of hitting on the ends of my compiler knowledge and would love some insight into a couple things. My goal ultimately is to try and get this in the compiler not with a full feature-set covering everything, but with some workable basics that we can iterate over, and that hopefully others can help with 😄.

Testing

I tried to follow the way semanticDB is tested. You can do very similiar things:

Regenerate your expect file

scala3-compiler-bootstrapped/test:runMain dotty.tools.dotc.coverage.updateExpect

Run the tests with the bootstrapped compiler

scala3-compiler-bootstrapped/testOnly dotty.tools.dotc.coverage.CoverageTests

While this does work ok and I do think it's important to have some sort of test like this, this produces a giant single file, so any changes make finding what changed pretty awful. There should be a better diff functionality if we stick with this. There is also probably a lot more things we should test, but just to start I just threw in some of the examples from the scala3-example-project

Re-running coverage in sbt shell

One thing I don't fully know how to change is that in Scala 2-land you could enter the interactive sbt shell, run coverage, test, clean and then test again, and the second test would work as expected and again write your instrumentation to disk. The Invoker is short lived, and goes away after each test run. This doesn't work the same here as the Invoker seems to stick around when in interactive mode. So it ends up keeping the state from the last test and won't write to disk again. I'm not fully sure how to get around this or if there is some sort of hook that I can tie into and clean up the invoker to ensure that the next run will work as expected. Again, this only pops up when you are in a long running session. In CI or one-off runs don't hit on this.

Unhandled trees

The set of trees that are instrumented probably aren't complete. I left a warning in there just to show what I mean. For example if you run this you'll see warnings about constructs that aren't handled. There still needs to be a bit of work about which ones are fine not to handle and which ones need to be handled.

Bare bones

For the moment, this is a pretty bare bones feature set. Some things that will likely need to be added in the future are things like ignoring packages, files, symbols. All things that exist in the Scala 2 version, but not yet here.

TODOs

Just to keep track of a few things

• Test the Invoker
• Finalize how we test the coverage data, is what we have ok?
• What other trees need to be handled before merging
• Add some documentation

All in all, I'd love some feedback on this. I've learned a lot diving into this, and I'm excited to hopefully get this into the compiler. I admire the desire of the Dotty team to upstream as much as possible. It'd a decision that will have huge beneficial ramifications for the future of Scala.

[ckipp01]

I wanted to follow back up on this. I'm sort of waiting on some feedback before bringing this any further. Mainly is the approach ok, any thoughts on the testing questions I brought up, etc. Once I sort of get the green light I can then address whatever we feel is remaining in order to get this to a point that it's mergeable.

[ches]

Perhaps this remark belongs in Discourse if there is a need for discussion about whether it belongs in the compiler or whether the community can devise an interim migration plan for scoverage until it gets there (including 3.{0,1}.x), but I'd just like to add in support:

In my large organization with Scala projects/services reaching hundreds, scoverage is a quality assurance standard. Coverage is an imperative piece of the tooling story for industrial Scala 3 migration.

Kudos to @ckipp01 for pushing the matter forward!

[ckipp01]

So just to be transparent I probably won't be working on this any time soon. It is quite close to being finished, there are already Milestone releases on both the sbt scoverage plugin and scoverage as outlined in the description, so if someone does want to pick this up, please do. I'll do whatever I can with helping getting the scoverage part reviewed and released.

[anatoliykmetyuk]

@ckipp01 would you be interested to mentor someone on the Issue Spree on this one?

[ckipp01]

@ckipp01 would you be interested to mentor someone on the Issue Spree on this one?

Hey @anatoliykmetyuk for sure. However, I think it'd be best to also have someone that knows the actual compiler better than myself also be part of it. The majority of the actual scoverage part is done, it's mainly finishing up dealing with the compiler parts.

[smarter]

For anyone interested in working on this, note that Chris gave a talk on this work at ScalaCon which is now online! https://www.youtube.com/watch?v=SIkNgemGmYQ

[andreisilviudragnea]

Does this implementation support interpolated strings too? The Scala 2 one does not: scoverage/scalac-scoverage-plugin#448

[TheElectronWill]

I intend to finish this!

TODO:

• make it compile on latest main
• cleanup phase definition (InstrumentCoverage)
• try new, simpler strategy for handling function calls
• handle more trees
• see if something can be done about interpolated strings
• avoid global state
• test (any advice on how to replace the big "expect" file, if it's not the best solution?)
• fix the tastyBootstrapTest (and, to do so, improve error reporting)
• do something about the Invoker's code being 8 years old and not counting properly?: some solutions (to fix the counting) would require to modify scoverage, we'll see
• fix weird bugs found on the way
• optimize: instrument literals only where necessary
• add Synthetic flag to trees where necessary

The coverage option is now named -coverage-out, and coverage-sourceroot is removed (you can use the existing option -sourceroot).

[TheElectronWill]

Update: I'm digging into the tastyBootstrapTest to understand why it fails. Once it's fixed, I'll mark this PR as ready.

I have some proposals to improve the coverage, mainly to make the Invoker count correctly while improving the performance. This require some discussion with the scoverage folks.

The idea is:

1. Since we generate the lists of statements that can be covered, we know the id range 0...N in advance. Thus, we could use an AtomicIntegerArray per directory, to count how many times each statement is called.

2. Since scoverage controls the sbt plugin, we can know when the coverage checks start and end, much more reliably than the JVM shutdown hooks (I've seen a comment in the coverage code somewhere about shutdown hooks). Therefore, we can pass N to the invoker at the beginning of the coverage task, and tell the Invoker to dump all its data at the end.

Benefits:

• accurate count per statement
• maximal performance, both for counting and for saving the results

ping @ckipp01 Does it seem possible? Is it quick to implement?

Maybe it's best if I finish this PR with an incorrect count (as incorrect as the scoverage for Scala 2), and open others for the new improvements.

[ckipp01]

ping @ckipp01 Does it seem possible? Is it quick to implement?

Maybe it's best if I finish this PR with an incorrect count (as incorrect as the scoverage for Scala 2), and open others for the new improvements.

To be honest I'm not really sure without digging into it. As for incorrect counts in general, I'll be honest, I'm not really sure how large of an issue this is, since I'm unsure if there is actually a lot of complaints or issues related to this. After taking over releases and small maintenance tasks with scoverage, I've never actually done a deep dive into some of these issues.

[godenji]

Maybe it's best if I finish this PR with an incorrect count (as incorrect as the scoverage for Scala 2), and open others for the new improvements.

👍 the sooner we have usable Scala 3 test coverage reports the better (i.e. don't let perfect be the enemy of the good).

Thanks for pushing this to the finish line @TheElectronWill !!

[smarter]

LGTM, huge thanks to everyone who contributed! 🎆

[ckipp01]

Really fantastic job bringing this across the finish line @TheElectronWill 🚀

[TheElectronWill]

Thank you for the initial work!

[jozic]

Hello folks, are there any plans to support exclusion from coverage feature from original scoverage?
If not, where should one start looking to make it happen?

[ckipp01]

@jozic I don't see a feature request for it yet, so I'd recommend creating one for it in this repo.

[TheElectronWill]

I agree, a new issue should be created for it. It shouldn't be too hard to implement, I think :)

[jozic]

#17684