tbroyer / gradle-errorprone-plugin

Gradle plugin to use the error-prone compiler for Java
Apache License 2.0
366 stars 32 forks source link

GroovyCompile tasks support #63

Closed remal closed 2 years ago

remal commented 3 years ago

Gradle groovy plugin supports Groovy joint compilation. It means that Gradle allows you to mix Groovy and Java code in one source dir.

GroovyCompile has options property of type CompileOptions. It's the same type, as used for JavaCompile tasks.

Could you please add the same Error Prone tweaks for all GroovyCompile tasks?

tbroyer commented 3 years ago

Oh, looks like it actually works, and for Scala joint compilation too! Will work on adding that, but please note that I'm using neither Groovy nor Scala so this will be a "best effort".

(I'd love to support Kotlin too BTW)

remal commented 3 years ago

@tbroyer I don't think it will be possible to support other JVM languages, as Java annotation processing is specifically intended for Java. Other languages like Groovy and Kotlin generate stubs (methods without code) and run annotation processors for them. It works for code generation (Dagger 2 for example), but other processors most likely fail.

But most JVM languages support interop with Java. So we can attach Error Prone to that part of code that is written in Java. Kotlin doesn't require any changes in this plugin, as Kotlin compilation targets only Kotlin and Java code is compiled by JavaCompile tasks. But Groovy can process Java code that is placed in Groovy folder like this:

src/main/java/pgk/SomeClass.java - will be handled by this plugin
src/main/groovy/pkg/OtherJavaClass.java - if I understand correctly, won't be handled by this plugin
src/main/groovy/pkg/GroovyClass.groovy - Groovy sources, which are not supported by Error Prone

In example above src/main/groovy/pkg/OtherJavaClass.java will be handed by GroovyCompile task, which executes Java compiler for Java sources. And we can attach Error Prone processor for such sources.

You may ask what is the reason to place Java classes into groovy folder? The answer is quite simple: this is the only way to use Groovy classes in Java sources, because by default in Gradle it's Groovy sources that depend on Java and not vise-versa. So, if your goal is to write most of the code in Java and only a small part of it in Groovy, you need to place everything in groovy folder.

tbroyer commented 3 years ago

You may ask what is the reason to place Java classes into groovy folder? The answer is quite simple: this is the only way to use Groovy classes in Java sources, because by default in Gradle it's Groovy sources that depend on Java and not vise-versa. So, if your goal is to write most of the code in Java and only a small part of it in Groovy, you need to place everything in groovy folder.

AFAIK, that's not true since Gradle 6.1: https://docs.gradle.org/6.1/release-notes.html#defining-compilation-order-between-groovy,-scala-and-java

The reason for putting Java classes in groovy or scala (or kotlin) source directory sets is to benefit from joint compilation, where you can have dependency cycles between Java and that other language. I did try it with an interface in Groovy/Scala, implemented in a Java class (that purposefully fails an ErrorProne check), that's instantiated from a Groovy/Scala class. It looks like Kotlin supports joint compilation too (i.e. it correctly compiled such code, I didn't try to plug ErrorProne in there to make it fail though, as Kotlin tasks are much different from Groovy/Scala ones)

From what you say, it looks like you might be able to reverse the Groovy/Java dependency in your case, move all your Java classes to src/main/java, and thus use the plugin without modification.

remal commented 3 years ago

Thanks for the link to Gradle docs, I'll definitely take a look. You're absolutely right about joint compilation and, as GroovyCompile executes Java compiler, I thought it makes sense to make this plugin to work with GroovyCompile tasks too.

remal commented 3 years ago

BTW I've just checked and I found out that I use joint compilation, so I'll be really grateful if you implement this issue.

tbroyer commented 2 years ago

I'm sorry but this complicates the code too much. This could be built in a companion plugin though if you still need it.