bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
364 stars 275 forks source link

java.lang.NullPointerException throw in coverage for Scala 2.13 #1291

Open gergelyfabian opened 3 years ago

gergelyfabian commented 3 years ago

I have a project where I have set up latest rules_scala with Scala 2.13.

rules_scala_version = "0f38f217d1313d564bcc6c00976551e775be0ade"  # update this as needed

http_archive(
    name = "io_bazel_rules_scala",
    strip_prefix = "rules_scala-%s" % rules_scala_version,
    type = "zip",
    url = "https://github.com/bazelbuild/rules_scala/archive/%s.zip" % rules_scala_version,
)

...

scala_config(scala_version = "2.13.2")

If I run coverage on this project with:

bazel coverage //...

Then it fails with:

java.io.IOException: Error while analyzing {my-secret-classname}.class.uninstrumented.
    at org.jacoco.core.analysis.Analyzer.analyzerError(Analyzer.java:168)
    at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:140)
    at com.google.testing.coverage.JacocoCoverageRunner.analyzeStructure(JacocoCoverageRunner.java:180)
    at com.google.testing.coverage.JacocoCoverageRunner.create(JacocoCoverageRunner.java:125)
    at com.google.testing.coverage.JacocoCoverageRunner$2.run(JacocoCoverageRunner.java:545)
Caused by: java.lang.NullPointerException
    at org.jacoco.core.internal.analysis.MethodCoverageCalculator.ignore(MethodCoverageCalculator.java:158)
    at org.jacoco.core.internal.analysis.filter.KotlinWhenStringFilter$Matcher.match(KotlinWhenStringFilter.java:96)
    at org.jacoco.core.internal.analysis.filter.KotlinWhenStringFilter.filter(KotlinWhenStringFilter.java:37)
    at org.jacoco.core.internal.analysis.filter.Filters.filter(Filters.java:57)
    at org.jacoco.core.internal.analysis.ClassAnalyzer.addMethodCoverage(ClassAnalyzer.java:110)
    at org.jacoco.core.internal.analysis.ClassAnalyzer.access$100(ClassAnalyzer.java:31)
    at org.jacoco.core.internal.analysis.ClassAnalyzer$1.accept(ClassAnalyzer.java:99)
    at org.jacoco.core.internal.flow.ClassProbesAdapter$2.visitEnd(ClassProbesAdapter.java:89)
    at com.google.testing.coverage.jarjar.org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1491)
    at com.google.testing.coverage.jarjar.org.objectweb.asm.ClassReader.accept(ClassReader.java:717)
    at com.google.testing.coverage.jarjar.org.objectweb.asm.ClassReader.accept(ClassReader.java:401)
    at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:122)
    at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:138)

The interesting thing is, that if I exchange rules_scala version to 67a7ac178a73d1d5ff4c2b0663a8eda6dfcbbc56, then the error is gone (this is the last version I could make it work).

Bazel version: 4.1.0 (tested on also other Bazel versions, like 3.7.0, and it's the same error).

liucijus commented 3 years ago

Thanks for reporting @gergelyfabian! Will you be able to provide a repro, or a fix?

gergelyfabian commented 3 years ago

Hi @liucijus :) The strange thing is, that in my example repo (https://github.com/gergelyfabian/bazel-scala-example) I cannot reproduce this error (I've just upgraded it to the latest rules_scala and Scala 2.13). Will try to create similar code there that can reproduce this issue.

gergelyfabian commented 3 years ago

I think I have the reproduction, just let me clean it up, and will upload it as a branch to my example repo.

gergelyfabian commented 3 years ago

Here is the reproduction: https://github.com/gergelyfabian/bazel-scala-example/blob/rules_scala_1291/example-maven/src/main/scala/mypackage/Maven.scala

I think it's the following construct that leads Jacoco to run a Kotlin when filter on the Scala code:

    literal match {
      case s if s == "true" => true
      case s if s == "false" => false
      case _ => throw new Exception("Boooom!")
    }

What's very interesting, my first attempt was to apply a custom Jacoco runner (and fix the error in Jacoco by changing the Kotlin when filter), but I could not get it working with the latest rules_scala version. Then, when I reverted rules_scala to the older version (67a7ac1) I discovered that there is no issue there at all. So maybe the Jacoco version used by rules_scala is a proper one in the older version and it may be improper in the new version? (especially that in the new version it seems that switching jacocorunner does not change anything)

EDIT: I wasn't using toolchains properly (was missing the extra toolchain parameter when executing coverage) that's why I could not exchange jacocorunner to a custom version with rules_scala.

gergelyfabian commented 3 years ago

Unfortunately reverting rules_scala to 67a7ac1 is not a solution. As I just discovered in that case there are no coverage metrics produced (maybe coverage is not called then at all?). With current master coverage is produced, but it fails when running this Kotlin When filter for this specific code.

liucijus commented 3 years ago

Then, when I reverted rules_scala to the older version (67a7ac1) I discovered that there is no issue there at all.

This is the last commit which requires coverage to be enabled on the toolchain, that's why it works. Switch it on with register_toolchains("@io_bazel_rules_scala//scala:code_coverage_toolchain") and it will fail the same way.

gergelyfabian commented 3 years ago

It seems this issue is specific to Scala 2.13. If I revert in my test branch (rules_scala_1291) the commit that upgraded to Scala 2.13 (git revert 46b97007f37b590833d6cd017ed628db387798b7), then with Scala 2.12 coverage works.

gergelyfabian commented 3 years ago

Uploaded a new branch to: https://github.com/gergelyfabian/bazel-scala-example/tree/jacoco_coverage_fix_new, this does jacocorunner change properly (using current rules_scala master). Created new branch that includes both the reproduction of the problem in this issue and jacoco_coverage_fix_new: https://github.com/gergelyfabian/bazel-scala-example/tree/rules_scala_1291_fixed.

If you build custom jacocorunner from a fixed Jacoco version (more on that later), then the issue is fixed.

gergelyfabian commented 3 years ago

Fix/workaround is in branches:

The first branch is automatically used in rules_scala's scripts/build_jacocorunner/build_jacocorunner.sh script, so you should have it if you use the built custom JacocoRunner (at least works for me).

gergelyfabian commented 3 years ago

According to the discussion in https://github.com/jacoco/jacoco/pull/1224 it will be enough to backport https://github.com/jacoco/jacoco/pull/942 in the custom 0.8.3 branches that I use to fix this issue (I have tested that, and it works). Will update my branches with this backport (instead of turning off parsing Scala files in the Kotlin when filter).

gergelyfabian commented 3 years ago

Updated branches:

The fix is in version 0.8.5 of jacoco, so any Bazel upgrade that will upgrade Jacoco to 0.8.5+ will fix this problem. Till that time one can use the scripts/build_jacocorunner/build_jacocorunner.sh script to build a JacocoRunner that contains the fix and use that with a custom scala_toolchain (do not forget to specify the toolchain in your .bazelrc or command line).

liucijus commented 3 years ago

I've tested it with https://github.com/bazelbuild/bazel/commit/9a061637bb2cd0ffde4a7300de5bbe0ef5df2daa and it passes without custom jacoco, so I guess we need to verify if it works with the next release of Bazel, and if not, ask to include latest java_tools build into it.

gergelyfabian commented 3 years ago

I checked with Bazel 5.0.0-pre.20210913.1 (updated in .bazelversion), and it passes without custom jacoco.

gergelyfabian commented 3 years ago

I think why it's passing is because on Bazel master Jacoco has been upgraded to 0.8.6 (includes a lot of useful fixes).