Skip to content

Commit 574cea1

Browse files
Stephan202Stephan202
committed
Prevent regression
1 parent d7cd72f commit 574cea1

File tree

3 files changed

+122
-60
lines changed

3 files changed

+122
-60
lines changed

‎check_api/src/main/java/com/google/errorprone/ErrorProneAnalyzer.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.base.Suppliers;
2525
import com.google.common.collect.ImmutableSet;
2626
import com.google.errorprone.BugPattern.SeverityLevel;
27+
import com.google.errorprone.ErrorProneOptions.Severity;
2728
import com.google.errorprone.RefactoringCollection.RefactoringResult;
2829
import com.google.errorprone.scanner.ErrorProneScannerTransformer;
2930
import com.google.errorprone.scanner.ScannerSupplier;
@@ -79,14 +80,19 @@ public static ErrorProneAnalyzer createAnalyzer(
7980
.or(
8081
Suppliers.memoize(
8182
() -> {
82-
ScannerSupplier toUse =
83-
ErrorPronePlugins.loadPlugins(scannerSupplier, context)
84-
.applyOverrides(epOptions);
8583
ImmutableSet<String> namedCheckers =
8684
epOptions.patchingOptions().namedCheckers();
87-
if (!namedCheckers.isEmpty()) {
88-
toUse = toUse.filter(bci -> namedCheckers.contains(bci.canonicalName()));
89-
}
85+
ScannerSupplier toUse =
86+
ErrorPronePlugins.loadPlugins(scannerSupplier, context)
87+
.applyOverrides(epOptions)
88+
.filter(
89+
bci -> {
90+
String name = bci.canonicalName();
91+
return epOptions.getSeverityMap().get(name) != Severity.OFF
92+
&& (namedCheckers.isEmpty()
93+
|| namedCheckers.contains(name));
94+
});
95+
9096
return ErrorProneScannerTransformer.create(toUse.get());
9197
}));
9298

‎core/src/test/java/com/google/errorprone/ErrorProneJavaCompilerTest.java‎

Lines changed: 110 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -434,34 +434,108 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
434434
}
435435

436436
@Test
437-
public void patchWithBugPatternCustomization() throws IOException {
438-
// Patching modifies files on disk, so we must create an actual file that matches the
439-
// `SimpleJavaFileObject` defined below.
440-
Path location = tempDir.getRoot().toPath().resolve("StringConstantWrapper.java");
441-
String source =
442-
"""
443-
class StringConstantWrapper {
444-
String s = "old-value";
445-
}
446-
""";
447-
Files.writeString(location, source);
437+
public void patchAll() throws IOException {
438+
JavaFileObject fileObject =
439+
createOnDiskFileObject(
440+
"StringConstantWrapper.java",
441+
"""
442+
class StringConstantWrapper {
443+
String s = "old-value";
444+
}
445+
""");
446+
447+
CompilationResult result =
448+
doCompile(
449+
Collections.singleton(fileObject),
450+
Arrays.asList("-XepPatchChecks:", "-XepPatchLocation:IN_PLACE"),
451+
Collections.singletonList(AssignmentUpdater.class));
452+
assertThat(result.succeeded).isTrue();
453+
assertThat(Files.readString(Path.of(fileObject.toUri())))
454+
.isEqualTo(
455+
"""
456+
class StringConstantWrapper {
457+
String s = "flag-not-set";
458+
}
459+
""");
460+
}
461+
462+
@Test
463+
public void patchAllWithCheckDisabled() throws IOException {
464+
JavaFileObject fileObject =
465+
createOnDiskFileObject(
466+
"StringConstantWrapper.java",
467+
"""
468+
class StringConstantWrapper {
469+
String s = "old-value";
470+
}
471+
""");
448472

449473
CompilationResult result =
450474
doCompile(
451-
Collections.singleton(
452-
new SimpleJavaFileObject(location.toUri(), SimpleJavaFileObject.Kind.SOURCE) {
453-
@Override
454-
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
455-
return source;
456-
}
457-
}),
475+
Collections.singleton(fileObject),
476+
Arrays.asList(
477+
"-XepPatchChecks:", "-XepPatchLocation:IN_PLACE", "-Xep:AssignmentUpdater:OFF"),
478+
Collections.singletonList(AssignmentUpdater.class));
479+
assertThat(result.succeeded).isTrue();
480+
assertThat(Files.readString(Path.of(fileObject.toUri())))
481+
.isEqualTo(
482+
"""
483+
class StringConstantWrapper {
484+
String s = "old-value";
485+
}
486+
""");
487+
}
488+
489+
@Test
490+
public void patchSingleWithCheckDisabled() throws IOException {
491+
JavaFileObject fileObject =
492+
createOnDiskFileObject(
493+
"StringConstantWrapper.java",
494+
"""
495+
class StringConstantWrapper {
496+
String s = "old-value";
497+
}
498+
""");
499+
500+
CompilationResult result =
501+
doCompile(
502+
Collections.singleton(fileObject),
503+
Arrays.asList(
504+
"-XepPatchChecks:AssignmentUpdater",
505+
"-XepPatchLocation:IN_PLACE",
506+
"-Xep:AssignmentUpdater:OFF"),
507+
Collections.singletonList(AssignmentUpdater.class));
508+
assertThat(result.succeeded).isTrue();
509+
assertThat(Files.readString(Path.of(fileObject.toUri())))
510+
.isEqualTo(
511+
"""
512+
class StringConstantWrapper {
513+
String s = "old-value";
514+
}
515+
""");
516+
}
517+
518+
@Test
519+
public void patchSingleWithBugPatternCustomization() throws IOException {
520+
JavaFileObject fileObject =
521+
createOnDiskFileObject(
522+
"StringConstantWrapper.java",
523+
"""
524+
class StringConstantWrapper {
525+
String s = "old-value";
526+
}
527+
""");
528+
529+
CompilationResult result =
530+
doCompile(
531+
Collections.singleton(fileObject),
458532
Arrays.asList(
459533
"-XepPatchChecks:AssignmentUpdater",
460534
"-XepPatchLocation:IN_PLACE",
461535
"-XepOpt:AssignmentUpdater:NewValue=new-value"),
462536
Collections.singletonList(AssignmentUpdater.class));
463537
assertThat(result.succeeded).isTrue();
464-
assertThat(Files.readString(location))
538+
assertThat(Files.readString(Path.of(fileObject.toUri())))
465539
.isEqualTo(
466540
"""
467541
class StringConstantWrapper {
@@ -470,6 +544,23 @@ class StringConstantWrapper {
470544
""");
471545
}
472546

