Skip to content

Commit c3581e9

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Fix JavacFileManager compiler crash during speculative recompiles in modular projects
PiperOrigin-RevId: 929164894
1 parent da1f32b commit c3581e9

2 files changed

Lines changed: 183 additions & 8 deletions

File tree

‎check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java‎

Lines changed: 132 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.google.common.base.Splitter;
4646
import com.google.common.base.Verify;
4747
import com.google.common.collect.ImmutableList;
48+
import com.google.common.collect.ImmutableMap;
4849
import com.google.common.collect.ImmutableSet;
4950
import com.google.common.collect.Iterables;
5051
import com.google.common.collect.Lists;
@@ -110,18 +111,21 @@
110111
import com.sun.tools.javac.util.JCDiagnostic;
111112
import com.sun.tools.javac.util.Options;
112113
import com.sun.tools.javac.util.Position;
114+
import java.io.File;
113115
import java.io.IOException;
114116
import java.io.UncheckedIOException;
115117
import java.lang.annotation.Target;
116118
import java.net.JarURLConnection;
117119
import java.net.URI;
120+
import java.nio.file.Path;
118121
import java.util.ArrayDeque;
119122
import java.util.ArrayList;
120123
import java.util.Arrays;
121124
import java.util.Collection;
122125
import java.util.Comparator;
123126
import java.util.Deque;
124127
import java.util.EnumSet;
128+
import java.util.HashMap;
125129
import java.util.HashSet;
126130
import java.util.Iterator;
127131
import java.util.List;
@@ -146,10 +150,14 @@
146150
import javax.lang.model.util.SimpleTypeVisitor8;
147151
import javax.tools.Diagnostic;
148152
import javax.tools.DiagnosticCollector;
153+
import javax.tools.FileObject;
154+
import javax.tools.ForwardingJavaFileManager;
149155
import javax.tools.JavaFileManager;
156+
import javax.tools.JavaFileManager.Location;
150157
import javax.tools.JavaFileObject;
151158
import javax.tools.JavaFileObject.Kind;
152159
import javax.tools.SimpleJavaFileObject;
160+
import javax.tools.StandardJavaFileManager;
153161
import org.jspecify.annotations.Nullable;
154162

155163
/** Factories for constructing {@link Fix}es. */
@@ -1447,23 +1455,46 @@ public static final class FixCompiler {
14471455
private final List<JavaFileObject> fileObjects;
14481456
private final VisitorState state;
14491457
private final BasicJavacTask javacTask;
1458+
private final ImmutableMap<URI, JavaFileObject> speculativeFiles;
14501459

14511460
private FixCompiler(
1452-
List<JavaFileObject> fileObjects, VisitorState state, BasicJavacTask javacTask) {
1461+
List<JavaFileObject> fileObjects,
1462+
VisitorState state,
1463+
BasicJavacTask javacTask,
1464+
Map<URI, JavaFileObject> speculativeFiles) {
14531465
this.fileObjects = fileObjects;
14541466
this.state = state;
14551467
this.javacTask = javacTask;
1468+
this.speculativeFiles = ImmutableMap.copyOf(speculativeFiles);
14561469
}
14571470

14581471
public Result compile(ImmutableList<String> extraOptions) {
14591472
DiagnosticCollector<JavaFileObject> diagnosticListener = new DiagnosticCollector<>();
14601473
Context context = createContext();
14611474
Arguments arguments = Arguments.instance(javacTask.getContext());
1475+
JavaFileManager originalFileManager = state.context.get(JavaFileManager.class);
1476+
JavaFileManager fileManager = originalFileManager;
1477+
if (originalFileManager instanceof StandardJavaFileManager standardFileManager) {
1478+
fileManager =
1479+
new StandardForwardingFileManager(standardFileManager) {
1480+
@Override
1481+
public boolean contains(Location location, FileObject fo) throws IOException {
1482+
JavaFileObject underlying = speculativeFiles.get(fo.toUri());
1483+
return super.contains(location, underlying != null ? underlying : fo);
1484+
}
1485+
1486+
@Override
1487+
public Path asPath(FileObject fo) {
1488+
JavaFileObject underlying = speculativeFiles.get(fo.toUri());
1489+
return super.asPath(underlying != null ? underlying : fo);
1490+
}
1491+
};
1492+
}
14621493
JavacTask newTask =
14631494
JavacTool.create()
14641495
.getTask(
14651496
CharStreams.nullWriter(),
1466-
state.context.get(JavaFileManager.class),
1497+
fileManager,
14671498
diagnosticListener,
14681499
extraOptions,
14691500
arguments.getClassNames(),
@@ -1511,11 +1542,16 @@ public static FixCompiler create(Fix fix, VisitorState state) throws IOException
15111542
}
15121543
Arguments arguments = Arguments.instance(javacTask.getContext());
15131544
ArrayList<JavaFileObject> fileObjects = new ArrayList<>(arguments.getFileObjects());
1514-
applyFix(fix, state, fileObjects);
1515-
return new FixCompiler(fileObjects, state, javacTask);
1545+
Map<URI, JavaFileObject> speculativeFiles = new HashMap<>();
1546+
applyFix(fix, state, fileObjects, speculativeFiles);
1547+
return new FixCompiler(fileObjects, state, javacTask, speculativeFiles);
15161548
}
15171549

1518-
private static void applyFix(Fix fix, VisitorState state, ArrayList<JavaFileObject> fileObjects)
1550+
private static void applyFix(
1551+
Fix fix,
1552+
VisitorState state,
1553+
ArrayList<JavaFileObject> fileObjects,
1554+
Map<URI, JavaFileObject> speculativeFiles)
15191555
throws IOException {
15201556

15211557
JCCompilationUnit compilationUnit = (JCCompilationUnit) state.getPath().getCompilationUnit();
@@ -1535,14 +1571,16 @@ private static void applyFix(Fix fix, VisitorState state, ArrayList<JavaFileObje
15351571
diff.handleFix(fix);
15361572
SourceFile fixSource = new SourceFile(modifiedFile.getName(), modifiedFileContent);
15371573
diff.applyDifferences(fixSource);
1538-
fileObjects.set(
1539-
i,
1574+
JavaFileObject originalFile = fileObjects.get(i);
1575+
SimpleJavaFileObject speculativeFile =
15401576
new SimpleJavaFileObject(sourceURI(modifiedFile.toUri()), Kind.SOURCE) {
15411577
@Override
15421578
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
15431579
return fixSource.getAsSequence();
15441580
}
1545-
});
1581+
};
1582+
fileObjects.set(i, speculativeFile);
1583+
speculativeFiles.put(speculativeFile.toUri(), originalFile);
15461584
});
15471585
}
15481586

