Add a helper to ASTHelpers for checking whether a symbol is {effectiv…

…ely,} final. This seems common and fiddly enough to be useful. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=287826152
HubSpot · Feb 19, 2020 · 519e817 · 519e817
1 parent cabe27c
commit 519e817
Show file tree

Hide file tree

Showing 13 changed files with 30 additions and 37 deletions.
diff --git a/...pi/src/main/java/com/google/errorprone/matchers/CompileTimeConstantExpressionMatcher.java b/...pi/src/main/java/com/google/errorprone/matchers/CompileTimeConstantExpressionMatcher.java
@@ -17,6 +17,7 @@
 package com.google.errorprone.matchers;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.annotations.CompileTimeConstant;
@@ -25,7 +26,6 @@
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.util.SimpleTreeVisitor;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import javax.lang.model.element.ElementKind;
 
@@ -119,8 +119,7 @@ public boolean matches(ExpressionTree t, VisitorState state) {
         return false;
       }
       // Check that the symbol is final
-      if ((varSymbol.flags() & Flags.FINAL) != Flags.FINAL
-          && (varSymbol.flags() & Flags.EFFECTIVELY_FINAL) != Flags.EFFECTIVELY_FINAL) {
+      if (!isConsideredFinal(varSymbol)) {
         return false;
       }
       // Check if the symbol has the @CompileTimeConstant annotation.

diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
@@ -1861,4 +1861,9 @@ public static ClassSymbol outermostClass(Symbol symbol) {
     }
     return curr;
   }
+
+  /** Returns whether {@code symbol} is final or effectively final. */
+  public static boolean isConsideredFinal(Symbol symbol) {
+    return (symbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) != 0;
+  }
 }
diff --git a/check_api/src/main/java/com/google/errorprone/util/FindIdentifiers.java b/check_api/src/main/java/com/google/errorprone/util/FindIdentifiers.java
@@ -16,6 +16,8 @@
 
 package com.google.errorprone.util;
 
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -39,7 +41,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Kinds.KindSelector;
 import com.sun.tools.javac.code.Scope;
 import com.sun.tools.javac.code.Scope.WriteableScope;
