-
Notifications
You must be signed in to change notification settings - Fork 855
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
Java 17 compatibility #612
Comments
You'll need to add the |
I am using google-java-format via spotless so I can workaround this with --add-exports on MAVEN_OPTS as suggested also here diffplug/spotless#834 Is there a way this can be fixed out of the box for users of the library or would it require the full Java module stuff? |
I don't know any good alternatives: |
Thanks @cushon ! |
What's the long-term solution?
|
|
Are you effectively saying that google-java-format intentionally depends on JDK internals, accepting the risk that it can break any time a new JDK version comes out? |
With Java 17 GA only 6 days away, some further insight on your plans to address this issue would be greatly appreciated. |
The plan is to continue using JDK 17 should already work and is being tested on CI, if you see issues with it or other upcoming versions please report them. |
It seems pretty unreasonable to require all users to jump through that sort of hoop. |
Any hope this bug gets fixed? JVM options are not an option in a lot of environments so we need an actual fix as other checkers did probably. |
As of ee81afc, the flags aren't needed when invoking the CLI using
Are there any specific suggestions about alternatives, or fixes that were used in other checkers? |
IIRC the workarounds were to either rewrite the related code to not depends on internals of the JDK or to force the modules to be opened programmatically (something like https://github.com/openjdk/jdk/blob/5a7f22ab94bff4899b2f36ba27ea120ee7d8f1db/src/java.base/share/classes/java/lang/Module.java#L862) but I don't have all the details handy anymore. Think lombok and error prone got this issue too and solved it. |
It would be possible to port the formatter to a different front-end instead of using javac's parser. That would be a principled solution, but there are costs to making the switch and then maintaining a different parser or relying on something that may not get updated to support new language features as quickly as javac.
That looks like an implementation detail of
That isn't the case for Error Prone, see the mention of Lombok is using unsupported APIs to disable the module system encapsulation. |
@cushon I fear the supported ways will rely on JPMS API and this one will not be available in most tools except the custom launcher option. Agree lombok uses unsupported API but it works. Another option would be to force all integrators to fork but it would be quite a regression IMHO in terms of behavior so having a way to run smoothly without hacking the JVM (which has side effects on other plugins/tasks) would be great even if it requires a few more frequent release probably (one per 6 months?). |
For now, and this is only temporary (see linked discussion on Lombok issue tracker) |
Sorry, it seems that Error Prone suffers the same problem. |
First of all sorry to comment on this closed issue. Sharing this info as I think might be useful for fixing the manual configuration. This is how kotlin solved this issue for kapt - JetBrains/kotlin@52e4506 |
I've just tried
|
@lazystone See Spotless' documentation: https://github.com/diffplug/spotless/tree/main/plugin-gradle#google-java-format |
@tbroyer Ah, I just saw that PR above mentions this issue, so I thought that it's fixed. But now docs says "This is a workaround to a diffplug/spotless#834." which is also closed... |
It seems that Java 17 (next LTS) will have stricter access requirements
https://openjdk.java.net/jeps/403
I tried to run this tool and noticed this error:
The text was updated successfully, but these errors were encountered: