Disallow demoting an error to a warning if the check is currently ena…

…bled and

is not disableable.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=79685818

core/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java

@@ -160,6 +160,14 @@ public ScannerSupplier applyOverrides(Map<String, Severity> severityMap)
           enabledChecks.add(supplier);
           break;
         case WARN:
+          // Demoting an enabled check from an error to a warning is a form of disabling
+          if (enabledChecks.contains(supplier)
+              && !supplier.disableable()
+              && supplier.severity() == SeverityLevel.ERROR) {
+            throw new InvalidCommandLineOptionException(supplier.canonicalName()
+                + " is not disableable and may not be demoted to a warning");
+          }
+
           // When the severity of a check is overridden, a new BugCheckerSupplier is produced.
           // The old BugCheckerSupplier must be removed from allChecks and enabledChecks,
           // and the new BugCheckerSupplier must be added.

core/src/test/java/com/google/errorprone/CommandLineFlagTest.java

@@ -86,7 +86,7 @@ public Description matchReturn(ReturnTree tree, VisitorState state) {
   @BugPattern(name = "ErrorChecker",
       summary = "Checker that flags all return statements as errors",
       explanation = "Checker that flags all return statements as errors",
-      category = ONE_OFF, severity = ERROR, maturity = MATURE)
+      category = ONE_OFF, severity = ERROR, maturity = MATURE, disableable = true)
   private static class ErrorChecker extends BugChecker implements ReturnTreeMatcher {
     @Override
     public Description matchReturn(ReturnTree tree, VisitorState state) {

core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java

@@ -355,17 +355,15 @@ public void flagEnablesCheck() throws Exception {
   @Test
   public void severityIsResetOnNextCompilation() throws Exception {
     String[] testFile = {"public class Test {",
-        "  public static int shiftBy43(int toShift) {",
-        "    return toShift << 43;",
-        "  }",
+        "  long myLong = 213124l;",
         "}"};
 
-    String[] args = {"-Xep:BadShiftAmount:WARN"};
+    String[] args = {"-Xep:LongLiteralLowerCaseSuffix:WARN"};
     Result exitCode = compiler.compile(args,
         Arrays.asList(compiler.fileManager().forSourceLines("Test.java", testFile)));
     outputStream.flush();
     Matcher<Iterable<Diagnostic<JavaFileObject>>> matcher = hasItem(
-        diagnosticMessage(containsString("[BadShiftAmount]")));
+        diagnosticMessage(containsString("[LongLiteralLowerCaseSuffix]")));
     assertThat(outputStream.toString(), exitCode, is(Result.OK));
     assertTrue(
         "Warning should be found. " + diagnosticHelper.describe(),

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

@@ -32,6 +32,7 @@
 import com.google.errorprone.bugpatterns.ArrayEqualsTest;
 import com.google.errorprone.bugpatterns.BadShiftAmount;
 import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.ChainingConstructorIgnoresParameter;
 import com.google.errorprone.bugpatterns.DepAnnTest;
 import com.google.errorprone.bugpatterns.EmptyIfStatementTest;
 import com.google.errorprone.bugpatterns.Finally;
@@ -297,9 +298,9 @@ public void testSeverityResetsAfterOverride() throws Exception {
     List<String> args = Lists.newArrayList(
         "-d", tempDir.getRoot().getAbsolutePath(),
         "-proc:none",
-        "-Xep:BadShiftAmount:WARN");
-    List<JavaFileObject> sources =
-        fileManager.sources(BadShiftAmount.class, "BadShiftAmountPositiveCases.java");
+        "-Xep:ChainingConstructorIgnoresParameter:WARN");
+    List<JavaFileObject> sources = fileManager.sources(ChainingConstructorIgnoresParameter.class,
+        "ChainingConstructorIgnoresParameterPositiveCases.java");
     fileManager.close();
 
     JavaCompiler.CompilationTask task = errorProneJavaCompiler.getTask(
@@ -312,15 +313,16 @@ public void testSeverityResetsAfterOverride() throws Exception {
     boolean succeeded = task.call();
     assertThat(succeeded).isTrue();
     Matcher<Iterable<Diagnostic<JavaFileObject>>> matcher =
-        hasItem(diagnosticMessage(containsString("[BadShiftAmount]")));
+        hasItem(diagnosticMessage(containsString("[ChainingConstructorIgnoresParameter]")));
     assertTrue(matcher.matches(diagnosticHelper.getDiagnostics()));
 
     // reset state between compilations
     diagnosticHelper.clearDiagnostics();
     fileManager = new ErrorProneInMemoryFileManager();
-    sources = fileManager.sources(BadShiftAmount.class, "BadShiftAmountPositiveCases.java");
+    sources = fileManager.sources(ChainingConstructorIgnoresParameter.class,
+        "ChainingConstructorIgnoresParameterPositiveCases.java");
     fileManager.close();
-    args.remove("-Xep:BadShiftAmount:WARN");
+    args.remove("-Xep:ChainingConstructorIgnoresParameter:WARN");
 
     task = errorProneJavaCompiler.getTask(
         printWriter,

core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java

@@ -197,29 +197,46 @@ public void applyOverridesThrowsExceptionWhenDisablingNonDisablableCheck() throw
     }
   }
 
+  @Test
+  // Calling ScannerSupplier.applyOverrides() just to make sure it throws the right exception
+  @SuppressWarnings("CheckReturnValue")
+  public void applyOverridesThrowsExceptionWhenDemotingNonDisablableCheck() throws Exception {
+    ScannerSupplier ss = ScannerSupplier.fromBugCheckers(new ArrayEquals());
+    Map<String, Severity> overrideMap = ImmutableMap.of("ArrayEquals", Severity.WARN);
+
+    try {
+      ss.applyOverrides(overrideMap);
+      fail();
+    } catch (InvalidCommandLineOptionException expected) {
+      assertThat(expected.getMessage()).contains("may not be demoted to a warning");
+    }
+  }
+
   @Test
   public void applyOverridesSetsSeverity() throws Exception {
     ScannerSupplier ss = ScannerSupplier.fromBugCheckers(
-        new ArrayEquals(),
         new BadShiftAmount(),
+        new ChainingConstructorIgnoresParameter(),
         new StringEquality());
     Map<String, Severity> overrideMap = ImmutableMap.of(
-        "ArrayEquals", Severity.WARN,
+        "ChainingConstructorIgnoresParameter", Severity.WARN,
         "StringEquality", Severity.ERROR);
     ScannerSupplier overriddenScannerSupplier = ss.applyOverrides(overrideMap);
 
     Map<String, BugCheckerSupplier> unexpected = ImmutableMap.of(
-        "ArrayEquals", fromInstance(new ArrayEquals()),
         "BadShiftAmount", fromInstance(new BadShiftAmount()),
+        "ChainingConstructorIgnoresParameter", fromInstance(
+            new ChainingConstructorIgnoresParameter()),
         "StringEquality", fromInstance(new StringEquality()));
     assertThat(overriddenScannerSupplier.getAllChecks()).isNotEqualTo(unexpected);
 
-    BugChecker arrayEqualsWithWarningSeverity = new ArrayEquals();
-    arrayEqualsWithWarningSeverity.setSeverity(SeverityLevel.WARNING);
+    BugChecker chainingConstructorCheckWithWarningSeverity =
+        new ChainingConstructorIgnoresParameter();
+    chainingConstructorCheckWithWarningSeverity.setSeverity(SeverityLevel.WARNING);
     BugChecker stringEqualityWithErrorSeverity = new StringEquality();
     stringEqualityWithErrorSeverity.setSeverity(SeverityLevel.ERROR);
     Set<BugCheckerSupplier> expected = ImmutableSet.of(
-        fromInstance(arrayEqualsWithWarningSeverity),
+        fromInstance(chainingConstructorCheckWithWarningSeverity),
         fromInstance(new BadShiftAmount()),
         fromInstance(stringEqualityWithErrorSeverity));
     assertThat(overriddenScannerSupplier.getEnabledChecks()).isEqualTo(expected);