scoverage / gradle-scoverage

A plugin to enable the use of Scoverage in a gradle Scala project
Apache License 2.0
53 stars 38 forks source link

Support mixed Java/Scala modules using Java annotation processors #159

Closed grollinger closed 3 years ago

grollinger commented 3 years ago

We use an annotation processor (lombok) on some Java files in a combined Java/Scala module.

When we tried running scoverage on that module, we noticed that the compileScoverage* tasks failed because the annotation processor wasn't being run, even though the non-scoverage compile* tasks succeed.

This PR changes the scoverage compile task to inherit the annotationProcessorPath from the original compile task, in addition to compileClasspath and runtimeClasspath.

That fixes the compilation problem we observed.

I'm not sure this is the best solution to this problem, so please let me know if you can think of a better one.

eyalroth commented 3 years ago

Thank you for the contribution!

I'm a bit hesitant to add this to the project, for a few reasons:

  1. The more custom integrations with other tools/plugins, the harder it would become to maintain this project and its compatibility with the other tools.

  2. I'm not exactly sure about this, but It seems as though Lombok is reliant on java-compile tasks rather than scala-compile ones. The thing is, mixed java-scala projects preferably should not use the java-compile tasks at all, but rather defer all compilation to the scala-compile tasks. You can see this configured in the relevant functional test.

    Given that, I'm not even sure that Lombok is compatible with the preferred way of compiling mixed projects, regardless of the usage of scoverage. I would perhaps seek their help in first making it compatible, and that would perhaps solve the issue here as well.

  3. I'm not seeing the Lombok gradle plugin being configured in the PR functional tests, so I'm not sure it actually works.

grollinger commented 3 years ago
  1. I'm only using lombok as an example of a Java annotation processor. Any other Java annotation processor will have the same problem with gradle-scoverage tasks.
  2. Yes, Java annotation processors rely on the Java compile task. Given that there are some very nice libraries and frameworks that rely on Java annotation processors, I don't think it's too much to ask that scoverage should not break projects depending on them.
  3. Java annotation processors are natively supported by Gradle. There is no need for a lombok gradle plugin to make it work.

Given that, I'm not even sure that Lombok is compatible with the preferred way of compiling mixed projects, regardless of the usage of scoverage.

As you say, there are some things one needs to consider when using Java annotation processors. Because running the annotation processor is done by the Java compile task, you have to use that task to compile any source file that needs to use an annotation processor. For these source files, you won't be able to refer to classes implemented in Scala in the same module. That doesn't mean that you have to compile all Java sources in this module using the Java compile task. In fact, you can have the best of both worlds. We configure our build with a dedicated source directory for sources requiring an annotation processor src/main/java_lombok. All other Java sources in the module are still compiled using the compileScala task.

The default configuration of the Gradle Scala plugin is to compile Java with the compileJava task and Scala with the compileScala task. Otherwise, the scala-java-multi-module project wouldn't have to explicitly override the srcDirs for Java and Scala. So I think it's a bit of a stretch to say that what scala-java-multi-module does is the preferred way of configuring mixed modules. But even if we grant that most people will want to set up their mixed modules that way, I don't see why that should break the compile for people with different requirements.

eyalroth commented 3 years ago
  1. Java annotation processors are natively supported by Gradle. There is no need for a lombok gradle plugin to make it work.

I missed that part in their documentation. In any case, they do recommend using the gradle plugin (though I'm not sure that matters).

  1. I'm only using lombok as an example of a Java annotation processor. Any other Java annotation processor will have the same problem with gradle-scoverage tasks.

I'm not familiar with Java annotation processing, but from what I gather, it is a Java compiler feature which is not supported by the Scala compiler (without even considering scoverage). See this question in the Gradle forums (though it's an old question and things might have changed) and this relatively recent issue in the gradle repostiory.

Furthermore, I see here a mentioning of Lombok being an exceptional annotation processor (I'm not sure it matters):

An important thing to note is the limitation of the annotation processing API — it can only be used to generate new files, not to change existing ones.

The notable exception is the Lombok library which uses annotation processing as a bootstrapping mechanism to include itself into the compilation process and modify the AST via some internal compiler APIs. This hacky technique has nothing to do with the intended purpose of annotation processing and therefore is not discussed in this article.

In any case, I'm not sure how does coverage breaks your build if you have a separate source directory with all the sources that require annotation processing. If I'm not mistaken, you could define a custom java compilation task to compile these source files, and make the scala compilation task (not even scoverage) depend on it. Would this not work?

grollinger commented 3 years ago

In any case, I'm not sure how does coverage breaks your build if you have a separate source directory with all the sources that require annotation processing.

It does that, because it copies the javaCompile task to javaScoverageCompile but doesn't copy over all the necessary configuration to make it work (i.e. the annotationProcessorPath) so the new scoverage task javaScoverageCompile is now broken. When scoverage compilation is run, therefore, it tries to compile the Java sources using the java compiler but without running the annotation processor.

You can see that in action when you revert the line I added to copy over the annotationProcessorPath and run the new functionalTest:

./gradlew functionalTest --tests "*ScalaJavaAnnotationProcessorTest*"

it fails with the following error:

  > Task :mixed_scala_java:compileJava
    > Task :mixed_scala_java:compileScala
    > Task :mixed_scala_java:compileScoverageJava
    > Task :mixed_scala_java:compileScoverageScala     FAILED

    [Error] /gradle-scoverage/build/resources/functionalTest/projects/scala-java-annotation-processor/mixed_scala_java/src/main/scala/org/hello/WorldScala.scala:6: value foo is not a member of org.hello.WorldJava

As you can see, the non-scoverage tasks compileJava and compileScala worked fine. But when compileScoverageScala is run, it tries to access a method foo() that should have been produced by compileScoverageJava by running the annotation processor. That method is not present because the annotation processor wasn't run.

If I'm not mistaken, you could define a custom java compilation task to compile these source files, and make the scala compilation task (not even scoverage) depend on it. Would this not work?

That might well work, but it would be a workaround. If you say that we shouldn't rely on the Java compile task, then gradle-scoverage shouldn't either, and it shouldn't try to (incompletely) re-create that task. Why should we have to create a custom task instead of using the Java compile task as it was intended to be used?

eyalroth commented 3 years ago

I see now. Thank you for being patience and working hard on explaining the situation at hand. Overall I'm happy that we have some sort of documentation for this issue.

May I add two requests to the PR?

  1. Can you confirm that the project passes all tests (./gradlew check) if you haven't already? It seems like Travis CI is no longer working for the project.
  2. In src/functionalTest/resources/projects/scala-java-annotation-processor/mixed_scala_java/build.gradle - could you add the configureSources part like in the scala-java-multi-module project, but commented out and with a comment explaining that this practice is incompatible with Java's annotation processing?
grollinger commented 3 years ago

Can you confirm that the project passes all tests (./gradlew check) if you haven't already? It seems like Travis CI is no longer working for the project.

Already done. But it seems CI is working again.

In src/functionalTest/resources/projects/scala-java-annotation-processor/mixed_scala_java/build.gradle - could you add the configureSources part like in the scala-java-multi-module project, but commented out and with a comment explaining that this practice is incompatible with Java's annotation processing?

Done. If you'd like to phrase the comment differently, just let me know.

eyalroth commented 3 years ago

Done. If you'd like to phrase the comment differently, just let me know.

It's looking good :)

maiflai commented 3 years ago

deployed 6.1.0 with this PR - thanks both