From c996313f98039939c53e7e4f4415a5e68acdd5a1 Mon Sep 17 00:00:00 2001 From: awturner Date: Tue, 19 Oct 2021 00:46:10 -0700 Subject: [PATCH] SuggestedFixes: refactor the compilesWithFix method so checks can make use of the diagnostics from the compiler. PiperOrigin-RevId: 404177580 --- .../errorprone/fixes/SuggestedFixes.java | 195 ++++++++++++------ 1 file changed, 128 insertions(+), 67 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java index 746d1ee5c8f..35509fbcbea 100644 --- a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java +++ b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java @@ -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; @@ -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; @@ -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; @@ -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 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 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 @@ -1303,7 +1246,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept int countWarnings = 0; boolean warningIsError = false; boolean warningInSameCompilationUnit = false; - for (Diagnostic diagnostic : diagnosticListener.getDiagnostics()) { + for (Diagnostic 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. @@ -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 fileObjects; + private final VisitorState state; + private final BasicJavacTask javacTask; + + private FixCompiler( + List fileObjects, VisitorState state, BasicJavacTask javacTask) { + this.fileObjects = fileObjects; + this.state = state; + this.javacTask = javacTask; + } + + public Result compile(ImmutableList extraOptions) { + DiagnosticCollector 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 fileObjects = new ArrayList<>(arguments.getFileObjects()); + applyFix(fix, state, fileObjects); + return new FixCompiler(fileObjects, state, javacTask); + } + + private static void applyFix(Fix fix, VisitorState state, ArrayList 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> diagnostics(); + + private static Result create(List> diagnostics) { + return new AutoValue_SuggestedFixes_FixCompiler_Result(diagnostics); + } + } + } + private static final ImmutableSet SOURCE_TARGET_OPTIONS = ImmutableSet.of("-source", "--source", "-target", "--target");