From 0ca53f9246ffeb8b5a8e0d9cab7d90070bf6f8b8 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sat, 9 Mar 2024 23:12:51 -0600 Subject: [PATCH 1/5] Handle @Nullable assignments to @Nonnull arrays in JSpecify mode --- .../main/java/com/uber/nullaway/NullAway.java | 15 ++++++++ .../nullaway/NullAwayJSpecifyArrayTests.java | 34 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 12e7370d92..9292b03936 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -32,6 +32,7 @@ import static com.uber.nullaway.ASTHelpersBackports.isStatic; import static com.uber.nullaway.ErrorBuilder.errMsgForInitializer; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static com.uber.nullaway.NullabilityUtil.isArrayElementNullable; import com.google.auto.service.AutoService; import com.google.auto.value.AutoValue; @@ -497,6 +498,20 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } + if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { + ArrayAccessTree arrayAccess = (ArrayAccessTree) tree.getVariable(); + ExpressionTree arrayExpr = arrayAccess.getExpression(); + ExpressionTree expression = tree.getExpression(); + Symbol arraySymbol = ASTHelpers.getSymbol(arrayExpr); + if (arraySymbol != null) { + boolean isElementNullable = isArrayElementNullable(arraySymbol, config); + if (!isElementNullable && mayBeNullExpr(state, expression)) { + String message = "assigning @Nullable expression to @NonNull field."; + return buildDescription(tree).setMessage(message).build(); + } + } + } + Symbol assigned = ASTHelpers.getSymbol(tree.getVariable()); if (assigned == null || assigned.getKind() != ElementKind.FIELD) { // not a field of nullable type diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java index 8141be9877..341f06d75b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java @@ -135,6 +135,40 @@ public void arrayContentsAndTopLevelAnnotation() { .doTest(); } + @Test + public void nullableAssignmentNonnullArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static String [] foo = new String[10];", + " static void foo() {", + " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", + " foo[1] = null;", + " }", + "}") + .doTest(); + } + + @Test + public void nullableAssignmentNullableArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String [] foo = new String[10];", + " static void foo() {", + " // OK: since array elements are @Nullable", + " foo[1] = null;", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From f540c85e6dc57c838c7ba5b08bb8a0561204fcf9 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sun, 10 Mar 2024 18:27:26 -0500 Subject: [PATCH 2/5] comment Co-authored-by: Manu Sridharan --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 1 + 1 file changed, 1 insertion(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 9292b03936..2461959f63 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -499,6 +499,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { + // check for a write of a @Nullable value into @NonNull array contents ArrayAccessTree arrayAccess = (ArrayAccessTree) tree.getVariable(); ExpressionTree arrayExpr = arrayAccess.getExpression(); ExpressionTree expression = tree.getExpression(); From ef0fe1a2b713269d585fe85da2d31ba651b42d76 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sun, 10 Mar 2024 18:56:21 -0500 Subject: [PATCH 3/5] Updated error handling and added tests for local and parameter arrays --- .../java/com/uber/nullaway/ErrorMessage.java | 1 + .../main/java/com/uber/nullaway/NullAway.java | 10 ++++- .../nullaway/NullAwayJSpecifyArrayTests.java | 45 ++++++++++++++++++- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index 186b386cde..017472fd9f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -58,6 +58,7 @@ public enum MessageTypes { PASS_NULLABLE_GENERIC, WRONG_OVERRIDE_RETURN_GENERIC, WRONG_OVERRIDE_PARAM_GENERIC, + ASSIGN_ARRAY_NULLABLE, } public String getMessage() { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 2461959f63..edd2ea376d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -507,8 +507,14 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { if (arraySymbol != null) { boolean isElementNullable = isArrayElementNullable(arraySymbol, config); if (!isElementNullable && mayBeNullExpr(state, expression)) { - String message = "assigning @Nullable expression to @NonNull field."; - return buildDescription(tree).setMessage(message).build(); + final String message = "Writing @Nullable expression into array with @NonNull contents."; + ErrorMessage errorMessage = new ErrorMessage(MessageTypes.ASSIGN_ARRAY_NULLABLE, message); + return errorBuilder.createErrorDescription( + errorMessage, + expression, + buildDescription(expression), + state, + ASTHelpers.getSymbol(tree.getVariable())); } } } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java index 341f06d75b..e16420cce4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java @@ -145,7 +145,7 @@ public void nullableAssignmentNonnullArray() { "class Test {", " static String [] foo = new String[10];", " static void foo() {", - " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents", " foo[1] = null;", " }", "}") @@ -169,6 +169,49 @@ public void nullableAssignmentNullableArray() { .doTest(); } + @Test + public void nullableAssignmentLocalArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static void foo() {", + " String [] nonNullArray = new String[10];", + " @Nullable String [] nullableArray = new String[10];", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents", + " nonNullArray[1] = null;", + " // OK: since array elements are @Nullable", + " nullableArray[1] = null;", + " }", + "}") + .doTest(); + } + + @Test + public void nullableAssignmentParameterArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static void fizz(String[] nonNullArray, @Nullable String[] nullableArray) {", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents", + " nonNullArray[1] = null;", + " // OK: since array elements are @Nullable", + " nullableArray[1] = null;", + " }", + " public static void main(String[] args) {", + " String[] foo = new String[10];", + " @Nullable String[] bar = new String[10];", + " fizz(foo, bar);", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From f0c5a99918c7128ef4f3c0aee6952f2fadcbce36 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sun, 10 Mar 2024 22:21:39 -0500 Subject: [PATCH 4/5] Added comments and modified error builder logic --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index edd2ea376d..c73e73196b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -509,10 +509,12 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { if (!isElementNullable && mayBeNullExpr(state, expression)) { final String message = "Writing @Nullable expression into array with @NonNull contents."; ErrorMessage errorMessage = new ErrorMessage(MessageTypes.ASSIGN_ARRAY_NULLABLE, message); + // Future enhancements which auto-fix such warnings will require modification to this + // logic return errorBuilder.createErrorDescription( errorMessage, expression, - buildDescription(expression), + buildDescription(tree), state, ASTHelpers.getSymbol(tree.getVariable())); } From bdb8f100925a3579b86dae0d6bbde7ab15537f34 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 11 Mar 2024 00:01:05 -0500 Subject: [PATCH 5/5] Fixed error message name --- nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java | 2 +- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index 017472fd9f..aa3c364306 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -58,7 +58,7 @@ public enum MessageTypes { PASS_NULLABLE_GENERIC, WRONG_OVERRIDE_RETURN_GENERIC, WRONG_OVERRIDE_PARAM_GENERIC, - ASSIGN_ARRAY_NULLABLE, + ASSIGN_NULLABLE_TO_NONNULL_ARRAY, } public String getMessage() { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index c73e73196b..ee1944ad04 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -508,7 +508,8 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { boolean isElementNullable = isArrayElementNullable(arraySymbol, config); if (!isElementNullable && mayBeNullExpr(state, expression)) { final String message = "Writing @Nullable expression into array with @NonNull contents."; - ErrorMessage errorMessage = new ErrorMessage(MessageTypes.ASSIGN_ARRAY_NULLABLE, message); + ErrorMessage errorMessage = + new ErrorMessage(MessageTypes.ASSIGN_NULLABLE_TO_NONNULL_ARRAY, message); // Future enhancements which auto-fix such warnings will require modification to this // logic return errorBuilder.createErrorDescription(