Allow -Xep severity overrides to use alt names

PiperOrigin-RevId: 490073374
google · Nov 21, 2022 · 48bbef0 · 48bbef0
1 parent 07baa7d
commit 48bbef0
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 55 deletions.
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
@@ -16,11 +16,14 @@
 
 package com.google.errorprone.scanner;
 
+import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
+
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
 import com.google.common.collect.HashBiMap;
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -160,64 +163,67 @@ public ScannerSupplier applyOverrides(ErrorProneOptions errorProneOptions) {
     }
 
     if (errorProneOptions.isDropErrorsToWarnings()) {
-      getAllChecks().values().stream()
+      checks.values().stream()
           .filter(c -> c.defaultSeverity() == SeverityLevel.ERROR && c.disableable())
           .forEach(c -> severities.put(c.canonicalName(), SeverityLevel.WARNING));
     }
 
     if (errorProneOptions.isDisableAllWarnings()) {
-      getAllChecks().values().stream()
+      checks.values().stream()
           .filter(c -> c.defaultSeverity() == SeverityLevel.WARNING)
           .forEach(c -> disabled.add(c.canonicalName()));
     }
     if (errorProneOptions.isDisableAllChecks()) {
-      getAllChecks().values().stream()
+      checks.values().stream()
           .filter(c -> c.disableable())
           .forEach(c -> disabled.add(c.canonicalName()));
     }
 
