Introduce command-line flag -XepAllSuggestionsAsWarnings

This enables users that build with `-Werror` to disallow violations of custom `SUGGESTION`-level bug patterns. Fixes #3301 FUTURE_COPYBARA_INTEGRATE_REVIEW=#3301 from PicnicSupermarket:rossendrijver/suggestions_as_warnings 5cd3aa1 PiperOrigin-RevId: 538605260
google · Jun 8, 2023 · 25fed4b · 25fed4b
1 parent 3602ab0
commit 25fed4b
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 2 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java b/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
@@ -52,8 +52,8 @@ public class ErrorProneOptions {
   private static final String PATCH_IMPORT_ORDER_PREFIX = "-XepPatchImportOrder:";
   private static final String EXCLUDED_PATHS_PREFIX = "-XepExcludedPaths:";
   private static final String IGNORE_LARGE_CODE_GENERATORS = "-XepIgnoreLargeCodeGenerators:";
-
   private static final String ERRORS_AS_WARNINGS_FLAG = "-XepAllErrorsAsWarnings";
+  private static final String SUGGESTIONS_AS_WARNINGS_FLAG = "-XepAllSuggestionsAsWarnings";
   private static final String ENABLE_ALL_CHECKS = "-XepAllDisabledChecksAsWarnings";
   private static final String IGNORE_SUPPRESSION_ANNOTATIONS = "-XepIgnoreSuppressionAnnotations";
   private static final String DISABLE_ALL_CHECKS = "-XepDisableAllChecks";
@@ -75,6 +75,7 @@ public static int isSupportedOption(String option) {
             || option.equals(IGNORE_UNKNOWN_CHECKS_FLAG)
             || option.equals(DISABLE_WARNINGS_IN_GENERATED_CODE_FLAG)
             || option.equals(ERRORS_AS_WARNINGS_FLAG)
+            || option.equals(SUGGESTIONS_AS_WARNINGS_FLAG)
             || option.equals(ENABLE_ALL_CHECKS)
             || option.equals(DISABLE_ALL_CHECKS)
             || option.equals(IGNORE_SUPPRESSION_ANNOTATIONS)
@@ -163,6 +164,7 @@ final PatchingOptions build() {
   private final boolean disableWarningsInGeneratedCode;
   private final boolean disableAllWarnings;
   private final boolean dropErrorsToWarnings;
+  private final boolean suggestionsAsWarnings;
   private final boolean enableAllChecksAsWarnings;
   private final boolean disableAllChecks;
   private final boolean isTestOnlyTarget;
@@ -180,6 +182,7 @@ private ErrorProneOptions(
       boolean disableWarningsInGeneratedCode,
       boolean disableAllWarnings,
       boolean dropErrorsToWarnings,
+      boolean suggestionsAsWarnings,
       boolean enableAllChecksAsWarnings,
       boolean disableAllChecks,
       boolean isTestOnlyTarget,
@@ -195,6 +198,7 @@ private ErrorProneOptions(
     this.disableWarningsInGeneratedCode = disableWarningsInGeneratedCode;
     this.disableAllWarnings = disableAllWarnings;
     this.dropErrorsToWarnings = dropErrorsToWarnings;
+    this.suggestionsAsWarnings = suggestionsAsWarnings;
     this.enableAllChecksAsWarnings = enableAllChecksAsWarnings;
     this.disableAllChecks = disableAllChecks;
     this.isTestOnlyTarget = isTestOnlyTarget;
@@ -230,6 +234,10 @@ public boolean isDropErrorsToWarnings() {
     return dropErrorsToWarnings;
   }
 
+  public boolean isSuggestionsAsWarnings() {
+    return suggestionsAsWarnings;
+  }
+
   public boolean isTestOnlyTarget() {
     return isTestOnlyTarget;
   }
@@ -263,6 +271,7 @@ private static class Builder {
     private boolean disableAllWarnings = false;
     private boolean disableWarningsInGeneratedCode = false;
     private boolean dropErrorsToWarnings = false;
+    private boolean suggestionsAsWarnings = false;
     private boolean enableAllChecksAsWarnings = false;
     private boolean disableAllChecks = false;
     private boolean isTestOnlyTarget = false;
@@ -319,6 +328,10 @@ public void setDropErrorsToWarnings(boolean dropErrorsToWarnings) {
       this.dropErrorsToWarnings = dropErrorsToWarnings;
     }
 
+    public void setSuggestionsAsWarnings(boolean suggestionsAsWarnings) {
+      this.suggestionsAsWarnings = suggestionsAsWarnings;
+    }
+
     public void setDisableAllWarnings(boolean disableAllWarnings) {
       severityMap.entrySet().stream()
           .filter(e -> e.getValue() == Severity.WARN)
@@ -364,6 +377,7 @@ public ErrorProneOptions build(ImmutableList<String> remainingArgs) {
           disableWarningsInGeneratedCode,
           disableAllWarnings,
           dropErrorsToWarnings,
+          suggestionsAsWarnings,
           enableAllChecksAsWarnings,
           disableAllChecks,
           isTestOnlyTarget,
@@ -420,6 +434,9 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
         case ERRORS_AS_WARNINGS_FLAG:
           builder.setDropErrorsToWarnings(true);
           break;
+        case SUGGESTIONS_AS_WARNINGS_FLAG:
+          builder.setSuggestionsAsWarnings(true);
+          break;
         case ENABLE_ALL_CHECKS:
           builder.setEnableAllChecksAsWarnings(true);
           break;

diff --git a/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java b/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java
@@ -147,7 +147,8 @@ public ScannerSupplier applyOverrides(ErrorProneOptions errorProneOptions) {
         && !errorProneOptions.isEnableAllChecksAsWarnings()
         && !errorProneOptions.isDropErrorsToWarnings()
         && !errorProneOptions.isDisableAllChecks()
-        && !errorProneOptions.isDisableAllWarnings()) {
+        && !errorProneOptions.isDisableAllWarnings()
+        && !errorProneOptions.isSuggestionsAsWarnings()) {
       return this;
     }
 
@@ -168,6 +169,12 @@ public ScannerSupplier applyOverrides(ErrorProneOptions errorProneOptions) {
           .forEach(c -> severities.put(c.canonicalName(), SeverityLevel.WARNING));
     }
 
+    if (errorProneOptions.isSuggestionsAsWarnings()) {
+      getAllChecks().values().stream()
+          .filter(c -> c.defaultSeverity() == SeverityLevel.SUGGESTION)
+          .forEach(c -> severities.put(c.canonicalName(), SeverityLevel.WARNING));
+    }
+
     if (errorProneOptions.isDisableAllWarnings()) {
       checks.values().stream()
           .filter(c -> c.defaultSeverity() == SeverityLevel.WARNING)

diff --git a/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java b/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java
@@ -152,6 +152,13 @@ public void recognizesDemoteErrorToWarning() {
     assertThat(options.isDropErrorsToWarnings()).isTrue();
   }
 
+  @Test
+  public void recognizesAllSuggestionsAsWarnings() {
+    ErrorProneOptions options =
+        ErrorProneOptions.processArgs(new String[] {"-XepAllSuggestionsAsWarnings"});
+    assertThat(options.isSuggestionsAsWarnings()).isTrue();
+  }
+
   @Test
   public void recognizesDisableAllChecks() {
     ErrorProneOptions options =

diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
@@ -42,12 +42,15 @@
 import com.google.errorprone.InvalidCommandLineOptionException;
 import com.google.errorprone.bugpatterns.ArrayEquals;
 import com.google.errorprone.bugpatterns.BadShiftAmount;
+import com.google.errorprone.bugpatterns.BooleanParameter;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.ChainingConstructorIgnoresParameter;
+import com.google.errorprone.bugpatterns.ConstantField;
 import com.google.errorprone.bugpatterns.DepAnn;
 import com.google.errorprone.bugpatterns.EqualsIncompatibleType;
 import com.google.errorprone.bugpatterns.LongLiteralLowerCaseSuffix;
 import com.google.errorprone.bugpatterns.MethodCanBeStatic;
+import com.google.errorprone.bugpatterns.MissingBraces;
 import com.google.errorprone.bugpatterns.PackageLocation;
 import com.google.errorprone.bugpatterns.ReferenceEquality;
 import com.google.errorprone.bugpatterns.StaticQualifiedUsingExpression;
@@ -557,6 +560,30 @@ public void allChecksAsWarningsWorks() {
     assertScanner(withOverrides).hasSeverities(expectedSeverities);
   }
 
+  @Test
+  public void allSuggestionsAsWarnings() {
+    ScannerSupplier ss =
+        ScannerSupplier.fromBugCheckerClasses(
+            BooleanParameter.class, ConstantField.class, MissingBraces.class);
+
+    assertScanner(ss)
+        .hasEnabledChecks(BooleanParameter.class, ConstantField.class, MissingBraces.class);
+
+    ErrorProneOptions epOptions =
+        ErrorProneOptions.processArgs(ImmutableList.of("-XepAllSuggestionsAsWarnings"));
+
+    ImmutableMap<String, SeverityLevel> expectedSeverities =
+        ImmutableMap.of(
+            "BooleanParameter",
+            SeverityLevel.WARNING,
+            "ConstantField",
+            SeverityLevel.WARNING,
+            "MissingBraces",
+            SeverityLevel.WARNING);
+
+    assertScanner(ss.applyOverrides(epOptions)).hasSeverities(expectedSeverities);
+  }
+
   @Test
   public void canSuppressViaAltName() {
     ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses(WithAltName.class);