@@ -1924,5 +1962,91 @@ public String avoidShadowing(String name) {
19241962
}
19251963
}
19261964

1965+
private static class StandardForwardingFileManager
1966+
extends ForwardingJavaFileManager<StandardJavaFileManager>
1967+
implements StandardJavaFileManager {
1968+
StandardForwardingFileManager(StandardJavaFileManager delegate) {
1969+
super(delegate);
1970+
}
1971+
1972+
@Override
1973+
public Iterable<? extends JavaFileObject> getJavaFileObjectsFromFiles(
1974+
Iterable<? extends File> files) {
1975+
return fileManager.getJavaFileObjectsFromFiles(files);
1976+
}
1977+
1978+
@Override
1979+
public Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
1980+
Collection<? extends Path> paths) {
1981+
return fileManager.getJavaFileObjectsFromPaths(paths);
1982+
}
1983+
1984+
// Implementing StandardJavaFileManager
1985+
@SuppressWarnings({"deprecation", "IterablePathParameter"})
1986+
@Override
1987+
public Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
1988+
Iterable<? extends Path> paths) {
1989+
return fileManager.getJavaFileObjectsFromPaths(paths);
1990+
}
1991+
1992+
@Override
1993+
public Iterable<? extends JavaFileObject> getJavaFileObjects(File... files) {
1994+
return fileManager.getJavaFileObjects(files);
1995+
}
1996+
1997+
@Override
1998+
public Iterable<? extends JavaFileObject> getJavaFileObjects(Path... paths) {
1999+
return fileManager.getJavaFileObjects(paths);
2000+
}
2001+
2002+
@Override
2003+
public Iterable<? extends JavaFileObject> getJavaFileObjects(String... names) {
2004+
return fileManager.getJavaFileObjects(names);
2005+
}
2006+
2007+
@Override
2008+
public Iterable<? extends JavaFileObject> getJavaFileObjectsFromStrings(
2009+
Iterable<String> names) {
2010+
return fileManager.getJavaFileObjectsFromStrings(names);
2011+
}
2012+
2013+
@Override
2014+
public void setLocation(Location location, Iterable<? extends File> files) throws IOException {
2015+
fileManager.setLocation(location, files);
2016+
}
2017+
2018+
@Override
2019+
public void setLocationFromPaths(Location location, Collection<? extends Path> paths)
2020+
throws IOException {
2021+
fileManager.setLocationFromPaths(location, paths);
2022+
}
2023+
2024+
@Override
2025+
public void setLocationForModule(
2026+
Location location, String moduleName, Collection<? extends Path> paths) throws IOException {
2027+
fileManager.setLocationForModule(location, moduleName, paths);
2028+
}
2029+
2030+
@Override
2031+
public Iterable<? extends File> getLocation(Location location) {
2032+
return fileManager.getLocation(location);
2033+
}
2034+
2035+
@Override
2036+
public Iterable<? extends Path> getLocationAsPaths(Location location) {
2037+
return fileManager.getLocationAsPaths(location);
2038+
}
2039+
2040+
@Override
2041+
public Path asPath(FileObject file) {
2042+
return fileManager.asPath(file);
2043+
}
2044+
2045+
@Override
2046+
public void setPathFactory(PathFactory f) {
2047+
fileManager.setPathFactory(f);
2048+
}
2049+
}
2050+
19272051
private SuggestedFixes() {}
19282052
}