+    ImmutableListMultimap<String, BugCheckerInfo> checksByAltName =
+        checks.values().stream()
+            .collect(flatteningToImmutableListMultimap(x -> x, c -> c.allNames().stream()))
+            .inverse();
+
     // Process overrides
     severityOverrides.forEach(
         (checkName, newSeverity) -> {
-          BugCheckerInfo check = getAllChecks().get(checkName);
-          if (check == null) {
+          if (!checksByAltName.containsKey(checkName)) {
             if (errorProneOptions.ignoreUnknownChecks()) {
               return;
             }
             throw new InvalidCommandLineOptionException(checkName + " is not a valid checker name");
           }
-
-          switch (newSeverity) {
-            case OFF:
-              if (!check.disableable()) {
-                throw new InvalidCommandLineOptionException(
-                    check.canonicalName() + " may not be disabled");
-              }
-              severities.remove(check.canonicalName());
-              disabled.add(check.canonicalName());
-              break;
-            case DEFAULT:
-              severities.put(check.canonicalName(), check.defaultSeverity());
-              disabled.remove(check.canonicalName());
-              break;
-            case WARN:
-              // Demoting an enabled check from an error to a warning is a form of disabling
-              if (!disabled().contains(check.canonicalName())
-                  && !check.disableable()
-                  && check.defaultSeverity() == SeverityLevel.ERROR) {
-                throw new InvalidCommandLineOptionException(
-                    check.canonicalName()
-                        + " is not disableable and may not be demoted to a warning");
-              }
-              severities.put(check.canonicalName(), SeverityLevel.WARNING);
-              disabled.remove(check.canonicalName());
-              break;
-            case ERROR:
-              severities.put(check.canonicalName(), SeverityLevel.ERROR);
-              disabled.remove(check.canonicalName());
-              break;
-            default:
-              throw new IllegalStateException("Unexpected severity level: " + newSeverity);
+          for (BugCheckerInfo check : checksByAltName.get(checkName)) {
+            switch (newSeverity) {
+              case OFF:
+                if (!check.disableable()) {
+                  throw new InvalidCommandLineOptionException(
+                      check.canonicalName() + " may not be disabled");
+                }
+                severities.remove(check.canonicalName());
+                disabled.add(check.canonicalName());
+                break;
+              case DEFAULT:
+                severities.put(check.canonicalName(), check.defaultSeverity());
+                disabled.remove(check.canonicalName());
+                break;
+              case WARN:
+                // Demoting an enabled check from an error to a warning is a form of disabling
+                if (!disabled().contains(check.canonicalName())
+                    && !check.disableable()
+                    && check.defaultSeverity() == SeverityLevel.ERROR) {
+                  throw new InvalidCommandLineOptionException(
+                      check.canonicalName()
+                          + " is not disableable and may not be demoted to a warning");
+                }
+                severities.put(check.canonicalName(), SeverityLevel.WARNING);
+                disabled.remove(check.canonicalName());
+                break;
+              case ERROR:
+                severities.put(check.canonicalName(), SeverityLevel.ERROR);
+                disabled.remove(check.canonicalName());
+                break;
+            }
           }
         });
 

diff --git a/core/src/test/java/com/google/errorprone/CommandLineFlagTest.java b/core/src/test/java/com/google/errorprone/CommandLineFlagTest.java
@@ -247,20 +247,6 @@ public void cantOverrideNonexistentCheck() {
     }
   }
 
-  @Test
-  public void cantOverrideByAltname() {
-    ErrorProneTestCompiler compiler =
-        builder.report(ScannerSupplier.fromBugCheckerClasses(DisableableChecker.class)).build();
-    ImmutableList<JavaFileObject> sources =
-        forResources(getClass(), "CommandLineFlagTestFile.java");
-
-    InvalidCommandLineOptionException expected =
-        assertThrows(
-            InvalidCommandLineOptionException.class,
-            () -> compiler.compile(new String[] {"-Xep:foo:OFF"}, sources));
-    assertThat(expected).hasMessageThat().contains("foo is not a valid checker name");
-  }
-
   @Test
   public void ignoreUnknownChecksFlagAllowsOverridingUnknownCheck() {
     ErrorProneTestCompiler compiler = builder.build();

diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
@@ -557,24 +557,28 @@ public void allChecksAsWarningsWorks() {
   }
 
   @Test
-  public void disablingPackageLocation_suppressible() {
-    ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses(PackageLocation.class);
+  public void canSuppressViaAltName() {
+    ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses(WithAltName.class);
     ErrorProneOptions epOptions =
-        ErrorProneOptions.processArgs(ImmutableList.of("-Xep:PackageLocation:OFF"));
+        ErrorProneOptions.processArgs(ImmutableList.of("-Xep:HeresMyAltName:OFF"));
 
     ScannerSupplier overrides = ss.applyOverrides(epOptions);
-    assertScanner(overrides).hasEnabledChecks(); // no checks are enabled
+    assertScanner(overrides).hasEnabledChecks(/* empty */ );
   }
 
   /** An unsuppressible version of {@link PackageLocation}. */
   @BugPattern(
       name = "PackageLocation",
       summary = "",
+      altNames = {"AlternativePackageLocation"},
       severity = ERROR,
       suppressionAnnotations = {},
       disableable = false)
   public static class UnsuppressiblePackageLocation extends PackageLocation {}
 
+  @BugPattern(altNames = "HeresMyAltName", summary = "", severity = ERROR)
+  public static class WithAltName extends PackageLocation {}
+
   @Test
   public void disablingPackageLocation_unsuppressible() {
     ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses(UnsuppressiblePackageLocation.class);
@@ -586,6 +590,17 @@ public void disablingPackageLocation_unsuppressible() {
     assertThat(exception).hasMessageThat().contains("may not be disabled");
   }
 
+  @Test
+  public void disablingPackageLocation_viaAltName_unsuppressible() {
+    ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses(UnsuppressiblePackageLocation.class);
+    ErrorProneOptions epOptions =
+        ErrorProneOptions.processArgs(ImmutableList.of("-Xep:AlternativePackageLocation:OFF"));
+
+    InvalidCommandLineOptionException exception =
+        assertThrows(InvalidCommandLineOptionException.class, () -> ss.applyOverrides(epOptions));
+    assertThat(exception).hasMessageThat().contains("may not be disabled");
+  }
+
   private static class ScannerSupplierSubject extends Subject {
     private final ScannerSupplier actual;