Skip to content

Commit

Permalink
Don't set checks as disableable = false in external
Browse files Browse the repository at this point in the history
to avoid preventing workarounds for crashes like
#2099

PiperOrigin-RevId: 351919242
  • Loading branch information
cushon authored and Error Prone Team committed Jan 15, 2021
1 parent a7f3413 commit 6861403
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,7 @@
"Non-compile-time constant expression passed to parameter with "
+ "@CompileTimeConstant type annotation.",
linkType = NONE,
severity = ERROR,
disableable = false,
suppressionAnnotations = {}
)
severity = ERROR)
public class CompileTimeConstantChecker extends BugChecker
implements LambdaExpressionTreeMatcher,
MemberReferenceTreeMatcher,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
altNames = {"InsecureCipherMode"},
summary =
"A standard cryptographic operation is used in a mode that is prone to vulnerabilities",
documentSuppression = false,
severity = ERROR)
public class InsecureCipherMode extends BugChecker implements MethodInvocationTreeMatcher {
private static final String MESSAGE_BASE = "Insecure usage of a crypto API: ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@
@BugPattern(
name = "RestrictedApiChecker",
summary = "Check for non-whitelisted callers to RestrictedApiChecker.",
severity = SeverityLevel.ERROR,
suppressionAnnotations = {},
disableable = false)
severity = SeverityLevel.ERROR)
public class RestrictedApiChecker extends BugChecker
implements MethodInvocationTreeMatcher,
NewClassTreeMatcher,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@
@BugPattern(
name = "FormatStringAnnotation",
summary = "Invalid format string passed to formatting method.",
severity = ERROR
)
severity = ERROR)
public final class FormatStringAnnotationChecker extends BugChecker
implements MethodInvocationTreeMatcher, MethodTreeMatcher, NewClassTreeMatcher {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,6 @@ public class CompileTimeConstantCheckerTest {
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(CompileTimeConstantChecker.class, getClass());

@Test
public void test_SuppressWarningsDoesntWork() {
compilationHelper
.addSourceLines(
"test/CompileTimeConstantTestCase.java",
"package test;",
"import com.google.errorprone.annotations.CompileTimeConstant;",
"public class CompileTimeConstantTestCase {",
" public static void m(@CompileTimeConstant String s) { }",
" @SuppressWarnings(\"CompileTimeConstant\")",
" // BUG: Diagnostic contains: Non-compile-time constant expression passed",
" public static void r(String x) { m(x); }",
"}")
.doTest();
}

@Test
public void testMatches_fieldAccessFailsWithNonConstant() {
compilationHelper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import com.google.errorprone.bugpatterns.EqualsIncompatibleType;
import com.google.errorprone.bugpatterns.LongLiteralLowerCaseSuffix;
import com.google.errorprone.bugpatterns.PackageLocation;
import com.google.errorprone.bugpatterns.RestrictedApiChecker;
import com.google.errorprone.bugpatterns.StaticQualifiedUsingExpression;
import com.google.errorprone.bugpatterns.StringEquality;
import com.google.errorprone.bugpatterns.nullness.UnnecessaryCheckNotNull;
Expand Down Expand Up @@ -330,52 +329,6 @@ public void applyOverridesEnableAllChecks() {
.hasEnabledChecks(BadShiftAmount.class, StaticQualifiedUsingExpression.class);
}

@Test
public void applyOverridesDisableAllChecks() {
// Create new scanner.
ScannerSupplier ss =
ScannerSupplier.fromBugCheckerClasses(
ArrayEquals.class, BadShiftAmount.class, StaticQualifiedUsingExpression.class);

// Make sure all checks in the scanner start enabled.
assertScanner(ss)
.hasEnabledChecks(
ArrayEquals.class, BadShiftAmount.class, StaticQualifiedUsingExpression.class);

// Apply the 'DisableAllChecks' flag, make sure all checks are disabled.
ErrorProneOptions epOptions =
ErrorProneOptions.processArgs(ImmutableList.of("-XepDisableAllChecks"));

assertScanner(ss.applyOverrides(epOptions)).hasEnabledChecks();

// Don't suppress unsuppressible checks.
ss = ss.plus(ScannerSupplier.fromBugCheckerClasses(RestrictedApiChecker.class));

assertScanner(ss.applyOverrides(epOptions)).hasEnabledChecks(RestrictedApiChecker.class);

// Apply 'DisableAllChecks' flag with another -Xep flag
epOptions =
ErrorProneOptions.processArgs(
ImmutableList.of("-XepDisableAllChecks", "-Xep:ArrayEquals:ERROR"));

// Make sure the severity flag overrides the global disable flag.
assertScanner(ss.applyOverrides(epOptions))
.hasEnabledChecks(RestrictedApiChecker.class, ArrayEquals.class);

// Order matters. The DisableAllChecks flag should override all severities that come before it.
epOptions =
ErrorProneOptions.processArgs(
ImmutableList.of("-Xep:ArrayEquals:ERROR", "-XepDisableAllChecks"));

// Make sure the global disable flag overrides flags that come before it.
assertScanner(ss.applyOverrides(epOptions)).hasEnabledChecks(RestrictedApiChecker.class);

// The 'DisableAllChecks' flag doesn't populate through to additional plugins.
assertScanner(
ss.applyOverrides(epOptions).plus(ScannerSupplier.fromBugCheckerClasses(DivZero.class)))
.hasEnabledChecks(RestrictedApiChecker.class, DivZero.class);
}

@Test
public void applyOverridesDisableErrors() {
// BadShiftAmount (error), ArrayEquals (unsuppressible error), StringEquality (warning)
Expand Down

0 comments on commit 6861403

Please sign in to comment.