Address review comments

uber · Aug 20, 2024 · 4b6187f · 4b6187f
1 parent e497e1b
commit 4b6187f
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 17 deletions.
diff --git a/nullaway/src/test/java/com/uber/nullaway/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/ArrayTests.java
@@ -52,7 +52,7 @@ public void arrayDeclarationAnnotation() {
 
   @Test
   public void arrayLegacyDeclarationAnnotation() {
-    makeHelper()
+    makeLegacyModeHelper()
         .addSourceLines(
             "Test.java",
             "package com.uber;",
@@ -72,11 +72,11 @@ public void arrayLegacyDeclarationAnnotation() {
 
   @Test
   public void typeUseLegacyAnnotationOnArray() {
-    makeHelper()
+    makeLegacyModeHelper()
         .addSourceLines(
             "Test.java",
             "package com.uber;",
-            "import org.checkerframework.checker.nullness.qual.Nullable;",
+            "import org.jspecify.annotations.Nullable;",
             "class Test {",
             "  // ok only for backwards compat",
             "  @Nullable Object[] foo1 = null;",
@@ -88,7 +88,7 @@ public void typeUseLegacyAnnotationOnArray() {
             "  @Nullable Object [][] foo4 = null;",
             "  // ok according to spec",
             "  Object @Nullable [][] foo5 = null;",
-            "  // NOT ok; @Nullable applies to first array dimension not the elements or the array ref",
+            "  // ok, but @Nullable applies to first array dimension not the elements or the array ref",
             "  Object [] @Nullable [] foo6 = null;",
             "}")
         .doTest();
@@ -100,16 +100,15 @@ public void typeUseAnnotationOnArray() {
         .addSourceLines(
             "Test.java",
             "package com.uber;",
-            "import org.checkerframework.checker.nullness.qual.Nullable;",
+            "import org.jspecify.annotations.Nullable;",
             "class Test {",
             "  // @Nullable is not applied on top-level of array",
             "  // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field",
             "  @Nullable Object[] foo1 = null;",
             "  // ok according to spec",
             "  Object @Nullable[] foo2 = null;",
-            "  // ok, but @Nullable is not applied on top-level of array ",
+            "  // ok according to spec",
             "  @Nullable Object @Nullable [] foo3 = null;",
-            "  // ok only for backwards compat",
             "  // @Nullable is not applied on top-level of array",
             "  // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field",
             "  @Nullable Object [][] foo4 = null;",
@@ -126,9 +125,12 @@ public void typeUseAnnotationOnArray() {
   public void typeUseAndDeclarationAnnotationOnArray() {
     defaultCompilationHelper
         .addSourceLines(
-            "Test.java",
+            "Nullable.java",
             "package com.uber;",
-            "import org.jetbrains.annotations.Nullable;",
+            "import java.lang.annotation.ElementType;",
+            "import java.lang.annotation.Target;",
+            "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})",
+            "public @interface Nullable {}",
             "class Test {",
             "  @Nullable Object[] foo1 = null;",
             "  Object @Nullable[] foo2 = null;",
@@ -143,11 +145,14 @@ public void typeUseAndDeclarationAnnotationOnArray() {
 
   @Test
   public void typeUseAndDeclarationLegacyAnnotationOnArray() {
-    makeHelper()
+    makeLegacyModeHelper()
         .addSourceLines(
-            "Test.java",
+            "Nullable.java",
             "package com.uber;",
-            "import org.jetbrains.annotations.Nullable;",
+            "import java.lang.annotation.ElementType;",
+            "import java.lang.annotation.Target;",
+            "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})",
+            "public @interface Nullable {}",
             "class Test {",
             "  @Nullable Object[] foo1 = null;",
             "  Object @Nullable[] foo2 = null;",
@@ -159,7 +164,7 @@ public void typeUseAndDeclarationLegacyAnnotationOnArray() {
         .doTest();
   }
 
-  private CompilationTestHelper makeHelper() {
+  private CompilationTestHelper makeLegacyModeHelper() {
     return makeTestHelperWithArgs(
         Arrays.asList(
             "-XepOpt:NullAway:AnnotatedPackages=com.uber",

diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java
@@ -609,15 +609,26 @@ public void mismatchedIndexUse() {
   }
 
   @Test
-  public void typeUseAndDeclarationLegacyAnnotationOnArray() {
+  public void typeUseAndDeclarationAnnotationOnArray() {
     makeHelper()
         .addSourceLines(
-            "Test.java",
+            "Nullable.java",
             "package com.uber;",
-            "import org.jetbrains.annotations.Nullable;",
+            "import java.lang.annotation.ElementType;",
+            "import java.lang.annotation.Target;",
+            "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})",
+            "public @interface Nullable {}",
             "class Test {",
             "  @Nullable Object[] foo1 = null;",
-            "  Object @Nullable[] foo2 = null;",
+            "  static String @Nullable[] foo2 = null;",
+            "  static void bar() {",
+            "      if (foo2 !=null){",
+            "           // annotation is treated as declaration",
+            "           String bar = foo2[0].toString(); ",
+            "      }",
+            "      // BUG: Diagnostic contains: dereferenced expression foo2 is @Nullable",
+            "      String bar = foo2[0].toString(); ",
+            "  }",
             "  @Nullable Object @Nullable [] foo3 = null;",
             "  @Nullable Object [][] foo4 = null;",
             "  Object @Nullable [][] foo5 = null;",