Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add --release flag support #1473

Closed
wants to merge 1 commit into from
Closed

Add --release flag support #1473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types.DefaultTypeVisitor;
import com.sun.tools.javac.main.Arguments;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.parser.Tokens;
import com.sun.tools.javac.parser.Tokens.Comment;
import com.sun.tools.javac.parser.Tokens.TokenKind;
Expand Down Expand Up @@ -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")) {
Copy link
Collaborator

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.

Copy link
Contributor Author

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?

if (key.equals(Option.PLUGIN.getPrimaryName()) && value.startsWith("ErrorProne")) {
// When using the -Xplugin Error Prone integration, disable Error Prone for speculative
// recompiles to avoid infinite recursion.
continue;
}
if ((key.equals(Option.SOURCE.getPrimaryName()) || key.equals(Option.TARGET.getPrimaryName()))
&& originalOptions.isSet(Option.RELEASE)) {
Comment on lines +1032 to +1033
Copy link
Contributor Author

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.

// javac does not allow -source and -target to be specified explicitly when --release is,
// but does add them in response to passing --release. Here we invert that operation.
continue;
}
options.put(key, value);
}
JavacTask newTask =
Expand Down
27 changes: 27 additions & 0 deletions core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.junit.Assume.assumeTrue;

import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
Expand All @@ -46,6 +47,7 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.RuntimeVersion;
import com.sun.source.doctree.LinkTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
Expand Down Expand Up @@ -1042,6 +1044,31 @@ public void compilesWithFixTest() {
.doTest();
}

@Test
public void compilesWithFix_releaseFlag() {
assumeTrue(RuntimeVersion.isAtLeast9());
BugCheckerRefactoringTestHelper.newInstance(new CompilesWithFixChecker(), getClass())
.setArgs("--release", "9")
.addInputLines(
"in/Test.java",
"class Test {",
" void f() {",
" int x = 0;",
" int y = 1;",
" System.err.println(y);",
" }",
"}")
.addOutputLines(
"out/Test.java",
"class Test {",
" void f() {",
" int y = 1;",
" System.err.println(y);",
" }",
"}")
.doTest();
}

/** A test bugchecker that deletes an exception from throws. */
@BugPattern(
name = "RemovesExceptionChecker",
Expand Down