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

JSpecify: handle Nullability for return types of lambda expressions for Generic Types. #854

Merged
merged 5 commits into from
Nov 1, 2023
+69 −9
Merged

JSpecify: handle Nullability for return types of lambda expressions for Generic Types. #854

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
558d34a
adding test case and changes for handling return types in lambda expr…
akulk022 Oct 29, 2023
0947ce3
adding positive test scenario for
akulk022 Oct 29, 2023
630e6c2
add another test
msridhar Oct 29, 2023
5588b5d
updating logic for using checkReturnExpression
akulk022 Oct 31, 2023
b88266a
tweaks and documentation
msridhar Nov 1, 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
7 changes: 7 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,13 @@ private Description checkReturnExpression(
retExpr, methodSymbol, this, state);
if (getMethodReturnNullness(methodSymbol, state, Nullness.NULLABLE).equals(Nullness.NULLABLE)) {
return Description.NO_MATCH;
} else if (config.isJSpecifyMode()
&& GenericsChecks.getGenericMethodReturnTypeNullness(
methodSymbol, ASTHelpers.getType(tree), state, config)
.equals(Nullness.NULLABLE)) {
// Get the Nullness if the Annotation is indirectly applied through a generic type if we
// are in JSpecify mode
return Description.NO_MATCH;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code won't work for the case when the lambda is written x -> { return null; }. I updated the test and we get a false positive. The issue is that with the return null case, the tree passed in is the ReturnTree, not the tree for the lambda itself. One way to fix this would be to add another parameter @Nullable Tree lambdaTree to checkReturnExpression and then only run this case when lambdaTree is non-null. The extant tree parameter can be renamed to errorTree, since its main purpose is for finding the right locations to report errors.

}
if (mayBeNullExpr(state, retExpr)) {
return errorBuilder.createErrorDescriptionForNullAssignment(
Expand Down
22 changes: 22 additions & 0 deletions nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,28 @@ public void testForLambdasInAnAssignmentWithoutJSpecifyMode() {
.doTest();
}

@Test
public void testForLambdaReturnTypeInAnAssignment() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
" interface A<T1 extends @Nullable Object> {",
" T1 function(Object o);",
" }",
" static void testPositive() {",
" // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type",
" A<String> p = x -> null;",
" }",
" static void testNegative() {",
" A<@Nullable String> p = x -> null;",
" }",
"}")
.doTest();
}

@Test
public void testForDiamondInAnAssignment() {
makeHelper()
Expand Down