Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Checker Framework 3.39.0 #839

Merged
merged 6 commits into from
Oct 5, 2023
Merged

Update to Checker Framework 3.39.0 #839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
37c3536
Update to CF 3.39.0
msridhar Oct 2, 2023
deae7be
add test
msridhar Oct 2, 2023
cdf83ce
initial test for record deconstruction
msridhar Oct 2, 2023
b8c9fd0
another test
msridhar Oct 2, 2023
cc0297a
add a todo
msridhar Oct 3, 2023
ffecd6a
add issue link
msridhar Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle/dependencies.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if (project.hasProperty("epApiVersion")) {

def versions = [
asm : "9.3",
checkerFramework : "3.38.0",
checkerFramework : "3.39.0",
// for comparisons in other parts of the build
errorProneLatest : latestErrorProneVersion,
// The version of Error Prone used to check NullAway's code.
Expand Down
42 changes: 42 additions & 0 deletions jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,4 +210,46 @@ public void recordEqualsNull() {
"}")
.doTest();
}

@Test
public void recordDeconstructionPatternInstanceOf() {
defaultCompilationHelper
.addSourceLines(
"Records.java",
"package com.uber;",
"import javax.annotation.Nullable;",
"class Records {",
" record Rec(Object first, @Nullable Object second) { }",
" void recordDeconstructionInstanceOf(Object obj) {",
" if (obj instanceof Rec(Object f, @Nullable Object s)) {",
" f.toString();",
" // TODO: NullAway should report a warning here!",
" // See https://github.com/uber/NullAway/issues/840",
" s.toString();",
" }",
" }",
"}")
.doTest();
}

@Test
public void recordDeconstructionPatternSwitchCase() {
defaultCompilationHelper
.addSourceLines(
"Records.java",
"package com.uber;",
"import javax.annotation.Nullable;",
"class Records {",
" record Rec(Object first, @Nullable Object second) { }",
" int recordDeconstructionSwitchCase(Object obj) {",
" return switch (obj) {",
" // TODO: NullAway should report a warning here!",
" // See https://github.com/uber/NullAway/issues/840",
" case Rec(Object f, @Nullable Object s) -> s.toString().length();",
" default -> 0;",
" };",
" }",
"}")
.doTest();
}
}
11 changes: 9 additions & 2 deletions jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.uber.nullaway.NullAway;
import java.util.Arrays;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand Down Expand Up @@ -176,7 +175,6 @@ public void testSwitchExprLambda() {
.doTest();
}

@Ignore("requires fix for crash in Checker dataflow library")
@Test
public void testSwitchExprNullCase() {
defaultCompilationHelper
Expand All @@ -196,6 +194,15 @@ public void testSwitchExprNullCase() {
" case null -> throw new IllegalArgumentException(\"NullableEnum parameter is required\");",
" };",
" }",
" static Object handleNullableEnumNoCaseNull(@Nullable NullableEnum nullableEnum) {",
" // NOTE: in this case NullAway should report a bug, as there will be an NPE if nullableEnum",
" // is null (since there is no `case null` in the switch). This requires Error Prone support",
" // for matching on switch expressions (https://github.com/google/error-prone/issues/4119)",
" return switch (nullableEnum) {",
" case A -> new Object();",
" case B -> new Object();",
" };",
" }",
"}")
.doTest();
}
Expand Down
8 changes: 8 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import org.checkerframework.nullaway.dataflow.cfg.node.ConditionalAndNode;
import org.checkerframework.nullaway.dataflow.cfg.node.ConditionalNotNode;
import org.checkerframework.nullaway.dataflow.cfg.node.ConditionalOrNode;
import org.checkerframework.nullaway.dataflow.cfg.node.DeconstructorPatternNode;
import org.checkerframework.nullaway.dataflow.cfg.node.DoubleLiteralNode;
import org.checkerframework.nullaway.dataflow.cfg.node.EqualToNode;
import org.checkerframework.nullaway.dataflow.cfg.node.ExplicitThisNode;
Expand Down Expand Up @@ -1085,6 +1086,13 @@ public TransferResult<Nullness, NullnessStore> visitExpressionStatement(
return noStoreChanges(NULLABLE, input);
}

@Override
public TransferResult<Nullness, NullnessStore> visitDeconstructorPattern(
DeconstructorPatternNode deconstructorPatternNode,
TransferInput<Nullness, NullnessStore> input) {
return noStoreChanges(NULLABLE, input);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this marked as not covered? Because the codecov report is for an older JDK?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's due to typetools/checker-framework#6215, which is now fixed. Before, these nodes were not getting added properly to CFGs by Checker dataflow. Turns out checking code coverage is handy!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't get the coverage until Checker Framework does another release and we update, but I don't think it's worth waiting for that to land this.

}

@Override
public TransferResult<Nullness, NullnessStore> visitPackageName(
PackageNameNode packageNameNode, TransferInput<Nullness, NullnessStore> input) {
Expand Down