From 8646811ef74a361c8ad888a4b729049f58aeaea0 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sun, 2 Jun 2024 22:22:29 -0500 Subject: [PATCH 01/10] Initial commit --- .../uber/nullaway/dataflow/AccessPath.java | 53 ++++++++++++++++-- .../nullaway/dataflow/AccessPathElement.java | 56 ++----------------- .../AccessPathNullnessPropagation.java | 11 +--- .../nullaway/dataflow/ArrayIndexElement.java | 42 ++++++++++++++ .../dataflow/FieldOrMethodCallElement.java | 55 ++++++++++++++++++ .../uber/nullaway/jspecify/ArrayTests.java | 41 ++++++++++++++ 6 files changed, 193 insertions(+), 65 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java create mode 100644 nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 784d0d0806..edaa59669f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -46,6 +46,7 @@ import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; import javax.lang.model.element.VariableElement; +import org.checkerframework.nullaway.dataflow.cfg.node.ArrayAccessNode; import org.checkerframework.nullaway.dataflow.cfg.node.ClassNameNode; import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode; import org.checkerframework.nullaway.dataflow.cfg.node.IntegerLiteralNode; @@ -206,7 +207,7 @@ static AccessPath switchRoot(AccessPath origAP, Element newRoot) { @Nullable public static AccessPath fromBaseAndElement( Node base, Element element, AccessPathContext apContext) { - return fromNodeElementAndContext(base, new AccessPathElement(element), apContext); + return fromNodeElementAndContext(base, new FieldOrMethodCallElement(element), apContext); } @Nullable @@ -239,7 +240,7 @@ private static AccessPath fromNodeElementAndContext( public static AccessPath fromBaseMethodAndConstantArgs( Node base, Element method, List constantArguments, AccessPathContext apContext) { return fromNodeElementAndContext( - base, new AccessPathElement(method, constantArguments), apContext); + base, new FieldOrMethodCallElement(method, constantArguments), apContext); } /** @@ -334,6 +335,28 @@ public static AccessPath getAccessPathForNode( return fromFieldAccess((FieldAccessNode) node, apContext); } else if (node instanceof MethodInvocationNode) { return fromMethodCall((MethodInvocationNode) node, state, apContext); + } else if (node instanceof ArrayAccessNode) { + return fromArrayAccess((ArrayAccessNode) node, apContext); + } else { + return null; + } + } + + @Nullable + private static AccessPath fromArrayAccess(ArrayAccessNode node, AccessPathContext apContext) { + return fromNodeAndContext(node, apContext); + } + + @Nullable + private static Element getElementFromArrayNode(Node arrayNode) { + if (arrayNode instanceof LocalVariableNode) { + return ((LocalVariableNode) arrayNode).getElement(); + } else if (arrayNode instanceof FieldAccessNode) { + return ((FieldAccessNode) arrayNode).getElement(); + } else if (arrayNode instanceof MethodInvocationNode) { + return ASTHelpers.getSymbol(((MethodInvocationNode) arrayNode).getTree()); + } else if (arrayNode instanceof VariableDeclarationNode) { + return TreeUtils.elementFromDeclaration(((VariableDeclarationNode) arrayNode).getTree()); } else { return null; } @@ -350,7 +373,7 @@ public static AccessPath fromFieldElement(VariableElement element) { Preconditions.checkArgument( element.getKind().isField(), "element must be of type: FIELD but received: " + element.getKind()); - return new AccessPath(null, ImmutableList.of(new AccessPathElement(element))); + return new AccessPath(null, ImmutableList.of(new FieldOrMethodCallElement(element))); } private static boolean isBoxingMethod(Symbol.MethodSymbol methodSymbol) { @@ -384,11 +407,28 @@ private static AccessPath buildAccessPathRecursive( result = new AccessPath(fieldAccess.getElement(), ImmutableList.copyOf(elements), mapKey); } else { // instance field access - elements.push(new AccessPathElement(fieldAccess.getElement())); + elements.push(new FieldOrMethodCallElement(fieldAccess.getElement())); result = buildAccessPathRecursive( stripCasts(fieldAccess.getReceiver()), elements, apContext, mapKey); } + } else if (node instanceof ArrayAccessNode) { + ArrayAccessNode arrayAccess = (ArrayAccessNode) node; + Node arrayNode = arrayAccess.getArray(); + Node indexNode = arrayAccess.getIndex(); + Element arrayElement = getElementFromArrayNode(arrayNode); + if (arrayElement == null) { + return null; + } + String indexValue = null; + if (indexNode instanceof IntegerLiteralNode) { + IntegerLiteralNode intIndexNode = (IntegerLiteralNode) indexNode; + indexValue = String.valueOf(intIndexNode.getValue()); + } else if (indexNode instanceof LocalVariableNode) { + indexValue = indexNode.toString(); + } + elements.push(new ArrayIndexElement(arrayElement, indexValue)); + result = buildAccessPathRecursive(stripCasts(arrayNode), elements, apContext, mapKey); } else if (node instanceof MethodInvocationNode) { MethodInvocationNode invocation = (MethodInvocationNode) node; AccessPathElement accessPathElement; @@ -399,7 +439,7 @@ private static AccessPath buildAccessPathRecursive( // a zero-argument static method call can be the root of an access path return new AccessPath(symbol, ImmutableList.copyOf(elements), mapKey); } else { - accessPathElement = new AccessPathElement(accessNode.getMethod()); + accessPathElement = new FieldOrMethodCallElement(accessNode.getMethod()); } } else { List constantArgumentValues = new ArrayList<>(); @@ -468,7 +508,8 @@ && isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) { return null; // Not an AP } } - accessPathElement = new AccessPathElement(accessNode.getMethod(), constantArgumentValues); + accessPathElement = + new FieldOrMethodCallElement(accessNode.getMethod(), constantArgumentValues); } elements.push(accessPathElement); result = diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java index 1111d79e57..5af51b65ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java @@ -1,62 +1,16 @@ package com.uber.nullaway.dataflow; -import com.google.common.collect.ImmutableList; -import java.util.Arrays; -import java.util.List; -import java.util.Objects; -import javax.annotation.Nullable; import javax.lang.model.element.Element; -/** - * Represents a (non-root) element of an AccessPath. - * - *

This is just a java Element (field, method, etc) in the access-path chain (e.g. f or g() in - * x.f.g()). Plus, optionally, a list of constant arguments, allowing access path elements for - * method calls with constant values (e.g. h(3) or k("STR_KEY") in x.h(3).g().k("STR_KEY")). - */ -public final class AccessPathElement { - private final Element javaElement; - @Nullable private final ImmutableList constantArguments; - - public AccessPathElement(Element javaElement, List constantArguments) { - this.javaElement = javaElement; - this.constantArguments = ImmutableList.copyOf(constantArguments); - } - - public AccessPathElement(Element javaElement) { - this.javaElement = javaElement; - this.constantArguments = null; - } - - public Element getJavaElement() { - return this.javaElement; - } +public interface AccessPathElement { + Element getJavaElement(); @Override - public boolean equals(Object obj) { - if (obj instanceof AccessPathElement) { - AccessPathElement otherNode = (AccessPathElement) obj; - return this.javaElement.equals(otherNode.javaElement) - && Objects.equals(constantArguments, otherNode.constantArguments); - } else { - return false; - } - } + String toString(); @Override - public int hashCode() { - int result = javaElement.hashCode(); - result = 31 * result + (constantArguments != null ? constantArguments.hashCode() : 0); - return result; - } + boolean equals(Object obj); @Override - public String toString() { - return "APElement{" - + "javaElement=" - + javaElement.toString() - + ", constantArguments=" - + Arrays.deepToString(constantArguments != null ? constantArguments.toArray() : null) - + '}'; - } + int hashCode(); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index a1efbe5b6a..d08e2d23fa 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -788,17 +788,12 @@ public TransferResult visitArrayAccess( ArrayAccessNode node, TransferInput input) { ReadableUpdates updates = new ReadableUpdates(); setNonnullIfAnalyzeable(updates, node.getArray()); - Nullness resultNullness; + Nullness resultNullness = defaultAssumption; // Unsoundly assume @NonNull, except in JSpecify mode where we check the type - boolean isElementNullable = false; if (config.isJSpecifyMode()) { - Symbol arraySymbol = ASTHelpers.getSymbol(node.getArray().getTree()); - if (arraySymbol != null) { - isElementNullable = NullabilityUtil.isArrayElementNullable(arraySymbol, config); - } + AccessPath arrayAccessPath = AccessPath.getAccessPathForNode(node, state, apContext); + resultNullness = input.getRegularStore().getNullnessOfAccessPath(arrayAccessPath); } - - resultNullness = isElementNullable ? Nullness.NULLABLE : defaultAssumption; return updateRegularStore(resultNullness, input, updates); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java new file mode 100644 index 0000000000..b7b2fb128a --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java @@ -0,0 +1,42 @@ +package com.uber.nullaway.dataflow; + +import java.util.Objects; +import javax.annotation.Nullable; +import javax.lang.model.element.Element; + +public class ArrayIndexElement implements AccessPathElement { + private final Element javaElement; + @Nullable final String index; + + public ArrayIndexElement(Element javaElement, @Nullable String index) { + this.javaElement = javaElement; + this.index = index; + } + + @Override + public Element getJavaElement() { + return this.javaElement; + } + + @Override + public String toString() { + return "ArrayIndexElement{" + "javaElement=" + javaElement + ", index=" + index + '}'; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof ArrayIndexElement) { + ArrayIndexElement otherNode = (ArrayIndexElement) obj; + return this.javaElement.equals(otherNode.javaElement) + && Objects.equals(index, otherNode.index); + } + return false; + } + + @Override + public int hashCode() { + int result = javaElement.hashCode(); + result = 31 * result + (index != null ? index.hashCode() : 0); + return result; + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java new file mode 100644 index 0000000000..f2acb317e0 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java @@ -0,0 +1,55 @@ +package com.uber.nullaway.dataflow; + +import com.google.common.collect.ImmutableList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import javax.annotation.Nullable; +import javax.lang.model.element.Element; + +public class FieldOrMethodCallElement implements AccessPathElement { + private final Element javaElement; + @Nullable private final ImmutableList constantArguments; + + public FieldOrMethodCallElement(Element javaElement, List constantArguments) { + this.javaElement = javaElement; + this.constantArguments = ImmutableList.copyOf(constantArguments); + } + + public FieldOrMethodCallElement(Element javaElement) { + this.javaElement = javaElement; + this.constantArguments = null; + } + + @Override + public Element getJavaElement() { + return this.javaElement; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof FieldOrMethodCallElement) { + FieldOrMethodCallElement other = (FieldOrMethodCallElement) obj; + return this.javaElement.equals(other.javaElement) + && Objects.equals(this.constantArguments, other.constantArguments); + } + return false; + } + + @Override + public int hashCode() { + int result = javaElement.hashCode(); + result = 31 * result + (constantArguments != null ? constantArguments.hashCode() : 0); + return result; + } + + @Override + public String toString() { + return "FieldOrMethodCallElement{" + + "javaElement=" + + javaElement + + ", constantArguments=" + + (constantArguments != null ? Arrays.toString(constantArguments.toArray()) : "null") + + '}'; + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index 108036fac3..7c7e9f4be9 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -356,6 +356,47 @@ public void ternaryOperator() { .doTest(); } + @Test + public void variableIndexArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String [] fizz = {\"1\"};", + " static final Integer i = 0;", + " static void foo() {", + " if (fizz[i]!=null) { ", + " fizz[i].toString();", + "}", + " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", + " fizz[i].toString();", + " }", + "}") + .doTest(); + } + + @Test + public void ConstantIndexArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String [] fizz = {\"1\"};", + " static void foo() {", + " if (fizz[0]!=null) { ", + " fizz[0].toString();", + "}", + " // BUG: Diagnostic contains: dereferenced expression fizz[0] is @Nullable", + " fizz[0].toString();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 351005b07bc6a50379c874f007735ceb922356a3 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 3 Jun 2024 01:29:42 -0500 Subject: [PATCH 02/10] Fix instances --- .../src/main/java/com/uber/nullaway/dataflow/AccessPath.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index edaa59669f..60a097e7ea 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -355,8 +355,6 @@ private static Element getElementFromArrayNode(Node arrayNode) { return ((FieldAccessNode) arrayNode).getElement(); } else if (arrayNode instanceof MethodInvocationNode) { return ASTHelpers.getSymbol(((MethodInvocationNode) arrayNode).getTree()); - } else if (arrayNode instanceof VariableDeclarationNode) { - return TreeUtils.elementFromDeclaration(((VariableDeclarationNode) arrayNode).getTree()); } else { return null; } From 0eb952e780924f3d3e3a5958010186108a58466f Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 3 Jun 2024 01:34:30 -0500 Subject: [PATCH 03/10] few more tests --- .../uber/nullaway/jspecify/ArrayTests.java | 52 +++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index 7c7e9f4be9..aab6631519 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -357,7 +357,7 @@ public void ternaryOperator() { } @Test - public void variableIndexArray() { + public void fieldAccessIndexArray() { makeHelper() .addSourceLines( "Test.java", @@ -365,10 +365,10 @@ public void variableIndexArray() { "import org.jspecify.annotations.Nullable;", "class Test {", " static @Nullable String [] fizz = {\"1\"};", - " static final Integer i = 0;", + " static final Integer index = 0;", " static void foo() {", - " if (fizz[i]!=null) { ", - " fizz[i].toString();", + " if (fizz[index]!=null) { ", + " fizz[index].toString();", "}", " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", " fizz[i].toString();", @@ -397,6 +397,50 @@ public void ConstantIndexArray() { .doTest(); } + @Test + public void localVariableIndexArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String[] fizz = {\"1\"};", + " static void foo() {", + " int index = 1;", + " if (fizz[index] != null) {", + " fizz[index].toString();", + " }", + " // BUG: Diagnostic contains: dereferenced expression fizz[index] is @Nullable", + " fizz[index].toString();", + " }", + "}") + .doTest(); + } + + @Test + public void methodInvocationIndexArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String[] fizz = {\"1\"};", + " static int getIndex() {", + " return 0;", + " }", + " static void foo() {", + " if (fizz[getIndex()] != null) {", + " fizz[getIndex()].toString();", + " }", + " // BUG: Diagnostic contains: dereferenced expression fizz[getIndex()] is @Nullable", + " fizz[getIndex()].toString();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From cad529c9d826d44c89091a84829e68a6c2074971 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 3 Jun 2024 01:35:51 -0500 Subject: [PATCH 04/10] case fix --- .../src/test/java/com/uber/nullaway/jspecify/ArrayTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index aab6631519..edf1451b79 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -378,7 +378,7 @@ public void fieldAccessIndexArray() { } @Test - public void ConstantIndexArray() { + public void constantIndexArray() { makeHelper() .addSourceLines( "Test.java", From da7c17b2f2a32572109f7becc0782bcac1a9852a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 3 Jun 2024 16:50:59 -0700 Subject: [PATCH 05/10] add failing test --- .../uber/nullaway/jspecify/ArrayTests.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index edf1451b79..18504b1b12 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -441,6 +441,25 @@ public void methodInvocationIndexArray() { .doTest(); } + @Test + public void forEachLoopOverArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.List;", + "class Test {", + " static String[] arr = {\"1\"};", + " static void foo() {", + " for (String s : arr) {", + " s.toString();", + " }", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 07783b855a902ab2b9915c93ddb7947f2e83c957 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 3 Jun 2024 20:56:45 -0500 Subject: [PATCH 06/10] Added direct checks --- .../uber/nullaway/dataflow/AccessPath.java | 13 +++--- .../AccessPathNullnessPropagation.java | 22 ++++++++- .../nullaway/dataflow/ArrayIndexElement.java | 45 +++++++++++++++---- .../uber/nullaway/jspecify/ArrayTests.java | 25 +++++++++-- 4 files changed, 87 insertions(+), 18 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 60a097e7ea..cac80a599c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -418,14 +418,17 @@ private static AccessPath buildAccessPathRecursive( if (arrayElement == null) { return null; } - String indexValue = null; if (indexNode instanceof IntegerLiteralNode) { IntegerLiteralNode intIndexNode = (IntegerLiteralNode) indexNode; - indexValue = String.valueOf(intIndexNode.getValue()); - } else if (indexNode instanceof LocalVariableNode) { - indexValue = indexNode.toString(); + elements.push(new ArrayIndexElement(arrayElement, intIndexNode.getValue())); + } else { + Element indexElement = getElementFromArrayNode(indexNode); + if (indexElement != null) { + elements.push(new ArrayIndexElement(arrayElement, indexElement)); + } else { + return null; + } } - elements.push(new ArrayIndexElement(arrayElement, indexValue)); result = buildAccessPathRecursive(stripCasts(arrayNode), elements, apContext, mapKey); } else if (node instanceof MethodInvocationNode) { MethodInvocationNode invocation = (MethodInvocationNode) node; diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index d08e2d23fa..a557c9e00e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -791,8 +791,26 @@ public TransferResult visitArrayAccess( Nullness resultNullness = defaultAssumption; // Unsoundly assume @NonNull, except in JSpecify mode where we check the type if (config.isJSpecifyMode()) { - AccessPath arrayAccessPath = AccessPath.getAccessPathForNode(node, state, apContext); - resultNullness = input.getRegularStore().getNullnessOfAccessPath(arrayAccessPath); + Symbol arraySymbol = ASTHelpers.getSymbol(node.getArray().getTree()); + boolean isElementNullable = false; + if (arraySymbol != null) { + isElementNullable = NullabilityUtil.isArrayElementNullable(arraySymbol, config); + } + + if (isElementNullable) { + AccessPath arrayAccessPath = AccessPath.getAccessPathForNode(node, state, apContext); + if (arrayAccessPath != null) { + @Nullable + Nullness accessPathNullness = + input.getRegularStore().getNullnessOfAccessPath(arrayAccessPath); + if (accessPathNullness == Nullness.NULLABLE) { + resultNullness = Nullness.NULLABLE; + } + } + + } else { + resultNullness = Nullness.NONNULL; + } } return updateRegularStore(resultNullness, input, updates); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java index b7b2fb128a..ae8c8ced9a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java @@ -6,11 +6,19 @@ public class ArrayIndexElement implements AccessPathElement { private final Element javaElement; - @Nullable final String index; + @Nullable private final Integer constantIndex; + @Nullable private final Element variableElement; - public ArrayIndexElement(Element javaElement, @Nullable String index) { + public ArrayIndexElement(Element javaElement, Integer constantIndex) { this.javaElement = javaElement; - this.index = index; + this.constantIndex = constantIndex; + this.variableElement = null; + } + + public ArrayIndexElement(Element javaElement, Element variableElement) { + this.javaElement = javaElement; + this.variableElement = variableElement; + this.constantIndex = null; } @Override @@ -18,17 +26,35 @@ public Element getJavaElement() { return this.javaElement; } + @Nullable + public Integer getConstantIndex() { + return this.constantIndex; + } + + @Nullable + public Element getVariableElement() { + return this.variableElement; + } + @Override public String toString() { - return "ArrayIndexElement{" + "javaElement=" + javaElement + ", index=" + index + '}'; + return "ArrayIndexElement{" + + "javaElement=" + + javaElement + + ", constantIndex=" + + constantIndex + + ", variableElement=" + + (variableElement != null ? variableElement.getSimpleName() : null) + + '}'; } @Override public boolean equals(Object obj) { if (obj instanceof ArrayIndexElement) { - ArrayIndexElement otherNode = (ArrayIndexElement) obj; - return this.javaElement.equals(otherNode.javaElement) - && Objects.equals(index, otherNode.index); + ArrayIndexElement other = (ArrayIndexElement) obj; + return Objects.equals(javaElement, other.javaElement) + && Objects.equals(constantIndex, other.constantIndex) + && Objects.equals(variableElement, other.variableElement); } return false; } @@ -36,7 +62,10 @@ public boolean equals(Object obj) { @Override public int hashCode() { int result = javaElement.hashCode(); - result = 31 * result + (index != null ? index.hashCode() : 0); + result = + 31 * result + + (constantIndex != null ? constantIndex.hashCode() : 0) + + (variableElement != null ? variableElement.hashCode() : 0); return result; } } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index edf1451b79..7c2e1174d7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -365,10 +365,10 @@ public void fieldAccessIndexArray() { "import org.jspecify.annotations.Nullable;", "class Test {", " static @Nullable String [] fizz = {\"1\"};", - " static final Integer index = 0;", + " static final Integer i = 0;", " static void foo() {", - " if (fizz[index]!=null) { ", - " fizz[index].toString();", + " if (fizz[i]!=null) { ", + " fizz[i].toString();", "}", " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", " fizz[i].toString();", @@ -441,6 +441,25 @@ public void methodInvocationIndexArray() { .doTest(); } + @Test + public void forEachLoopOverArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.List;", + "class Test {", + " static String[] arr = {\"1\"};", + " static void foo() {", + " for (String s : arr) {", + " s.toString();", + " }", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From ff785761ba07e6970a951ee206c26c8731d21fa4 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Mon, 3 Jun 2024 22:14:28 -0500 Subject: [PATCH 07/10] Added direct checks --- .../com/uber/nullaway/dataflow/ArrayIndexElement.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java index ae8c8ced9a..cf39b1937a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java @@ -26,16 +26,6 @@ public Element getJavaElement() { return this.javaElement; } - @Nullable - public Integer getConstantIndex() { - return this.constantIndex; - } - - @Nullable - public Element getVariableElement() { - return this.variableElement; - } - @Override public String toString() { return "ArrayIndexElement{" From c51136c0eb8731cfe631e432ce5d7e9b4593cb23 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sat, 8 Jun 2024 01:10:31 -0500 Subject: [PATCH 08/10] Addressed PR comments --- .../uber/nullaway/dataflow/AccessPath.java | 8 +-- .../nullaway/dataflow/AccessPathElement.java | 32 +++++++--- .../AccessPathNullnessPropagation.java | 5 +- .../nullaway/dataflow/ArrayIndexElement.java | 47 +++++++------- .../dataflow/FieldOrMethodCallElement.java | 7 +++ .../uber/nullaway/jspecify/ArrayTests.java | 62 +++++++++++++++++-- 6 files changed, 117 insertions(+), 44 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index cac80a599c..b83b54b98b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -353,8 +353,6 @@ private static Element getElementFromArrayNode(Node arrayNode) { return ((LocalVariableNode) arrayNode).getElement(); } else if (arrayNode instanceof FieldAccessNode) { return ((FieldAccessNode) arrayNode).getElement(); - } else if (arrayNode instanceof MethodInvocationNode) { - return ASTHelpers.getSymbol(((MethodInvocationNode) arrayNode).getTree()); } else { return null; } @@ -412,9 +410,10 @@ private static AccessPath buildAccessPathRecursive( } } else if (node instanceof ArrayAccessNode) { ArrayAccessNode arrayAccess = (ArrayAccessNode) node; - Node arrayNode = arrayAccess.getArray(); + Node arrayNode = stripCasts(arrayAccess.getArray()); Node indexNode = arrayAccess.getIndex(); Element arrayElement = getElementFromArrayNode(arrayNode); + Element indexElement = getElementFromArrayNode(indexNode); if (arrayElement == null) { return null; } @@ -422,14 +421,13 @@ private static AccessPath buildAccessPathRecursive( IntegerLiteralNode intIndexNode = (IntegerLiteralNode) indexNode; elements.push(new ArrayIndexElement(arrayElement, intIndexNode.getValue())); } else { - Element indexElement = getElementFromArrayNode(indexNode); if (indexElement != null) { elements.push(new ArrayIndexElement(arrayElement, indexElement)); } else { return null; } } - result = buildAccessPathRecursive(stripCasts(arrayNode), elements, apContext, mapKey); + result = buildAccessPathRecursive(arrayNode, elements, apContext, mapKey); } else if (node instanceof MethodInvocationNode) { MethodInvocationNode invocation = (MethodInvocationNode) node; AccessPathElement accessPathElement; diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java index 5af51b65ca..ef3fcf5d7b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathElement.java @@ -2,15 +2,29 @@ import javax.lang.model.element.Element; +/** + * Represents a generic element in an access path used for nullability analysis. + * + *

This interface abstracts over different kinds of path elements that can be part of an access + * path, including fields and methods, or array indices. Implementations of this interface should + * specify the type of the access path element: + * + *

    + *
  • {@code FieldOrMethodCallElement} - Represents access to a field or the invocation of a + * method, potentially with constant arguments. + *
  • {@code ArrayIndexElement} - Represents access to an array element either by a constant + * index or via an index that is calculated dynamically. + *
+ * + *

The {@code getJavaElement()} method returns the corresponding Java {@link Element} that the + * access path element refers to. + */ public interface AccessPathElement { + /** + * Returns the Java element associated with this access path element. + * + * @return the Java {@link Element} related to this path element, such as a field, method, or the + * array itself. + */ Element getJavaElement(); - - @Override - String toString(); - - @Override - boolean equals(Object obj); - - @Override - int hashCode(); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index a557c9e00e..c73cba56c5 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -796,7 +796,6 @@ public TransferResult visitArrayAccess( if (arraySymbol != null) { isElementNullable = NullabilityUtil.isArrayElementNullable(arraySymbol, config); } - if (isElementNullable) { AccessPath arrayAccessPath = AccessPath.getAccessPathForNode(node, state, apContext); if (arrayAccessPath != null) { @@ -806,11 +805,15 @@ public TransferResult visitArrayAccess( if (accessPathNullness == Nullness.NULLABLE) { resultNullness = Nullness.NULLABLE; } + } else { + resultNullness = Nullness.NULLABLE; } } else { resultNullness = Nullness.NONNULL; } + } else { + resultNullness = Nullness.NONNULL; } return updateRegularStore(resultNullness, input, updates); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java index cf39b1937a..3c3075318b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java @@ -1,24 +1,30 @@ package com.uber.nullaway.dataflow; import java.util.Objects; -import javax.annotation.Nullable; import javax.lang.model.element.Element; +/** + * Represents an array index element of an AccessPath, encapsulating access to array elements either + * via constant or variable indices. + * + *

This class holds an element that represents the array itself and an index that specifies the + * position within the array. The index can be a constant (Integer) if it's statically known, or an + * Element representing a variable index. + */ public class ArrayIndexElement implements AccessPathElement { private final Element javaElement; - @Nullable private final Integer constantIndex; - @Nullable private final Element variableElement; - - public ArrayIndexElement(Element javaElement, Integer constantIndex) { - this.javaElement = javaElement; - this.constantIndex = constantIndex; - this.variableElement = null; - } - - public ArrayIndexElement(Element javaElement, Element variableElement) { + private final Object index; + + /** + * Constructs an ArrayIndexElement. + * + * @param javaElement The element of the array. + * @param index The index used to access the array. Must be either an Integer (for constant + * indices) or an Element (for variable indices). + */ + public ArrayIndexElement(Element javaElement, Object index) { this.javaElement = javaElement; - this.variableElement = variableElement; - this.constantIndex = null; + this.index = index; } @Override @@ -31,10 +37,8 @@ public String toString() { return "ArrayIndexElement{" + "javaElement=" + javaElement - + ", constantIndex=" - + constantIndex - + ", variableElement=" - + (variableElement != null ? variableElement.getSimpleName() : null) + + ", index=" + + (index instanceof Element ? ((Element) index).getSimpleName() : index) + '}'; } @@ -42,9 +46,7 @@ public String toString() { public boolean equals(Object obj) { if (obj instanceof ArrayIndexElement) { ArrayIndexElement other = (ArrayIndexElement) obj; - return Objects.equals(javaElement, other.javaElement) - && Objects.equals(constantIndex, other.constantIndex) - && Objects.equals(variableElement, other.variableElement); + return Objects.equals(javaElement, other.javaElement) && Objects.equals(index, other.index); } return false; } @@ -52,10 +54,7 @@ public boolean equals(Object obj) { @Override public int hashCode() { int result = javaElement.hashCode(); - result = - 31 * result - + (constantIndex != null ? constantIndex.hashCode() : 0) - + (variableElement != null ? variableElement.hashCode() : 0); + result = 31 * result + (index != null ? index.hashCode() : 0); return result; } } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java index f2acb317e0..8b519a0fa5 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/FieldOrMethodCallElement.java @@ -7,6 +7,13 @@ import javax.annotation.Nullable; import javax.lang.model.element.Element; +/** + * Represents a (non-root) field or method call element of an AccessPath. + * + *

This is just a java Element (field or method call) in the access-path chain (e.g. f or g() in + * x.f.g()). Plus, optionally, a list of constant arguments, allowing access path elements for + * method calls with constant values (e.g. h(3) or k("STR_KEY") in x.h(3).g().k("STR_KEY")). + */ public class FieldOrMethodCallElement implements AccessPathElement { private final Element javaElement; @Nullable private final ImmutableList constantArguments; diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index 7c2e1174d7..db00ea1b81 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -368,6 +368,8 @@ public void fieldAccessIndexArray() { " static final Integer i = 0;", " static void foo() {", " if (fizz[i]!=null) { ", + " // field access indexes aren't handled", + " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", " fizz[i].toString();", "}", " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", @@ -388,6 +390,7 @@ public void constantIndexArray() { " static @Nullable String [] fizz = {\"1\"};", " static void foo() {", " if (fizz[0]!=null) { ", + " // OK: constant integer indexes are handled by dataflow", " fizz[0].toString();", "}", " // BUG: Diagnostic contains: dereferenced expression fizz[0] is @Nullable", @@ -409,6 +412,7 @@ public void localVariableIndexArray() { " static void foo() {", " int index = 1;", " if (fizz[index] != null) {", + " // OK: local variable indexes are handled by dataflow", " fizz[index].toString();", " }", " // BUG: Diagnostic contains: dereferenced expression fizz[index] is @Nullable", @@ -432,6 +436,8 @@ public void methodInvocationIndexArray() { " }", " static void foo() {", " if (fizz[getIndex()] != null) {", + " // index methods aren't handled by dataflow", + " // BUG: Diagnostic contains: dereferenced expression fizz[getIndex()] is @Nullable", " fizz[getIndex()].toString();", " }", " // BUG: Diagnostic contains: dereferenced expression fizz[getIndex()] is @Nullable", @@ -442,18 +448,64 @@ public void methodInvocationIndexArray() { } @Test - public void forEachLoopOverArray() { + public void arithmeticIndexArray() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String[] fizz = {\"1\", null};", + " static void foo() {", + " int i = 0;", + " if (fizz[i+1] != null) {", + " // index expressions aren't handled by dataflow", + " // BUG: Diagnostic contains: dereferenced expression fizz[i+1] is @Nullable", + " fizz[i+1].toString();", + " }", + " // BUG: Diagnostic contains: dereferenced expression fizz[i+1] is @Nullable", + " fizz[i+1].toString();", + " }", + "}") + .doTest(); + } + + @Test + public void arrayMethodInvocationIndex() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static @Nullable String[] getArray() { return new String[] {\"1\", null}; }", + " static void foo() {", + " if (getArray()[0] != null) {", + " // array resulting from method invocation isn't handled by dataflow", + " // BUG: Diagnostic contains: dereferenced expression getArray()[0] is @Nullable", + " getArray()[0].toString();", + " }", + " // BUG: Diagnostic contains: dereferenced expression getArray()[0] is @Nullable", + " getArray()[0].toString();", + " }", + "}") + .doTest(); + } + + @Test + public void mismatchedIndexUse() { makeHelper() .addSourceLines( "Test.java", "package com.uber;", "import org.jspecify.annotations.Nullable;", - "import java.util.List;", "class Test {", - " static String[] arr = {\"1\"};", + " static @Nullable String[] fizz = {\"1\", null};", " static void foo() {", - " for (String s : arr) {", - " s.toString();", + " int i = 0;", + " if (fizz[i] != null) {", + " // BUG: Diagnostic contains: dereferenced expression fizz[i+1] is @Nullable", + " fizz[i+1].toString();", " }", " }", "}") From 49d9fec8c673dff1dbb7c92f83c2c6c1218ce5fc Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sat, 8 Jun 2024 12:54:59 -0500 Subject: [PATCH 09/10] Addressed PR comments --- .../uber/nullaway/dataflow/AccessPath.java | 4 +-- .../AccessPathNullnessPropagation.java | 5 ++-- .../nullaway/dataflow/ArrayIndexElement.java | 29 ++++++++++++++----- .../uber/nullaway/jspecify/ArrayTests.java | 10 +++++-- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index b83b54b98b..436547b2f2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -419,10 +419,10 @@ private static AccessPath buildAccessPathRecursive( } if (indexNode instanceof IntegerLiteralNode) { IntegerLiteralNode intIndexNode = (IntegerLiteralNode) indexNode; - elements.push(new ArrayIndexElement(arrayElement, intIndexNode.getValue())); + elements.push(ArrayIndexElement.withIntegerIndex(arrayElement, intIndexNode.getValue())); } else { if (indexElement != null) { - elements.push(new ArrayIndexElement(arrayElement, indexElement)); + elements.push(ArrayIndexElement.withVariableIndex(arrayElement, indexElement)); } else { return null; } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index c73cba56c5..d6f9d3a644 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -788,7 +788,7 @@ public TransferResult visitArrayAccess( ArrayAccessNode node, TransferInput input) { ReadableUpdates updates = new ReadableUpdates(); setNonnullIfAnalyzeable(updates, node.getArray()); - Nullness resultNullness = defaultAssumption; + Nullness resultNullness; // Unsoundly assume @NonNull, except in JSpecify mode where we check the type if (config.isJSpecifyMode()) { Symbol arraySymbol = ASTHelpers.getSymbol(node.getArray().getTree()); @@ -799,11 +799,12 @@ public TransferResult visitArrayAccess( if (isElementNullable) { AccessPath arrayAccessPath = AccessPath.getAccessPathForNode(node, state, apContext); if (arrayAccessPath != null) { - @Nullable Nullness accessPathNullness = input.getRegularStore().getNullnessOfAccessPath(arrayAccessPath); if (accessPathNullness == Nullness.NULLABLE) { resultNullness = Nullness.NULLABLE; + } else { + resultNullness = Nullness.NONNULL; } } else { resultNullness = Nullness.NULLABLE; diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java index 3c3075318b..e3cc9fd3c7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/ArrayIndexElement.java @@ -15,16 +15,31 @@ public class ArrayIndexElement implements AccessPathElement { private final Element javaElement; private final Object index; + private ArrayIndexElement(Element javaElement, Object index) { + this.javaElement = javaElement; + this.index = index; + } + /** - * Constructs an ArrayIndexElement. + * Creates an ArrayIndexElement with an integer index. * - * @param javaElement The element of the array. - * @param index The index used to access the array. Must be either an Integer (for constant - * indices) or an Element (for variable indices). + * @param javaElement the element of the array + * @param index the integer index to access the array + * @return an instance of ArrayIndexElement */ - public ArrayIndexElement(Element javaElement, Object index) { - this.javaElement = javaElement; - this.index = index; + public static ArrayIndexElement withIntegerIndex(Element javaElement, Integer index) { + return new ArrayIndexElement(javaElement, index); + } + + /** + * Creates an ArrayIndexElement with a local variable or field index. + * + * @param javaElement the element of the array + * @param indexElement the local variable or field element representing the index + * @return an instance of ArrayIndexElement + */ + public static ArrayIndexElement withVariableIndex(Element javaElement, Element indexElement) { + return new ArrayIndexElement(javaElement, indexElement); } @Override diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index db00ea1b81..386f77fe15 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -365,11 +365,9 @@ public void fieldAccessIndexArray() { "import org.jspecify.annotations.Nullable;", "class Test {", " static @Nullable String [] fizz = {\"1\"};", - " static final Integer i = 0;", + " static final int i = 0;", " static void foo() {", " if (fizz[i]!=null) { ", - " // field access indexes aren't handled", - " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", " fizz[i].toString();", "}", " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", @@ -434,12 +432,18 @@ public void methodInvocationIndexArray() { " static int getIndex() {", " return 0;", " }", + " static final Integer i = 0;", " static void foo() {", " if (fizz[getIndex()] != null) {", " // index methods aren't handled by dataflow", " // BUG: Diagnostic contains: dereferenced expression fizz[getIndex()] is @Nullable", " fizz[getIndex()].toString();", " }", + " if (fizz[i] != null) {", + " // wrapper class indexes aren't handled by dataflow", + " // BUG: Diagnostic contains: dereferenced expression fizz[i] is @Nullable", + " fizz[i].toString();", + " }", " // BUG: Diagnostic contains: dereferenced expression fizz[getIndex()] is @Nullable", " fizz[getIndex()].toString();", " }", From 11e8c816b410c714b5e2db330bd802b38710a3d7 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Sun, 9 Jun 2024 12:33:04 -0500 Subject: [PATCH 10/10] Added test --- .../java/com/uber/nullaway/CoreTests.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/CoreTests.java b/nullaway/src/test/java/com/uber/nullaway/CoreTests.java index 14dbdea5c4..f3eb4d3711 100644 --- a/nullaway/src/test/java/com/uber/nullaway/CoreTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/CoreTests.java @@ -167,6 +167,29 @@ public void arrayIndexUnbox() { .doTest(); } + @Test + public void arrayAccessDataflowTest() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "class Test {", + " static class Foo {", + " @Nullable String f;", + " }", + " static Foo[] arr = new Foo[10];", + " static void fizz() {", + " int i = 0;", + " if (arr[i].f != null) {", + " //TODO: This should raise an error in non-JSpecify mode", + " arr[i].f.toString();", + " }", + " }", + "}") + .doTest(); + } + @Test public void cfNullableArrayField() { defaultCompilationHelper