Skip to content

Commit

Permalink
JSpecify: Handle @nullable elements for enhanced-for-loops on arrays (#…
Browse files Browse the repository at this point in the history
…986)

Currently, enhanced-for-loops do not honor nullability annotations on
arrays.

**Current Behavior**
Both these dereferences work fine.
```
static @nullable String[] fizz = {"1"}
for (String s: fizz){
     s.toString();
     if (s!=null){
          s.toString();
     }  
}

```
**New Behavior**
The first one throws an error, as expected.
```
static @nullable String[] fizz = {"1"}
for (String s: fizz){
     // BUG: Diagnostic contains: dereferenced expression s is @nullable
     s.toString();
     if (s!=null){
          s.toString();
     }  
}

```



Fixes #983
  • Loading branch information
armughan11 committed Jun 22, 2024
1 parent cc3dee3 commit 3ae9b23
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
Expand Down Expand Up @@ -791,8 +792,15 @@ public TransferResult<Nullness, NullnessStore> visitArrayAccess(
Nullness resultNullness;
// Unsoundly assume @NonNull, except in JSpecify mode where we check the type
if (config.isJSpecifyMode()) {
Symbol arraySymbol = ASTHelpers.getSymbol(node.getArray().getTree());
Symbol arraySymbol;
boolean isElementNullable = false;
// For enhanced-for-loops we get the symbol from the array expression as the node is desugared
ExpressionTree arrayExpr = node.getArrayExpression();
if (arrayExpr != null) {
arraySymbol = ASTHelpers.getSymbol(arrayExpr);
} else {
arraySymbol = ASTHelpers.getSymbol(node.getArray().getTree());
}
if (arraySymbol != null) {
isElementNullable = NullabilityUtil.isArrayElementNullable(arraySymbol, config);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.google.errorprone.CompilationTestHelper;
import com.uber.nullaway.NullAwayTestsBase;
import java.util.Arrays;
import org.junit.Ignore;
import org.junit.Test;

public class ArrayTests extends NullAwayTestsBase {
Expand Down Expand Up @@ -492,7 +491,6 @@ public void loopVariableIndex() {
}

@Test
@Ignore("for-each handling needs to be fixed; see https://github.com/uber/NullAway/issues/983")
public void forEachLoop() {
makeHelper()
.addSourceLines(
Expand All @@ -506,8 +504,6 @@ public void forEachLoop() {
" if (s != null) {",
" s.toString();",
" }",
" }",
" for (String s : fizz) {",
" // BUG: Diagnostic contains: dereferenced expression s is @Nullable",
" s.toString();",
" }",
Expand Down

0 comments on commit 3ae9b23

Please sign in to comment.