Skip to content

Commit

Permalink
SuggestedFixes: refactor the compilesWithFix method so checks can mak…
Browse files Browse the repository at this point in the history 
…e use of the diagnostics from the compiler.

PiperOrigin-RevId: 404177580
  • Loading branch information
@awturner
awturner authored and Error Prone Team committed Oct 19, 2021
1 parent 543967b commit c996313
Showing 1 changed file with 128 additions and 67 deletions.
195 changes: 128 additions & 67 deletions check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java
View file
Expand Up @@ -34,6 +34,7 @@
import static com.sun.tools.javac.util.Position.NOPOS;
import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Objects;
Expand All @@ -51,6 +52,7 @@
import com.google.errorprone.apply.DescriptionBasedDiff;
import com.google.errorprone.apply.ImportOrganizer;
import com.google.errorprone.apply.SourceFile;
import com.google.errorprone.fixes.SuggestedFixes.FixCompiler.Result;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.ErrorProneToken;
import com.google.errorprone.util.FindIdentifiers;
Expand Down Expand Up @@ -122,6 +124,7 @@
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.IntStream;
import java.util.stream.StreamSupport;
import javax.annotation.Nullable;
import javax.lang.model.element.Element;
Expand Down Expand Up @@ -1225,76 +1228,16 @@ private static boolean compilesWithFix(
return true;
}