@@ -398,7 +399,7 @@ private static boolean isVisible(VarSymbol var, final TreePath path) {
                         && ((NewClassTree) curr).getClassBody() != null)
                     || (curr.getKind() == Kind.CLASS && parent.getKind() == Kind.BLOCK),
             (curr, unused) -> Objects.equals(var.owner, ASTHelpers.getSymbol(curr)))) {
-          if ((var.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) == 0) {
+          if (!isConsideredFinal(var)) {
             return false;
           }
         }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMustBeClosedChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMustBeClosedChecker.java
@@ -24,6 +24,7 @@
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSameType;
 import static com.google.errorprone.util.ASTHelpers.isSubtype;
 
@@ -43,7 +44,6 @@
 import com.sun.source.tree.TryTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Type;
@@ -177,7 +177,7 @@ private static MethodTree enclosingMethod(VisitorState state) {
   }
 
   private boolean tryFinallyClose(VarSymbol var, TreePath path, VisitorState state) {
-    if ((var.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) == 0) {
+    if (!isConsideredFinal(var)) {
       return false;
     }
     Tree parent = path.getParentPath().getLeaf();

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimes.java b/core/src/main/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimes.java
@@ -21,6 +21,7 @@
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.Multiset;
@@ -41,7 +42,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.TryTree;
 import com.sun.source.util.TreePathScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import java.util.HashMap;
@@ -75,8 +75,7 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) {
             && getCurrentPath().getParentPath().getLeaf() instanceof StatementTree
             && CHECK_NOT_NULL.matches(tree, state)) {
           Symbol symbol = getSymbol(arguments.get(0));
-          if (symbol instanceof VarSymbol
-              && (symbol.flags() & (Flags.EFFECTIVELY_FINAL | Flags.FINAL)) != 0) {
+          if (symbol instanceof VarSymbol && isConsideredFinal(symbol)) {
             variables.add((VarSymbol) symbol);
             lastCheck.put((VarSymbol) symbol, tree);
           }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DeduplicateConstants.java b/core/src/main/java/com/google/errorprone/bugpatterns/DeduplicateConstants.java
@@ -17,6 +17,7 @@
 package com.google.errorprone.bugpatterns;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.Table;
@@ -33,7 +34,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.util.Name;
 import java.util.Collection;
@@ -161,7 +161,7 @@ private void saveConstValue(VariableTree tree, Scope scope) {
         if (sym == null) {
           return;
         }
-        if ((sym.flags() & (Flags.EFFECTIVELY_FINAL | Flags.FINAL)) == 0) {
+        if (!isConsideredFinal(sym)) {
           return;
         }
         // heuristic: long string constants are generally more interesting than short ones, or

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ModifiedButNotUsed.java b/core/src/main/java/com/google/errorprone/bugpatterns/ModifiedButNotUsed.java
@@ -28,6 +28,7 @@
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static java.util.stream.Collectors.joining;
 
 import com.google.common.collect.ImmutableList;
@@ -61,7 +62,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreePathScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import java.util.ArrayList;
@@ -197,7 +197,7 @@ public class ModifiedButNotUsed extends BugChecker
   @Override
   public Description matchVariable(VariableTree tree, VisitorState state) {
     VarSymbol symbol = getSymbol(tree);
-    if (!effectivelyFinal(symbol)) {
+    if (!isConsideredFinal(symbol)) {
       return NO_MATCH;
     }
     if (state.getPath().getParentPath().getLeaf() instanceof ClassTree) {
@@ -389,8 +389,4 @@ private ImmutableList<SuggestedFix> buildFixes(List<TreePath> removals) {
           : ImmutableList.of(withoutSideEffects.build());
     }
   }
-
-  private static boolean effectivelyFinal(Symbol symbol) {
-    return (symbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) != 0;
-  }
 }
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NonFinalCompileTimeConstant.java b/core/src/main/java/com/google/errorprone/bugpatterns/NonFinalCompileTimeConstant.java
@@ -18,8 +18,7 @@
 
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
-import static com.sun.tools.javac.code.Flags.EFFECTIVELY_FINAL;
-import static com.sun.tools.javac.code.Flags.FINAL;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
@@ -55,8 +54,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
       if (!CompileTimeConstantExpressionMatcher.hasCompileTimeConstantAnnotation(state, sym)) {
         continue;
       }
-      if ((sym.flags() & FINAL) == FINAL
-          || (sym.flags() & EFFECTIVELY_FINAL) == EFFECTIVELY_FINAL) {
+      if (isConsideredFinal(sym)) {
         continue;
       }
       return describeMatch(parameter);

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ProtoFieldNullComparison.java b/core/src/main/java/com/google/errorprone/bugpatterns/ProtoFieldNullComparison.java
@@ -26,6 +26,7 @@
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
 import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSameType;
 
 import com.google.common.collect.ImmutableList;
@@ -54,7 +55,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.SimpleTreeVisitor;
 import com.sun.source.util.TreePathScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Type;
@@ -197,7 +197,7 @@ public Void visitClass(ClassTree clazz, Void unused) {
     @Override
     public Void visitVariable(VariableTree variable, Void unused) {
       Symbol symbol = ASTHelpers.getSymbol(variable);
-      if (variable.getInitializer() != null && isEffectivelyFinal(symbol)) {
+      if (variable.getInitializer() != null && symbol != null && isConsideredFinal(symbol)) {
         if (descendIntoInitializers) {
           getInitializer(variable.getInitializer())
               .ifPresent(e -> effectivelyFinalValues.put(symbol, e));
@@ -276,10 +276,6 @@ public Void visitMethodInvocation(MethodInvocationTree node, Void unused) {
       return super.visitMethodInvocation(node, null);
     }
 
-    private boolean isEffectivelyFinal(@Nullable Symbol symbol) {
-      return symbol != null && (symbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) != 0;
-    }
-
     private Optional<Fixer> getFixer(ExpressionTree tree, VisitorState state) {
       ExpressionTree resolvedTree = getEffectiveTree(tree);
       if (resolvedTree == null || !PROTO_RECEIVER.matches(resolvedTree, state)) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/VarChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/VarChecker.java
@@ -17,6 +17,7 @@
 package com.google.errorprone.bugpatterns;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.ProvidesFix;
@@ -32,7 +33,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.tree.JCTree;
@@ -107,7 +107,7 @@ private Description handleLocalOrParam(VariableTree tree, VisitorState state, Sy
       // flow information isn't collected for body-less methods
       return Description.NO_MATCH;
     }
-    if ((sym.flags() & (Flags.EFFECTIVELY_FINAL | Flags.FINAL)) != 0) {
+    if (isConsideredFinal(sym)) {
       return Description.NO_MATCH;
     }
     return describeMatch(tree, addVarAnnotation(tree));

diff --git a/...ain/java/com/google/errorprone/bugpatterns/formatstring/StrictFormatStringValidation.java b/...ain/java/com/google/errorprone/bugpatterns/formatstring/StrictFormatStringValidation.java
@@ -17,6 +17,7 @@
 package com.google.errorprone.bugpatterns.formatstring;
 
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
@@ -31,7 +32,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
@@ -85,7 +85,7 @@ public static ValidationResult validate(
               formatStringTree));
     }
 
-    if ((formatStringSymbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) == 0) {
+    if (!isConsideredFinal(formatStringSymbol)) {
       return ValidationResult.create(
           null, "All variables passed as @FormatString must be final or effectively final");
     }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsBrokenForNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsBrokenForNull.java
@@ -19,6 +19,7 @@
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.ProvidesFix;
@@ -45,7 +46,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreePathScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import java.util.HashSet;
@@ -101,8 +101,7 @@ public Void visitVariable(VariableTree variableTree, Void unused) {
         // Track variables assigned from our parameter.
         Tree initializer = variableTree.getInitializer();
         VarSymbol symbol = getSymbol(variableTree);
-        if ((symbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) != 0
-            && initializer instanceof InstanceOfTree) {
+        if (isConsideredFinal(symbol) && initializer instanceof InstanceOfTree) {
           InstanceOfTree instanceOf = (InstanceOfTree) initializer;
           if (instanceOf.getExpression() instanceof IdentifierTree
               && incomingVariableSymbols.contains(getSymbol(instanceOf.getExpression()))) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/StronglyTypeDuration.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/StronglyTypeDuration.java
@@ -20,6 +20,7 @@
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSameType;
 
 import com.google.common.collect.HashMultimap;
@@ -44,7 +45,6 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePathScanner;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Type;
@@ -169,7 +169,7 @@ public Void visitVariable(VariableTree variableTree, Void unused) {
         Type type = state.getTypes().unboxedTypeOrType(symbol.type);
         if (symbol.getKind() == ElementKind.FIELD
             && symbol.getModifiers().contains(Modifier.PRIVATE)
-            && (symbol.flags() & (Flags.FINAL | Flags.EFFECTIVELY_FINAL)) != 0
+            && isConsideredFinal(symbol)
             && variableTree.getInitializer() != null
             && (isSameType(type, state.getSymtab().intType, state)
                 || isSameType(type, state.getSymtab().longType, state))