Fix PatternMatchingInstanceof false positive with generics

Include generics when checking that the instanceof and cast types match. `ASTHelpers#isSameType` erases the types before doing a comparison, for unclear historical reasons. Fixes #4421 PiperOrigin-RevId: 639904327
google · Jun 4, 2024 · 0d5bd81 · 0d5bd81
1 parent 2941a10
commit 0d5bd81
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 2 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceof.java b/core/src/main/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceof.java
@@ -20,7 +20,6 @@
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
-import static com.google.errorprone.util.ASTHelpers.isSameType;
 import static com.google.errorprone.util.SourceVersion.supportsPatternMatchingInstanceof;
 
 import com.google.common.collect.ImmutableSet;
@@ -76,7 +75,7 @@ public Description matchIf(IfTree tree, VisitorState state) {
         instanceofChecks.stream()
             .filter(
                 i ->
-                    isSameType(getType(i.getType()), getType(typeCast.getType()), state)
+                    state.getTypes().isSameType(getType(i.getType()), getType(typeCast.getType()))
                         && getSymbol(i.getExpression()) instanceof VarSymbol
                         && getSymbol(i.getExpression()).equals(getSymbol(typeCast.getExpression())))
             .findFirst()

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceofTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceofTest.java
@@ -148,4 +148,23 @@ public void differentVariable() {
         .expectUnchanged()
         .doTest();
   }
+
+  @Test
+  public void generic() {
+    helper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Map;",
+            "class Test {",
+            "  void test(Object x, String k) {",
+            "    if (x instanceof Map) {",
+            "      @SuppressWarnings(\"unchecked\")",
+            "      Map<String, Integer> m = (Map<String, Integer>) x;",
+            "      System.err.println(m.get(k));",
+            "    }",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
 }