MOE Sync 2020-05-17

check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java

@@ -56,6 +56,7 @@ public class ErrorProneOptions {
   private static final String ENABLE_ALL_CHECKS = "-XepAllDisabledChecksAsWarnings";
   private static final String IGNORE_SUPPRESSION_ANNOTATIONS = "-XepIgnoreSuppressionAnnotations";
   private static final String DISABLE_ALL_CHECKS = "-XepDisableAllChecks";
+  private static final String DISABLE_ALL_WARNINGS = "-XepDisableAllWarnings";
   private static final String IGNORE_UNKNOWN_CHECKS_FLAG = "-XepIgnoreUnknownCheckNames";
   private static final String DISABLE_WARNINGS_IN_GENERATED_CODE_FLAG =
       "-XepDisableWarningsInGeneratedCode";
@@ -75,7 +76,8 @@ public static int isSupportedOption(String option) {
             || option.equals(ENABLE_ALL_CHECKS)
             || option.equals(DISABLE_ALL_CHECKS)
             || option.equals(IGNORE_SUPPRESSION_ANNOTATIONS)
-            || option.equals(COMPILING_TEST_ONLY_CODE);
+            || option.equals(COMPILING_TEST_ONLY_CODE)
+            || option.equals(DISABLE_ALL_WARNINGS);
     return isSupported ? 0 : -1;
   }
 
@@ -156,6 +158,7 @@ final PatchingOptions build() {
   private final ImmutableMap<String, Severity> severityMap;
   private final boolean ignoreUnknownChecks;
   private final boolean disableWarningsInGeneratedCode;
+  private final boolean disableAllWarnings;
   private final boolean dropErrorsToWarnings;
   private final boolean enableAllChecksAsWarnings;
   private final boolean disableAllChecks;
@@ -171,6 +174,7 @@ private ErrorProneOptions(
       ImmutableList<String> remainingArgs,
       boolean ignoreUnknownChecks,
       boolean disableWarningsInGeneratedCode,
+      boolean disableAllWarnings,
       boolean dropErrorsToWarnings,
       boolean enableAllChecksAsWarnings,
       boolean disableAllChecks,
@@ -184,6 +188,7 @@ private ErrorProneOptions(
     this.remainingArgs = remainingArgs;
     this.ignoreUnknownChecks = ignoreUnknownChecks;
     this.disableWarningsInGeneratedCode = disableWarningsInGeneratedCode;
+    this.disableAllWarnings = disableAllWarnings;
     this.dropErrorsToWarnings = dropErrorsToWarnings;
     this.enableAllChecksAsWarnings = enableAllChecksAsWarnings;
     this.disableAllChecks = disableAllChecks;
@@ -211,6 +216,10 @@ public boolean disableWarningsInGeneratedCode() {
     return disableWarningsInGeneratedCode;
   }
 
+  public boolean isDisableAllWarnings() {
+    return disableAllWarnings;
+  }
+
   public boolean isDropErrorsToWarnings() {
     return dropErrorsToWarnings;
   }
@@ -241,6 +250,7 @@ public Pattern getExcludedPattern() {
 
   private static class Builder {
     private boolean ignoreUnknownChecks = false;
+    private boolean disableAllWarnings = false;
     private boolean disableWarningsInGeneratedCode = false;
     private boolean dropErrorsToWarnings = false;
     private boolean enableAllChecksAsWarnings = false;
@@ -298,6 +308,13 @@ public void setDropErrorsToWarnings(boolean dropErrorsToWarnings) {
       this.dropErrorsToWarnings = dropErrorsToWarnings;
     }
 
+    public void setDisableAllWarnings(boolean disableAllWarnings) {
+      severityMap.entrySet().stream()
+          .filter(e -> e.getValue() == Severity.WARN)
+          .forEach(e -> e.setValue(Severity.OFF));
+      this.disableAllWarnings = disableAllWarnings;
+    }
+
     public void setEnableAllChecksAsWarnings(boolean enableAllChecksAsWarnings) {
       // Checks manually disabled before this flag are reset to warning-level
       severityMap.entrySet().stream()
@@ -330,6 +347,7 @@ public ErrorProneOptions build(ImmutableList<String> remainingArgs) {
           remainingArgs,
           ignoreUnknownChecks,
           disableWarningsInGeneratedCode,
+          disableAllWarnings,
           dropErrorsToWarnings,
           enableAllChecksAsWarnings,
           disableAllChecks,
@@ -395,6 +413,9 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
         case COMPILING_TEST_ONLY_CODE:
           builder.setTestOnlyTarget(true);
           break;
+        case DISABLE_ALL_WARNINGS:
+          builder.setDisableAllWarnings(true);
+          break;
         default:
           if (arg.startsWith(SEVERITY_PREFIX)) {
             builder.parseSeverity(arg);

check_api/src/main/java/com/google/errorprone/RefactoringCollection.java

@@ -179,6 +179,7 @@ private static void writePatchFile(
           throw new UncheckedIOException(e);
         }
       }
+      Files.createDirectories(patchFilePatch.getParent());
       Files.write(patchFilePatch, patchFile.getBytes(UTF_8), APPEND, CREATE);
     }
   }

check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java

@@ -1175,7 +1175,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
         // recompiles to avoid infinite recursion.
         continue;
       }
-      if ((key.equals("-source") || key.equals("-target")) && originalOptions.isSet("--release")) {
+      if (SOURCE_TARGET_OPTIONS.contains(key) && originalOptions.isSet("--release")) {
         // javac does not allow -source and -target to be specified explicitly when --release is,
         // but does add them in response to passing --release. Here we invert that operation.
         continue;
@@ -1234,6 +1234,9 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
     return true;
   }
 
+  private static final ImmutableSet<String> SOURCE_TARGET_OPTIONS =
+      ImmutableSet.of("-source", "--source", "-target", "--target");
+
   /** Create a plausible URI to use in {@link #compilesWithFix}. */
   @VisibleForTesting
   static URI sourceURI(URI uri) {

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

@@ -166,6 +166,11 @@ public ScannerSupplier applyOverrides(ErrorProneOptions errorProneOptions) {
           .forEach(c -> severities.put(c.canonicalName(), SeverityLevel.WARNING));
     }
 
+    if (errorProneOptions.isDisableAllWarnings()) {
+      getAllChecks().values().stream()
+          .filter(c -> c.defaultSeverity() == SeverityLevel.WARNING)
+          .forEach(c -> disabled.add(c.canonicalName()));
+    }
     if (errorProneOptions.isDisableAllChecks()) {
       getAllChecks().values().stream()
           .filter(c -> c.disableable())

check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java

@@ -161,6 +161,13 @@ public void recognizesCompilingTestOnlyCode() {
     assertThat(options.isTestOnlyTarget()).isTrue();
   }
 
+  @Test
+  public void recognizesDisableAllWarnings() {
+    ErrorProneOptions options =
+        ErrorProneOptions.processArgs(new String[] {"-XepDisableAllWarnings"});
+    assertThat(options.isDisableAllWarnings()).isTrue();
+  }
+
   @Test
   public void recognizesVisitSuppressedCode() {
     ErrorProneOptions options =

core/src/main/java/com/google/errorprone/bugpatterns/CanBeStaticAnalyzer.java

@@ -69,23 +69,27 @@ private CanBeStaticAnalyzer(Symbol owner, VisitorState state) {
   public void visitIdent(JCTree.JCIdent tree) {
     // check for unqualified references to instance members (fields and methods) declared
     // in an enclosing scope
-    if (tree.sym.isStatic()) {
+    Symbol sym = tree.sym;
+    if (sym == null) {
       return;
     }
-    switch (tree.sym.getKind()) {
+    if (sym.isStatic()) {
+      return;
+    }
+    switch (sym.getKind()) {
       case TYPE_PARAMETER:
         // declaring a class as non-static just to access a type parameter is silly -
         // why not just re-declare the type parameter instead of capturing it?
         // TODO(cushon): consider making the suggestion anyways, maybe with a fix?
         // fall through
       case FIELD:
-        if (!isOwnedBy(tree.sym, owner, state.getTypes())) {
+        if (!isOwnedBy(sym, owner, state.getTypes())) {
           canPossiblyBeStatic = false;
         }
         break;
       case METHOD:
-        if (!isOwnedBy(tree.sym, owner, state.getTypes())) {
-          outerReferences.add((MethodSymbol) tree.sym);
+        if (!isOwnedBy(sym, owner, state.getTypes())) {
+          outerReferences.add((MethodSymbol) sym);
         }
         break;
       case CLASS:

core/src/main/java/com/google/errorprone/bugpatterns/DefaultPackage.java

@@ -18,6 +18,7 @@
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 
+import com.google.common.io.Files;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
@@ -53,6 +54,10 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
     if (tree.getPackageName() != null) {
       return Description.NO_MATCH;
     }
+    // module-info.* is a special file name so whitelisting it.
+    if (Files.getNameWithoutExtension(ASTHelpers.getFileName(tree)).equals("module-info")) {
+      return Description.NO_MATCH;
+    }
     return describeMatch(tree);
   }
 }

core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByUtils.java

@@ -44,7 +44,7 @@ public static ImmutableSet<String> getGuardValues(Symbol sym) {
   static ImmutableSet<String> getGuardValues(Tree tree, VisitorState state) {
     Symbol sym = getSymbol(tree);
     if (sym == null) {
-      return null;
+      return ImmutableSet.of();
     }
     return getAnnotationValueAsStrings(sym);
   }

core/src/main/java/com/google/errorprone/refaster/Inliner.java

@@ -24,6 +24,7 @@
 import com.google.errorprone.refaster.Bindings.Key;
 import com.google.errorprone.refaster.UTypeVar.TypeWithExpression;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.RuntimeVersion;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.ClassSymbol;
 import com.sun.tools.javac.code.Symbol.TypeSymbol;
@@ -228,11 +229,24 @@ public TypeVar inlineAsVar(UTypeVar var) throws CouldNotResolveImportException {
     sym.type = typeVar;
     typeVarCache.put(var.getName(), typeVar);
     // Any recursive uses of var will point to the same TypeVar object generated above.
-    typeVar.bound = var.getUpperBound().inline(this);
+    setUpperBound(typeVar, var.getUpperBound().inline(this));
     typeVar.lower = var.getLowerBound().inline(this);
     return typeVar;
   }
 
+  private static void setUpperBound(TypeVar typeVar, Type bound) {
+    try {
+      // https://bugs.openjdk.java.net/browse/JDK-8193367
+      if (RuntimeVersion.isAtLeast13()) {
+        TypeVar.class.getMethod("setUpperBound", Type.class).invoke(typeVar, bound);
+      } else {
+        TypeVar.class.getField("bound").set(typeVar, bound);
+      }
+    } catch (ReflectiveOperationException e) {
+      throw new LinkageError(e.getMessage(), e);
+    }
+  }
+
   Type inlineTypeVar(UTypeVar var) throws CouldNotResolveImportException {
     Optional<TypeWithExpression> typeVarBinding = getOptionalBinding(var.key());
     if (typeVarBinding.isPresent()) {

core/src/main/java/com/google/errorprone/refaster/PlaceholderUnificationVisitor.java

@@ -16,13 +16,16 @@
 
 package com.google.errorprone.refaster;
 
+import static com.google.common.base.Preconditions.checkState;
+
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.errorprone.refaster.PlaceholderUnificationVisitor.State;
 import com.google.errorprone.refaster.UPlaceholderExpression.PlaceholderParamIdent;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.RuntimeVersion;
 import com.sun.source.tree.ArrayAccessTree;
 import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.BinaryTree;
@@ -517,7 +520,7 @@ public Choice<State<JCAssignOp>> visitCompoundAssignment(
             state,
             s -> unifyExpression(node.getVariable(), s),
             s -> unifyExpression(node.getExpression(), s),
-            (var, expr) -> maker().Assignop(((JCAssignOp) node).getTag(), var, expr))
+            (variable, expr) -> maker().Assignop(((JCAssignOp) node).getTag(), variable, expr))
         .condition(assignOp -> !(assignOp.result().getVariable() instanceof PlaceholderParamIdent));
   }
 
@@ -663,9 +666,39 @@ public Choice<State<JCSwitch>> visitSwitch(final SwitchTree node, State<?> state
   @Override
   public Choice<State<JCCase>> visitCase(final CaseTree node, State<?> state) {
     return chooseSubtrees(
-        state,
-        s -> unifyStatements(node.getStatements(), s),
-        stmts -> maker().Case((JCExpression) node.getExpression(), stmts));
+        state, s -> unifyStatements(node.getStatements(), s), stmts -> makeCase(node, stmts));
+  }
+
+  private JCCase makeCase(CaseTree node, List<JCStatement> stmts) {
+    try {
+      if (RuntimeVersion.isAtLeast12()) {
+        Enum<?> caseKind = (Enum) CaseTree.class.getMethod("getCaseKind").invoke(node);
+        checkState(
+            caseKind.name().contentEquals("STATEMENT"),
+            "expression switches are not supported yet");
+        return (JCCase)
+            TreeMaker.class
+                .getMethod(
+                    "Case",
+                    Class.forName("com.sun.source.tree.CaseTree.CaseKind"),
+                    List.class,
+                    List.class,
+                    JCTree.class)
+                .invoke(
+                    maker(),
+                    caseKind,
+                    List.of((JCExpression) node.getExpression()),
+                    stmts,
+                    /* body= */ null);
+      } else {
+        return (JCCase)
+            TreeMaker.class
+                .getMethod("Case", JCExpression.class, List.class)
+                .invoke(maker(), node.getExpression(), stmts);
+      }
+    } catch (ReflectiveOperationException e) {
+      throw new LinkageError(e.getMessage(), e);
+    }
   }
 
   @Override