Skip to content

Fork removeUnusedImports() and googleJavaFormat() to support later JLS than the JRE used for the build #713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
marcphilipp opened this issue Oct 10, 2020 · 7 comments

Comments

@marcphilipp
Copy link

marcphilipp commented Oct 10, 2020

EDIT by @nedtwigg: summarizing the current situation, easy workaround, and a hard-to-implement potential fix:

If you use googleJavaFormat() or removeUnusedImports(), and you also use cutting edge java language features such as the Java 15 text blocks, then you must also run your build on JRE 15+ (same for language features in any other release, e.g. must use JRE 14+ to use Java 14 language features, etc.). This is because google-java-format selects which JLS specification to use based on which version of JVM it is running on.

The easy solution is to run your build with a JVM at least as new as the code you are formatting. The hard solution is to add a feature to Spotless which allows forking a more modern JVM just to run formatting. Happy to take a PR for this feature, but we have no plans to implement the feature ourselves.


When trying to use removeUnusedImports with a Java file that contains a Java 15 text block, the following exception is thrown:
https://ge.junit.org/s/5jcvukwcdcckg/console-log?task=:junit-jupiter-engine:spotlessJava

Config:
https://github.com/junit-team/junit5/blob/4dd6981ece3a5d5275844ba6d9b4fa7ab60bf0af/build.gradle.kts#L134-L154

@sormuras
Copy link
Contributor

I guess, an outdated GJF is bundled. Perhaps an upgrade to 1.9 does help?

https://github.com/google/google-java-format/releases

@jbduncan
Copy link
Member

When using the googleJavaFormat() step, by default we import 1.7 if the JDK that Spotless is running on is JDK >=8, or 1.9 if it's JDK >=11. This is because GJF 1.8+ now requires JDK 11. However, I don't know how we deal with removeUnusedImports(), as I don't have an IDE in front of me at the moment...

@nedtwigg Do you happen to know which version we use for removeUnusedImports()? Regardless, I can think of a few potential fixes:

  • Import GJF 1.7/1.9 depending on the JDK
  • Allow the user to choose which version of GJF to use
  • Find an alternative to GJF that's Java 8-compatible. Maybe Eclipse has something here? But I recall @fvgh saying that it requires JDK 11 now...

Any thoughts on this?

@nedtwigg
Copy link
Member

We do use the latest 1.9 when run on Java 11+. But, gjf picks formatting rules based on the JVM it's running on. So if you want to format Java 15 constructs, you need to run the build on Java 15.

@marcphilipp
Copy link
Author

Does gjf run in the build VM or a forked one?

@nedtwigg
Copy link
Member

The build VM. Not forked. Happy to take a PR which adds that feature though.

@marcphilipp
Copy link
Author

For the time being, I made it optional depending on the Java version the build is running on.

@nedtwigg nedtwigg changed the title removeUnusedImports is incompatible with Java 15 text blocks Fork removeUnusedImports() and googleJavaFormat() to support later JLS Oct 11, 2020
@nedtwigg nedtwigg changed the title Fork removeUnusedImports() and googleJavaFormat() to support later JLS Fork removeUnusedImports() and googleJavaFormat() to support later JLS than the JRE used for the build Oct 11, 2020
@nedtwigg
Copy link
Member

This issue was first, but I'm going to close it as a dupe of #724, because that issue is more general-purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants