From e3a3c7672d51ec2e90c0e60c30af477ccf6f7757 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 11 Mar 2024 12:42:37 -0500 Subject: [PATCH] JSpecify: Handle @Nullable assignments to @Nonnull arrays (#929) Handles cases in JSpecify mode where a `@Nullable` element is assigned to an unannotated array. Added relevant unit tests. **Current Behavior** Both are valid assignments. ``` String [] foo = new String[10]; @Nullable String [] bar = new String [10]; foo[1] = null; bar[1] = null; ``` **New Behavior** Assignment to `foo` generates an error since array elements are `@Nonnull`. ``` String [] foo = new String[10]; @Nullable String [] bar = new String [10]; foo[1] = null; bar[1]=null; ``` --------- Co-authored-by: Manu Sridharan --- .../java/com/uber/nullaway/ErrorMessage.java | 1 + .../main/java/com/uber/nullaway/NullAway.java | 25 ++++++ .../nullaway/NullAwayJSpecifyArrayTests.java | 77 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index 186b386cde..aa3c364306 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_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 12e7370d92..ee1944ad04 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,30 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, 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(); + Symbol arraySymbol = ASTHelpers.getSymbol(arrayExpr); + if (arraySymbol != null) { + 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_NULLABLE_TO_NONNULL_ARRAY, message); + // Future enhancements which auto-fix such warnings will require modification to this + // logic + return errorBuilder.createErrorDescription( + errorMessage, + expression, + buildDescription(tree), + state, + ASTHelpers.getSymbol(tree.getVariable())); + } + } + } + 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..e16420cce4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java @@ -135,6 +135,83 @@ 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: Writing @Nullable expression into array with @NonNull contents", + " 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(); + } + + @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(