-
Notifications
You must be signed in to change notification settings - Fork 745
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
Add --release
flag support
#1473
Conversation
The java compiler does not allow `-source` and `-target` to be specified explicitly when `--release` is [1], but does add them in response to passing `--release` [2]. As such `SuggestedFixes#compilesWithFix` must compensate for this by removing `-source` and `-target` before triggering another compilation round. Fixes google#1265. [1] https://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#l297 [2] https://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#l315
if ((key.equals(Option.SOURCE.getPrimaryName()) || key.equals(Option.TARGET.getPrimaryName())) | ||
&& originalOptions.isSet(Option.RELEASE)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code can be optimized by factoring out originalOptions.isSet(Option.RELEASE)
and creating a set containing Option.SOURCE.getPrimaryName()
and Option.TARGET.getPrimaryName()
. Decided against this because I figured it'd make this already-very-long method even more complex to follow. Let me know if you feel differently and I'll change it.
@@ -1023,11 +1024,17 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept | |||
Options originalOptions = Options.instance(javacTask.getContext()); | |||
for (String key : originalOptions.keySet()) { | |||
String value = originalOptions.get(key); | |||
if (key.equals("-Xplugin:") && value.startsWith("ErrorProne")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather use the string names of the flags here. com.sun.tools.javac.main.Optionn
is an unsupported internal API that has changed in the past, and the literal flag names are part of javac's supported API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack! Since you assigned the PR to yourself: want me to update this, or have you already applied those changes internally?
The java compiler does not allow `-source` and `-target` to be specified explicitly when `--release` is [1], but does add them in response to passing `--release` [2]. As such `SuggestedFixes#compilesWithFix` must compensate for this by removing `-source` and `-target` before triggering another compilation round. Fixes #1265. [1] https://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#l297 Fixes #1473 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=311235043
The java compiler does not allow `-source` and `-target` to be specified explicitly when `--release` is [1], but does add them in response to passing `--release` [2]. As such `SuggestedFixes#compilesWithFix` must compensate for this by removing `-source` and `-target` before triggering another compilation round. Fixes #1265. [1] https://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#l297 Fixes #1473 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=311235043
The java compiler does not allow
-source
and-target
to be specified explicitly when--release
is, but does add them in response to passing--release
. As suchSuggestedFixes#compilesWithFix
must compensate for this by removing-source
and-target
before triggering another compilation round.Fixes #1265.