Instanceof pattern variable nullness annotation checks (#1062)

eisop · Jan 15, 2025 · 730914a · 730914a
1 parent 4935c5d
commit 730914a
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 3 deletions.
diff --git a/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java b/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java
@@ -55,6 +55,7 @@
 import org.checkerframework.javacutil.TreePathUtil;
 import org.checkerframework.javacutil.TreeUtils;
 import org.checkerframework.javacutil.TreeUtilsAfterJava11;
+import org.checkerframework.javacutil.TreeUtilsAfterJava11.BindingPatternUtils;
 import org.checkerframework.javacutil.TreeUtilsAfterJava11.SwitchExpressionUtils;
 import org.checkerframework.javacutil.TypesUtils;
 
@@ -471,16 +472,31 @@ public Void visitInstanceOf(InstanceOfTree tree, Void p) {
             // Handle them properly.
             return null;
         }
+
+        List<? extends AnnotationMirror> annotations = null;
         if (refTypeTree.getKind() == Tree.Kind.ANNOTATED_TYPE) {
-            List<? extends AnnotationMirror> annotations =
-                    TreeUtils.annotationsFromTree((AnnotatedTypeTree) refTypeTree);
+            annotations = TreeUtils.annotationsFromTree((AnnotatedTypeTree) refTypeTree);
+        } else {
+            Tree patternTree = TreeUtilsAfterJava11.InstanceOfUtils.getPattern(tree);
+            if (patternTree != null && TreeUtils.isBindingPatternTree(patternTree)) {
+                VariableTree variableTree = BindingPatternUtils.getVariable(patternTree);
+                if (variableTree.getModifiers() != null) {
+                    List<? extends AnnotationTree> annotationTree =
+                            variableTree.getModifiers().getAnnotations();
+                    annotations = TreeUtils.annotationsFromTypeAnnotationTrees(annotationTree);
+                }
+            }
+        }
+
+        if (annotations != null) {
             if (AnnotationUtils.containsSame(annotations, NULLABLE)) {
                 checker.reportError(tree, "instanceof.nullable");
             }
             if (AnnotationUtils.containsSame(annotations, NONNULL)) {
                 checker.reportWarning(tree, "instanceof.nonnull.redundant");
             }
         }
+
         // Don't call super because it will issue an incorrect instanceof.unsafe warning.
         return null;
     }

diff --git a/checker/tests/nullness/java17/NullnessInstanceOf.java b/checker/tests/nullness/java17/NullnessInstanceOf.java
@@ -0,0 +1,53 @@
+// @below-java14-jdk-skip-test
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+public class NullnessInstanceOf {
+
+    public void testClassicInstanceOfNullable(Object x) {
+        // :: error: (instanceof.nullable)
+        if (x instanceof @Nullable String) {
+            System.out.println("Nullable String instanceof check.");
+        }
+    }
+
+    public void testClassicInstanceOfNonNull(Object x) {
+        // :: warning: (instanceof.nonnull.redundant)
+        if (x instanceof @NonNull Number) {
+            System.out.println("NonNull Number instanceof check.");
+        }
+    }
+
+    public void testPatternVariableNullable(Object x) {
+        // :: error: (instanceof.nullable)
+        if (x instanceof @Nullable String n) {
+            System.out.println("Length of String: " + n.length());
+        }
+    }
+
+    public void testPatternVariableNonNull(Object x) {
+        // :: warning: (instanceof.nonnull.redundant)
+        if (x instanceof @NonNull Number nn) {
+            System.out.println("Number's hashCode: " + nn.hashCode());
+        }
+    }
+
+    public void testUnannotatedClassic(Object x) {
+        if (x instanceof String) {
+            System.out.println("Unannotated String instanceof check.");
+        }
+    }
+
+    public void testUnannotatedPatternVariable(Object x) {
+        if (x instanceof String unannotatedString) {
+            System.out.println("Unannotated String length: " + unannotatedString.length());
+        }
+    }
+
+    public void testUnusedPatternVariable(Object x) {
+        // :: error: (instanceof.nullable)
+        if (x instanceof @Nullable String unusedString) {}
+        // :: warning: (instanceof.nonnull.redundant)
+        if (x instanceof @NonNull Number unusedNumber) {}
+    }
+}
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -3,11 +3,14 @@ Version 3.42.0-eisop6 (January ??, 2025)
 
 **User-visible changes:**
 
+The Nullness Checker now reports an error if any instanceof pattern variables are annotated with `@Nullable`
+and a redundant warning if they are annotated with `@NonNull`.
+
 **Implementation details:**
 
 **Closed issues:**
 
-eisop#1003, eisop#1033.
+eisop#1003, eisop#1033, eisop#1058.
 
 
 Version 3.42.0-eisop5 (December 20, 2024)