-
Notifications
You must be signed in to change notification settings - Fork 299
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
JSpecify: Modify Array Type Use Annotation Syntax (#850)
Added unit tests for annotations on array in JSpecify mode and modified behavior. Currently, NullAway does not support `@Nullable` annotations on array elements (#708). Both `@Nullable String[]` and `String @nullable []` are currently treated identically, and NullAway considers both annotations as the array itself might be null. This is the first step to bring the annotation syntax in line with JSpecify norms, which treats `@Nullable String[]` as the array elements could be null while `String @nullable []` implies the array itself could be null. After this change, NullAway completely ignores type-use annotations on array element types, such as `@Nullable String[]`, and only considers type-use annotations in the correct position for the top-level type, i.e., `String @nullable []`. This is an intermediary change, and support for type annotations will be extended eventually. Additionally, this **only** applies to **JSpecify mode** Unit tests currently conform to the current behavior of NullAway after the changes and some tests have TODOs until we support annotations on type and are able to detect the nullability of array elements. --------- Co-authored-by: Manu Sridharan <[email protected]> Co-authored-by: Lázaro Clapp <[email protected]>
- Loading branch information
3 people
authored
Oct 25, 2023
1 parent
97e92b9
commit def015a
Showing
4 changed files
with
153 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package com.uber.nullaway; | ||
|
|
||
| import com.google.errorprone.CompilationTestHelper; | ||
| import java.util.Arrays; | ||
| import org.junit.Test; | ||
|
|
||
| public class NullAwayJSpecifyArrayTests extends NullAwayTestsBase { | ||
|
|
||
| @Test | ||
| public void arrayTopLevelAnnotationDereference() { | ||
| makeHelper() | ||
| .addSourceLines( | ||
| "Test.java", | ||
| "package com.uber;", | ||
| "import org.jspecify.annotations.Nullable;", | ||
| "class Test {", | ||
| " static Integer @Nullable [] fizz = {1};", | ||
| " static void foo() {", | ||
| " // BUG: Diagnostic contains: dereferenced expression fizz is @Nullable", | ||
| " int bar = fizz.length;", | ||
| " }", | ||
| " static void bar() {", | ||
| " // BUG: Diagnostic contains: dereferenced expression fizz is @Nullable", | ||
| " int bar = fizz[0];", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void arrayTopLevelAnnotationAssignment() { | ||
| makeHelper() | ||
| .addSourceLines( | ||
| "Test.java", | ||
| "package com.uber;", | ||
| "import org.jspecify.annotations.Nullable;", | ||
| "class Test {", | ||
| " Object foo = new Object();", | ||
| " void m( Integer @Nullable [] bar) {", | ||
| " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", | ||
| " foo = bar;", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void arrayContentsAnnotationDereference() { | ||
| makeHelper() | ||
| .addSourceLines( | ||
| "Test.java", | ||
| "package com.uber;", | ||
| "import org.jspecify.annotations.Nullable;", | ||
| "class Test {", | ||
| " static @Nullable String [] fizz = {\"1\"};", | ||
| " static Object foo = new Object();", | ||
| " static void foo() {", | ||
| " // TODO: This should report an error due to dereference of @Nullable fizz[0]", | ||
| " int bar = fizz[0].length();", | ||
| " // OK: valid dereference since only elements of the array can be null", | ||
| " foo = fizz.length;", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void arrayContentsAnnotationAssignment() { | ||
| makeHelper() | ||
| .addSourceLines( | ||
| "Test.java", | ||
| "package com.uber;", | ||
| "import org.jspecify.annotations.Nullable;", | ||
| "class Test {", | ||
| " Object fizz = new Object();", | ||
| " void m( @Nullable Integer [] foo) {", | ||
| " // TODO: This should report an error due to assignment of @Nullable foo[0] to @NonNull field", | ||
| " fizz = foo[0];", | ||
| " // OK: valid assignment since only elements can be null", | ||
| " fizz = foo;", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| /** | ||
| * Currently in JSpecify mode, JSpecify syntax only applies to type-use annotations. Declaration | ||
| * annotations preserve their existing behavior, with annotations being treated on the top-level | ||
| * type. We will very likely revisit this design in the future. | ||
| */ | ||
| @Test | ||
| public void arrayDeclarationAnnotation() { | ||
| makeHelper() | ||
| .addSourceLines( | ||
| "Test.java", | ||
| "package com.uber;", | ||
| "import javax.annotation.Nullable;", | ||
| "class Test {", | ||
| " static @Nullable String [] fizz = {\"1\"};", | ||
| " static Object o1 = new Object();", | ||
| " static void foo() {", | ||
| " // This should not report an error while using JSpecify type-use annotation", | ||
| " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", | ||
| " o1 = fizz;", | ||
| " // This should not report an error while using JSpecify type-use annotation", | ||
| " // BUG: Diagnostic contains: dereferenced expression fizz is @Nullable", | ||
| " o1 = fizz.length;", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| private CompilationTestHelper makeHelper() { | ||
| return makeTestHelperWithArgs( | ||
| Arrays.asList( | ||
| "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true")); | ||
| } | ||
| } |