547+
/**
548+
* Creates a {@link JavaFileObject} with matching on-disk contents.
549+
*
550+
* <p>This method aids in testing patching functionality, as {@code IN_PLACE} patching requires
551+
* that compiled code actually exists on disk.
552+
*/
553+
private JavaFileObject createOnDiskFileObject(String fileName, String source) throws IOException {
554+
Path location = tempDir.getRoot().toPath().resolve(fileName);
555+
Files.writeString(location, source);
556+
return new SimpleJavaFileObject(location.toUri(), SimpleJavaFileObject.Kind.SOURCE) {
557+
@Override
558+
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
559+
return source;
560+
}
561+
};
562+
}
563+
473564
private static class CompilationResult {
474565
public final boolean succeeded;
475566
public final String output;

‎core/src/test/java/com/google/errorprone/scanner/ScannerTest.java‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -106,41 +106,6 @@ class Test {
106106
.doTest();
107107
}
108108

109-
@Test
110-
public void dontRunPatchForDisabledChecks() {
111-
compilationHelper
112-
.addSourceLines(
113-
"Test.java",
114-
"""
115-
import com.google.errorprone.scanner.ScannerTest.Foo;
116-
117-
class Test {
118-
Foo foo;
119-
}
120-
""")
121-
.setArgs("-XepPatchLocation:IN_PLACE", "-XepPatchChecks:", "-Xep:ShouldNotUseFoo:OFF")
122-
.doTest();
123-
}
124-
125-
@Test
126-
public void dontRunPatchUnlessRequested() {
127-
compilationHelper
128-
.addSourceLines(
129-
"Test.java",
130-
"""
131-
import com.google.errorprone.scanner.ScannerTest.Foo;
132-
133-
class Test {
134-
Foo foo;
135-
}
136-
""")
137-
.setArgs(
138-
"-XepPatchLocation:IN_PLACE",
139-
"-Xep:ShouldNotUseFoo:WARN",
140-
"-XepPatchChecks:DeadException")
141-
.doTest();
142-
}
143-
144109
@OkToUseFoo // Foo can use itself. But this shouldn't suppress errors on *usages* of Foo.
145110
public static final class Foo<T> {}
146111

0 commit comments

Comments
 (0)