‎core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
import static com.google.errorprone.matchers.Matchers.staticMethod;
2626
import static com.google.errorprone.suppliers.Suppliers.typeFromString;
2727
import static java.lang.annotation.RetentionPolicy.RUNTIME;
28+
import static java.nio.charset.StandardCharsets.UTF_8;
2829

2930
import com.google.common.base.Verify;
3031
import com.google.common.collect.ImmutableList;
3132
import com.google.common.collect.ImmutableMap;
33+
import com.google.errorprone.BaseErrorProneJavaCompiler;
3234
import com.google.errorprone.BugCheckerRefactoringTestHelper;
3335
import com.google.errorprone.BugPattern;
3436
import com.google.errorprone.CompilationTestHelper;
@@ -45,6 +47,7 @@
4547
import com.google.errorprone.bugpatterns.RemoveUnusedImports;
4648
import com.google.errorprone.matchers.Description;
4749
import com.google.errorprone.matchers.Matcher;
50+
import com.google.errorprone.scanner.ScannerSupplier;
4851
import com.google.errorprone.suppliers.Supplier;
4952
import com.google.errorprone.util.ASTHelpers;
5053
import com.sun.source.doctree.LinkTree;
@@ -64,17 +67,25 @@
6467
import com.sun.source.util.DocTreePath;
6568
import com.sun.source.util.DocTreePathScanner;
6669
import com.sun.tools.javac.code.Type;
70+
import com.sun.tools.javac.file.JavacFileManager;
6771
import com.sun.tools.javac.tree.DCTree;
6872
import com.sun.tools.javac.tree.DCTree.DCDocComment;
6973
import com.sun.tools.javac.tree.DocCommentTable;
7074
import com.sun.tools.javac.tree.JCTree;
75+
import com.sun.tools.javac.util.Context;
7176
import java.io.IOException;
77+
import java.io.StringWriter;
7278
import java.lang.annotation.Retention;
7379
import java.net.URI;
80+
import java.nio.file.Files;
81+
import java.nio.file.Path;
7482
import java.util.Arrays;
7583
import java.util.Optional;
7684
import java.util.function.Function;
7785
import javax.lang.model.element.Modifier;
86+
import javax.tools.DiagnosticCollector;
87+
import javax.tools.JavaFileObject;
88+
import javax.tools.StandardLocation;
7889
import org.junit.Test;
7990
import org.junit.runner.RunWith;
8091
import org.junit.runners.JUnit4;
@@ -2568,6 +2579,46 @@ class ClassContainingRawType {
25682579
.doTest();
25692580
}
25702581

2582+
@Test
2583+
public void compilesWithFix_modularCompilation() throws Exception {
2584+
Path tempDir = Files.createTempDirectory("test-modular");
2585+
Path testFile = tempDir.resolve("Test.java");
2586+
Files.writeString(
2587+
testFile,
2588+
"""
2589+
package foo;
2590+
class Test { int x = 0; }
2591+
""");
2592+
Path moduleInfo = tempDir.resolve("module-info.java");
2593+
Files.writeString(
2594+
moduleInfo,
2595+
"""
2596+
module foo {}
2597+
""");
2598+
try (JavacFileManager fm = new JavacFileManager(new Context(), false, UTF_8)) {
2599+
fm.setLocationFromPaths(StandardLocation.SOURCE_PATH, ImmutableList.of(tempDir));
2600+
BaseErrorProneJavaCompiler compiler =
2601+
new BaseErrorProneJavaCompiler(
2602+
ScannerSupplier.fromBugCheckerClasses(CompilesWithFixChecker.class));
2603+
DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>();
2604+
boolean success =
2605+
compiler
2606+
.getTask(
2607+
new StringWriter(),
2608+
fm,
2609+
diagnostics,
2610+
ImmutableList.of(),
2611+
ImmutableList.of(),
2612+
fm.getJavaFileObjects(moduleInfo, testFile))
2613+
.call();
2614+
assertThat(success).isFalse();
2615+
assertThat(
2616+
diagnostics.getDiagnostics().stream()
2617+
.anyMatch(d -> d.getMessage(null).contains("[CompilesWithFixChecker]")))
2618+
.isTrue();
2619+
}
2620+
}
2621+
25712622
private static Description addSuppressWarningsIfCompilationSucceeds(
25722623
ClassTree tree,
25732624
VisitorState state,

0 commit comments

Comments
 (0)