JCCompilationUnit compilationUnit = (JCCompilationUnit) state.getPath().getCompilationUnit();
JavaFileObject modifiedFile = compilationUnit.getSourceFile();
BasicJavacTask javacTask = (BasicJavacTask) state.context.get(JavacTask.class);
if (javacTask == null) {
throw new IllegalArgumentException("No JavacTask in context.");
}
Arguments arguments = Arguments.instance(javacTask.getContext());
List<JavaFileObject> fileObjects = new ArrayList<>(arguments.getFileObjects());
URI modifiedFileUri = modifiedFile.toUri();
for (int i = 0; i < fileObjects.size(); i++) {
final JavaFileObject oldFile = fileObjects.get(i);
if (modifiedFileUri.equals(oldFile.toUri())) {
DescriptionBasedDiff diff =
DescriptionBasedDiff.create(compilationUnit, ImportOrganizer.STATIC_FIRST_ORGANIZER);
diff.handleFix(fix);
SourceFile fixSource;
try {
fixSource =
new SourceFile(
modifiedFile.getName(),
modifiedFile.getCharContent(false /*ignoreEncodingErrors*/));
} catch (IOException e) {
return false;
}
diff.applyDifferences(fixSource);
fileObjects.set(
i,
new SimpleJavaFileObject(sourceURI(modifiedFile.toUri()), Kind.SOURCE) {
@Override
public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
return fixSource.getAsSequence();
}
});
break;
}
}
DiagnosticCollector<JavaFileObject> diagnosticListener = new DiagnosticCollector<>();
Context context = new Context();
Options options = Options.instance(context);
Options originalOptions = Options.instance(javacTask.getContext());
for (String key : originalOptions.keySet()) {
String value = originalOptions.get(key);
if (key.equals("-Xplugin:") && value.startsWith("ErrorProne")) {
// When using the -Xplugin Error Prone integration, disable Error Prone for speculative
// recompiles to avoid infinite recursion.
continue;
}
if (SOURCE_TARGET_OPTIONS.contains(key) && originalOptions.isSet("--release")) {
// 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 =
JavacTool.create()
.getTask(
CharStreams.nullWriter(),
state.context.get(JavaFileManager.class),
diagnosticListener,
extraOptions,
arguments.getClassNames(),
fileObjects,
context);
FixCompiler fixCompiler;
try {
newTask.analyze();
fixCompiler = FixCompiler.create(fix, state);
} catch (IOException e) {
throw new UncheckedIOException(e);
return false;
}

Result compilationResult = fixCompiler.compile(extraOptions);
URI modifiedFileUri = FixCompiler.getModifiedFileUri(state);

// If we reached the maximum number of diagnostics of a given kind without finding one in the
// modified compilation unit, we won't find any more diagnostics, but we can't be sure that
// there isn't an diagnostic, as the diagnostic may simply be the (max+1)-th diagnostic, and
Expand All @@ -1303,7 +1246,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
int countWarnings = 0;
boolean warningIsError = false;
boolean warningInSameCompilationUnit = false;
for (Diagnostic<? extends JavaFileObject> diagnostic : diagnosticListener.getDiagnostics()) {
for (Diagnostic<? extends JavaFileObject> diagnostic : compilationResult.diagnostics()) {
warningIsError |= diagnostic.getCode().equals("compiler.err.warnings.and.werror");
JavaFileObject diagnosticSource = diagnostic.getSource();
// If the source's origin is unknown, assume that new diagnostics are due to a modification.
Expand Down Expand Up @@ -1333,6 +1276,124 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
return true;
}

/**
* A class to hold the files from the compilation context, with a diff applied to the
* currently-processed one; the files can then be recompiled.
*/
public static final class FixCompiler {
private final List<JavaFileObject> fileObjects;
private final VisitorState state;
private final BasicJavacTask javacTask;

private FixCompiler(
List<JavaFileObject> fileObjects, VisitorState state, BasicJavacTask javacTask) {
this.fileObjects = fileObjects;
this.state = state;
this.javacTask = javacTask;
}

public Result compile(ImmutableList<String> extraOptions) {
DiagnosticCollector<JavaFileObject> diagnosticListener = new DiagnosticCollector<>();
Context context = createContext();
Arguments arguments = Arguments.instance(javacTask.getContext());
JavacTask newTask =
JavacTool.create()
.getTask(
CharStreams.nullWriter(),
state.context.get(JavaFileManager.class),
diagnosticListener,
extraOptions,
arguments.getClassNames(),
fileObjects,
context);
try {
newTask.analyze();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
return Result.create(diagnosticListener.getDiagnostics());
}

private Context createContext() {
Context context = new Context();
Options options = Options.instance(context);
Options originalOptions = Options.instance(javacTask.getContext());
for (String key : originalOptions.keySet()) {
String value = originalOptions.get(key);
if (key.equals("-Xplugin:") && value.startsWith("ErrorProne")) {
// When using the -Xplugin Error Prone integration, disable Error Prone for speculative
// recompiles to avoid infinite recursion.
continue;
}
if (SOURCE_TARGET_OPTIONS.contains(key) && originalOptions.isSet("--release")) {
// 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);
}
return context;
}

public static URI getModifiedFileUri(VisitorState state) {
JCCompilationUnit compilationUnit = (JCCompilationUnit) state.getPath().getCompilationUnit();
JavaFileObject modifiedFile = compilationUnit.getSourceFile();
return modifiedFile.toUri();
}

public static FixCompiler create(Fix fix, VisitorState state) throws IOException {
BasicJavacTask javacTask = (BasicJavacTask) state.context.get(JavacTask.class);
if (javacTask == null) {
throw new IllegalArgumentException("No JavacTask in context.");
}
Arguments arguments = Arguments.instance(javacTask.getContext());
ArrayList<JavaFileObject> fileObjects = new ArrayList<>(arguments.getFileObjects());
applyFix(fix, state, fileObjects);
return new FixCompiler(fileObjects, state, javacTask);
}

private static void applyFix(Fix fix, VisitorState state, ArrayList<JavaFileObject> fileObjects)
throws IOException {

JCCompilationUnit compilationUnit = (JCCompilationUnit) state.getPath().getCompilationUnit();
JavaFileObject modifiedFile = compilationUnit.getSourceFile();
CharSequence modifiedFileContent =
modifiedFile.getCharContent(/* ignoreEncodingErrors= */ false);

URI modifiedFileUri = getModifiedFileUri(state);
IntStream.range(0, fileObjects.size())
.filter(i -> fileObjects.get(i).toUri().equals(modifiedFileUri))
.findFirst()
.ifPresent(
i -> {
DescriptionBasedDiff diff =
DescriptionBasedDiff.create(
compilationUnit, ImportOrganizer.STATIC_FIRST_ORGANIZER);
diff.handleFix(fix);
SourceFile fixSource = new SourceFile(modifiedFile.getName(), modifiedFileContent);
diff.applyDifferences(fixSource);
fileObjects.set(
i,
new SimpleJavaFileObject(sourceURI(modifiedFile.toUri()), Kind.SOURCE) {
@Override
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
return fixSource.getAsSequence();
}
});
});
}

/** The result of the compilation. */
@AutoValue
public abstract static class Result {
public abstract List<Diagnostic<? extends JavaFileObject>> diagnostics();

private static Result create(List<Diagnostic<? extends JavaFileObject>> diagnostics) {
return new AutoValue_SuggestedFixes_FixCompiler_Result(diagnostics);
}
}
}

private static final ImmutableSet<String> SOURCE_TARGET_OPTIONS =
ImmutableSet.of("-source", "--source", "-target", "--target");

Expand Down

0 comments on commit c996313

Please sign in to comment.