Give PatternMatchingInstanceof a second run over EP, with the improve…

…ments in unknown commit. This catches a decent bit more. Turns out not assigning a specific variable is common. I did have to do some manual cleaning up around colliding variable names. PiperOrigin-RevId: 696893465
google · Nov 26, 2024 · 8298635 · 8298635
1 parent 81e5350
commit 8298635
Show file tree

Hide file tree

Showing 90 changed files with 536 additions and 556 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/SuppressionInfo.java b/check_api/src/main/java/com/google/errorprone/SuppressionInfo.java
@@ -158,8 +158,8 @@ public SuppressionInfo withExtendedSuppressions(
         for (Pair<MethodSymbol, Attribute> value : attr.values) {
           if (value.fst.name.equals(valueName)) {
             if (value.snd
-                instanceof Attribute.Array) { // SuppressWarnings/SuppressLint take an array
-              for (Attribute suppress : ((Attribute.Array) value.snd).values) {
+                instanceof Attribute.Array array) { // SuppressWarnings/SuppressLint take an array
+              for (Attribute suppress : array.values) {
                 String suppressedWarning = (String) suppress.getValue();
                 if (!suppressWarningsStrings.contains(suppressedWarning)) {
                   anyModification = true;

diff --git a/check_api/src/main/java/com/google/errorprone/dataflow/AccessPath.java b/check_api/src/main/java/com/google/errorprone/dataflow/AccessPath.java
@@ -160,14 +160,14 @@ public static AccessPath fromVariableDecl(VariableDeclarationNode node) {
    * path and null otherwise
    */
   public static @Nullable AccessPath fromNodeIfTrackable(Node node) {
-    if (node instanceof LocalVariableNode) {
-      return fromLocalVariable((LocalVariableNode) node);
-    } else if (node instanceof VariableDeclarationNode) {
-      return fromVariableDecl((VariableDeclarationNode) node);
-    } else if (node instanceof FieldAccessNode) {
-      return fromFieldAccess((FieldAccessNode) node);
-    } else if (node instanceof AssignmentNode) {
-      return fromNodeIfTrackable(((AssignmentNode) node).getTarget());
+    if (node instanceof LocalVariableNode localVariableNode) {
+      return fromLocalVariable(localVariableNode);
+    } else if (node instanceof VariableDeclarationNode variableDeclarationNode) {
+      return fromVariableDecl(variableDeclarationNode);
+    } else if (node instanceof FieldAccessNode fieldAccessNode) {
+      return fromFieldAccess(fieldAccessNode);
+    } else if (node instanceof AssignmentNode assignmentNode) {
+      return fromNodeIfTrackable(assignmentNode.getTarget());
     }
     return null;
   }

diff --git a/check_api/src/main/java/com/google/errorprone/dataflow/DataFlow.java b/check_api/src/main/java/com/google/errorprone/dataflow/DataFlow.java
@@ -98,11 +98,11 @@ public ControlFlowGraph load(CfgParams key) {
                   ClassTree classTree = null;
                   MethodTree methodTree = null;
                   for (Tree parent : methodPath) {
-                    if (parent instanceof MethodTree) {
-                      methodTree = (MethodTree) parent;
+                    if (parent instanceof MethodTree m) {
+                      methodTree = m;
                     }
-                    if (parent instanceof ClassTree) {
-                      classTree = (ClassTree) parent;
+                    if (parent instanceof ClassTree c) {
+                      classTree = c;
                       break;
                     }
                   }

diff --git a/...src/main/java/com/google/errorprone/dataflow/nullnesspropagation/NullnessAnnotations.java b/...src/main/java/com/google/errorprone/dataflow/nullnesspropagation/NullnessAnnotations.java
@@ -92,10 +92,10 @@ public static ImmutableList<AnnotationTree> annotationsRelevantToNullness(
 
   private static String simpleName(AnnotationTree annotation) {
     Tree annotationType = annotation.getAnnotationType();
-    if (annotationType instanceof IdentifierTree) {
-      return ((IdentifierTree) annotationType).getName().toString();
-    } else if (annotationType instanceof MemberSelectTree) {
-      return ((MemberSelectTree) annotationType).getIdentifier().toString();
+    if (annotationType instanceof IdentifierTree identifierTree) {
+      return identifierTree.getName().toString();
+    } else if (annotationType instanceof MemberSelectTree memberSelectTree) {
+      return memberSelectTree.getIdentifier().toString();
     } else {
       throw new AssertionError(annotationType.getKind());
     }

diff --git a/.../java/com/google/errorprone/dataflow/nullnesspropagation/NullnessPropagationTransfer.java b/.../java/com/google/errorprone/dataflow/nullnesspropagation/NullnessPropagationTransfer.java
@@ -468,12 +468,12 @@ Nullness visitAssignment(AssignmentNode node, SubNodeValues inputs, Updates upda
     Nullness value = inputs.valueOfSubNode(node.getExpression());
 
     Node target = node.getTarget();
-    if (target instanceof LocalVariableNode) {
-      updates.set((LocalVariableNode) target, value);
+    if (target instanceof LocalVariableNode localVariableNode) {
+      updates.set(localVariableNode, value);
     }
 
-    if (target instanceof ArrayAccessNode) {
-      setNonnullIfTrackable(updates, ((ArrayAccessNode) target).getArray());
+    if (target instanceof ArrayAccessNode arrayAccessNode) {
+      setNonnullIfTrackable(updates, arrayAccessNode.getArray());
     }
 
     if (target instanceof FieldAccessNode fieldAccess) {
@@ -681,16 +681,16 @@ private static boolean hasNonNullConstantValue(LocalVariableNode node) {
 
   private static @Nullable ClassAndField tryGetFieldSymbol(Tree tree) {
     Symbol symbol = tryGetSymbol(tree);
-    if (symbol instanceof VarSymbol) {
-      return ClassAndField.make((VarSymbol) symbol);
+    if (symbol instanceof VarSymbol varSymbol) {
+      return ClassAndField.make(varSymbol);
     }
     return null;
   }
 
   static @Nullable ClassAndMethod tryGetMethodSymbol(MethodInvocationTree tree, Types types) {
     Symbol symbol = tryGetSymbol(tree.getMethodSelect());
-    if (symbol instanceof MethodSymbol) {
-      return ClassAndMethod.make((MethodSymbol) symbol, types);
+    if (symbol instanceof MethodSymbol methodSymbol) {
+      return ClassAndMethod.make(methodSymbol, types);
     }
     return null;
   }
@@ -700,14 +700,14 @@ private static boolean hasNonNullConstantValue(LocalVariableNode node) {
    * back on core.
    */
   private static @Nullable Symbol tryGetSymbol(Tree tree) {
-    if (tree instanceof JCIdent) {
-      return ((JCIdent) tree).sym;
+    if (tree instanceof JCIdent jCIdent) {
+      return jCIdent.sym;
     }
-    if (tree instanceof JCFieldAccess) {
-      return ((JCFieldAccess) tree).sym;
+    if (tree instanceof JCFieldAccess jCFieldAccess) {
+      return jCFieldAccess.sym;
     }
-    if (tree instanceof JCVariableDecl) {
-      return ((JCVariableDecl) tree).sym;
+    if (tree instanceof JCVariableDecl jCVariableDecl) {
+      return jCVariableDecl.sym;
     }
     return null;
   }
@@ -763,8 +763,8 @@ Nullness standardFieldNullness(
     Optional<Nullness> declaredNullness = NullnessAnnotations.fromAnnotationsOn(accessed.symbol);
     if (declaredNullness.isEmpty()) {
       Type ftype = accessed.symbol.type;
-      if (ftype instanceof TypeVariable) {
-        declaredNullness = NullnessAnnotations.getUpperBound((TypeVariable) ftype);
+      if (ftype instanceof TypeVariable typeVariable) {
+        declaredNullness = NullnessAnnotations.getUpperBound(typeVariable);
       } else {
         declaredNullness = NullnessAnnotations.fromDefaultAnnotations(accessed.symbol);
       }
@@ -843,12 +843,12 @@ private Nullness returnValueNullness(MethodInvocationNode node, @Nullable ClassA
   }
 
   private static void setNonnullIfTrackable(Updates updates, Node node) {
-    if (node instanceof LocalVariableNode) {
-      updates.set((LocalVariableNode) node, NONNULL);
-    } else if (node instanceof FieldAccessNode) {
-      updates.set((FieldAccessNode) node, NONNULL);
-    } else if (node instanceof VariableDeclarationNode) {
-      updates.set((VariableDeclarationNode) node, NONNULL);
+    if (node instanceof LocalVariableNode localVariableNode) {
+      updates.set(localVariableNode, NONNULL);
+    } else if (node instanceof FieldAccessNode fieldAccessNode) {
+      updates.set(fieldAccessNode, NONNULL);
+    } else if (node instanceof VariableDeclarationNode variableDeclarationNode) {
+      updates.set(variableDeclarationNode, NONNULL);
     }
   }
 
@@ -928,8 +928,8 @@ private static List<LocalVariableNode> variablesAtIndexes(
       // TODO(cpovirk): better handling of varargs
       if (i >= 0 && i < arguments.size()) {
         Node argument = arguments.get(i);
-        if (argument instanceof LocalVariableNode) {
-          result.add((LocalVariableNode) argument);
+        if (argument instanceof LocalVariableNode localVariableNode) {
+          result.add(localVariableNode);
         }
       }
     }

diff --git a/...ava/com/google/errorprone/dataflow/nullnesspropagation/inference/InferredNullability.java b/...ava/com/google/errorprone/dataflow/nullnesspropagation/inference/InferredNullability.java
@@ -73,8 +73,8 @@ private Optional<Nullness> getNullness(InferenceVariable iv) {
     Optional<Nullness> result;
     // short-circuit and return if...
     // ...this inference variable is a `proper` bound, i.e. a concrete nullness lattice element
-    if (iv instanceof ProperInferenceVar) {
-      return Optional.of(((ProperInferenceVar) iv).nullness());
+    if (iv instanceof ProperInferenceVar properInferenceVar) {
+      return Optional.of(properInferenceVar.nullness());
       // ...we've already computed and memoized a nullness value for it.
     } else if ((result = inferredMemoTable.get(iv)) != null) {
       return result;

diff --git a/.../google/errorprone/dataflow/nullnesspropagation/inference/NullnessQualifierInference.java b/.../google/errorprone/dataflow/nullnesspropagation/inference/NullnessQualifierInference.java
@@ -162,10 +162,10 @@ private void generateConstraintsFromAnnotations(
       fromAnnotations = NullnessAnnotations.fromAnnotationsOn(inferredType);
     }
     if (!fromAnnotations.isPresent()) {
-      if (declaredType instanceof TypeVariable) {
+      if (declaredType instanceof TypeVariable typeVariable) {
         // Check bounds second so explicit annotations take precedence. Even for bounds we still use
         // equality constraint below since we have to assume the bound as the "worst" case.
-        fromAnnotations = NullnessAnnotations.getUpperBound((TypeVariable) declaredType);
+        fromAnnotations = NullnessAnnotations.getUpperBound(typeVariable);
       } else {
         // Look for a default annotation in scope of either the symbol we're looking at or, if this
         // is a type variable, the type variable declaration's scope, which is effectively the type
@@ -485,8 +485,8 @@ private void generateConstraintsForWrite(
     Optional<Nullness> fromAnnotations =
         extractExplicitNullness(lType, argSelector.isEmpty() ? decl : null);
     if (!fromAnnotations.isPresent()) {
-      if (lType instanceof TypeVariable) {
-        fromAnnotations = NullnessAnnotations.getUpperBound((TypeVariable) lType);
+      if (lType instanceof TypeVariable typeVariable) {
+        fromAnnotations = NullnessAnnotations.getUpperBound(typeVariable);
         isBound = true;
       } else {
         fromAnnotations = NullnessAnnotations.fromDefaultAnnotations(decl);

diff --git a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java
@@ -795,11 +795,11 @@ public static SuggestedFix renameMethodInvocation(
     Tree methodSelect = tree.getMethodSelect();
     Name identifier;
     int startPos;
-    if (methodSelect instanceof MemberSelectTree) {
-      identifier = ((MemberSelectTree) methodSelect).getIdentifier();
-      startPos = state.getEndPosition(((MemberSelectTree) methodSelect).getExpression());
-    } else if (methodSelect instanceof IdentifierTree) {
-      identifier = ((IdentifierTree) methodSelect).getName();
+    if (methodSelect instanceof MemberSelectTree memberSelectTree) {
+      identifier = memberSelectTree.getIdentifier();
+      startPos = state.getEndPosition(memberSelectTree.getExpression());
+    } else if (methodSelect instanceof IdentifierTree identifierTree) {
+      identifier = identifierTree.getName();
       startPos = getStartPosition(tree);
     } else {
       throw malformedMethodInvocationTree(tree);

diff --git a/check_api/src/main/java/com/google/errorprone/matchers/AnnotationMatcher.java b/check_api/src/main/java/com/google/errorprone/matchers/AnnotationMatcher.java
@@ -40,18 +40,18 @@ public AnnotationMatcher(MatchType matchType, Matcher<AnnotationTree> nodeMatche
 
   @Override
   protected Iterable<? extends AnnotationTree> getChildNodes(T tree, VisitorState state) {
-    if (tree instanceof ClassTree) {
-      return ((ClassTree) tree).getModifiers().getAnnotations();
-    } else if (tree instanceof VariableTree) {
-      return ((VariableTree) tree).getModifiers().getAnnotations();
-    } else if (tree instanceof MethodTree) {
-      return ((MethodTree) tree).getModifiers().getAnnotations();
-    } else if (tree instanceof CompilationUnitTree) {
-      return ((CompilationUnitTree) tree).getPackageAnnotations();
-    } else if (tree instanceof AnnotatedTypeTree) {
-      return ((AnnotatedTypeTree) tree).getAnnotations();
-    } else if (tree instanceof PackageTree) {
-      return ((PackageTree) tree).getAnnotations();
+    if (tree instanceof ClassTree classTree) {
+      return classTree.getModifiers().getAnnotations();
+    } else if (tree instanceof VariableTree variableTree) {
+      return variableTree.getModifiers().getAnnotations();
+    } else if (tree instanceof MethodTree methodTree) {
+      return methodTree.getModifiers().getAnnotations();
+    } else if (tree instanceof CompilationUnitTree compilationUnitTree) {
+      return compilationUnitTree.getPackageAnnotations();
+    } else if (tree instanceof AnnotatedTypeTree annotatedTypeTree) {
+      return annotatedTypeTree.getAnnotations();
+    } else if (tree instanceof PackageTree packageTree) {
+      return packageTree.getAnnotations();
     } else {
       throw new IllegalArgumentException(
           "Cannot access annotations from tree of type " + tree.getClass());

diff --git a/check_api/src/main/java/com/google/errorprone/matchers/ConstructorOfClass.java b/check_api/src/main/java/com/google/errorprone/matchers/ConstructorOfClass.java
@@ -40,8 +40,8 @@ protected Iterable<? extends MethodTree> getChildNodes(ClassTree classTree, Visi
     // Iterate over members of class (methods and fields).
     for (Tree member : classTree.getMembers()) {
       // If this member is a constructor...
-      if (member instanceof MethodTree && ASTHelpers.getSymbol(member).isConstructor()) {
-        result.add((MethodTree) member);
+      if (member instanceof MethodTree methodTree && ASTHelpers.getSymbol(member).isConstructor()) {
+        result.add(methodTree);
       }
     }
     return result.build();

diff --git a/check_api/src/main/java/com/google/errorprone/matchers/Enclosing.java b/check_api/src/main/java/com/google/errorprone/matchers/Enclosing.java
@@ -90,10 +90,10 @@ public boolean matches(T unused, VisitorState state) {
       }
       Tree enclosing = pathToEnclosing.getLeaf();
       state = state.withPath(pathToEnclosing);
-      if (enclosing instanceof BlockTree) {
-        return blockTreeMatcher.matches((BlockTree) enclosing, state);
-      } else if (enclosing instanceof CaseTree) {
-        return caseTreeMatcher.matches((CaseTree) enclosing, state);
+      if (enclosing instanceof BlockTree blockTree) {
+        return blockTreeMatcher.matches(blockTree, state);
+      } else if (enclosing instanceof CaseTree caseTree) {
+        return caseTreeMatcher.matches(caseTree, state);
       } else {
         // findEnclosing given two types must return something of one of those types
         throw new IllegalStateException("enclosing tree not a BlockTree or CaseTree");

diff --git a/check_api/src/main/java/com/google/errorprone/matchers/method/BaseMethodMatcher.java b/check_api/src/main/java/com/google/errorprone/matchers/method/BaseMethodMatcher.java
@@ -36,8 +36,8 @@ interface BaseMethodMatcher {
           // Don't match constructors as they are neither static nor instance methods.
           return null;
         }
-        if (tree instanceof MethodInvocationTree) {
-          tree = ((MethodInvocationTree) tree).getMethodSelect();
+        if (tree instanceof MethodInvocationTree methodInvocationTree) {
+          tree = methodInvocationTree.getMethodSelect();
         }
         return MethodMatchState.create(tree, (MethodSymbol) sym);
       };