palantir-java-format icon indicating copy to clipboard operation
palantir-java-format copied to clipboard

Formatter's behaviour depends on Gradle runtime's JDK version, not the toolchain's version

Open j-baker opened this issue 3 years ago • 2 comments

What happened?

With usage of Gradle toolchains, it's much more common for the Java version used for Gradle to substantially differ from the Java version used for compiling and running code.

What I observed is that formatting behaviour depended heavily on the JDK used to run Gradle. For example, if one runs Gradle with Java 11, the formatter will be unable to handle switch expressions, even though Gradle itself will happily compile the Java code, e.g.

19:17: error: illegal start of expression
com.palantir.javaformat.java.FormatterException: 19:17: error: illegal start of expression
        at com.palantir.javaformat.java.FormatterExceptions.fromJavacDiagnostics(FormatterExceptions.java:28)
        at com.palantir.javaformat.java.Formatter.parseJcCompilationUnit(Formatter.java:206)
        at com.palantir.javaformat.java.RemoveUnusedImports.parse(RemoveUnusedImports.java:214)
        at com.palantir.javaformat.java.RemoveUnusedImports.removeUnusedImports(RemoveUnusedImports.java:202)
        at com.palantir.javaformat.java.Formatter.formatSourceAndFixImports(Formatter.java:268)
        at com.palantir.javaformat.java.FormatterServiceImpl.formatSourceReflowStringsAndFixImports(FormatterServiceImpl.java:43)

but once JAVA_HOME is pointed at a Java version that supports switch expressions, formatting is successful.

What did you want to happen?

Preferred: Formatter respects the toolchain defined for the sourceset. Acceptable: Formatter warns when the toolchain is incompatible.

j-baker avatar Aug 03 '22 11:08 j-baker

In IntelliJ it looks like this problem does not exist because of the 'bootstrapping formatter'.

j-baker avatar Aug 03 '22 14:08 j-baker

I think solving this is probably also the right solution to the Gradle 16+ module access problem. In Spotless, they solve the problem by using Unsafe to make the modules be accessible at runtime, which seems fragile with regards to future JDK changes. If PJF were to fork an appropriate JVM (like the compiler, test, javaexec, checkstyle tasks), it would be able to add the appropriate JVM args at fork time and get itself going properly.

j-baker avatar Aug 19 '22 09:08 j-baker