Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.scanner;

import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
import static java.util.function.Predicate.not;

import com.google.common.base.Predicate;
import com.google.common.base.Supplier;
Expand Down Expand Up @@ -298,7 +299,7 @@ public ScannerSupplier plus(ScannerSupplier other) {
public ScannerSupplier filter(Predicate<? super BugCheckerInfo> predicate) {
ImmutableSet<String> disabled =
getAllChecks().values().stream()
.filter(predicate.negate())
.filter(bci -> disabled().contains(bci.canonicalName()) || !predicate.test(bci))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in #4699 assumed that ScannerSupplier#filter would merely disable additional checks, but in fact it could re-enable disabled checks. This change proposes to resolve that, yielding more intuitive semantics.

.map(BugCheckerInfo::canonicalName)
.collect(ImmutableSet.toImmutableSet());
return new ScannerSupplierImpl(getAllChecks(), severities(), disabled, getFlags());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,32 @@ class StringConstantWrapper {
""");
}

@Test
public void patchAllWithDisableAllChecks() throws IOException {
JavaFileObject fileObject =
createOnDiskFileObject(
"StringConstantWrapper.java",
"""
class StringConstantWrapper {
String s = "old-value";
}
""");

CompilationResult result =
doCompile(
Collections.singleton(fileObject),
Arrays.asList("-XepPatchChecks:", "-XepPatchLocation:IN_PLACE", "-XepDisableAllChecks"),
ImmutableList.of(AssignmentUpdater.class));
assertThat(result.succeeded).isTrue();
assertThat(Files.readString(Path.of(fileObject.toUri())))
.isEqualTo(
"""
class StringConstantWrapper {
String s = "old-value";
}
""");
}
Comment on lines +489 to +513
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 new test fails prior to the proposed changes. Observe that the only difference with the preceding patchAllWithCheckDisabled test is the use of -XepDisableAllChecks instead of -Xep:AssignmentUpdater:OFF.


@Test
public void patchSingleWithCheckDisabled() throws IOException {
JavaFileObject fileObject =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,9 @@ public void filterWorks() {
ScannerSupplier.fromBugCheckerClasses(
ArrayEquals.class, BadShiftAmount.class, StaticQualifiedUsingExpression.class);

assertScanner(ss.filter(input -> input.canonicalName().equals("BadShiftAmount")))
ScannerSupplier derivedSupplier = ss.filter(input -> !input.canonicalName().startsWith("S"));
assertScanner(derivedSupplier).hasEnabledChecks(ArrayEquals.class, BadShiftAmount.class);
assertScanner(derivedSupplier.filter(input -> !input.canonicalName().startsWith("A")))
.hasEnabledChecks(BadShiftAmount.class);
}

Expand Down