Skip to content

Commit 0161a80

Browse files
authored
JSpecify: properly handle lambdas in anonymous inner classes (#1165)
Fixes #1156 When checking correctness of lambda overrides we get the type from `javac`'s inferred type rather than using the enclosing class. Also, when getting the type of an anonymous class we add some extra assertions to ensure we find the right `NewClassTree` representing the anonymous class. I _think_ we'll always get the right tree now, but the asserts give peace of mind.
1 parent 8786e88 commit 0161a80

File tree

3 files changed

+75
-9
lines changed

3 files changed

+75
-9
lines changed

nullaway/src/main/java/com/uber/nullaway/NullAway.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -807,12 +807,17 @@ private Description checkParamOverriding(
807807
} else if (config.isJSpecifyMode()) {
808808
// Check if the parameter type is a type variable and the corresponding generic type
809809
// argument is @Nullable
810-
if (memberReferenceTree != null) {
811-
// For a method reference, we get generic type arguments from the javac's inferred type
812-
// for the tree, which seems to properly preserve type-use annotations
810+
if (memberReferenceTree != null || lambdaExpressionTree != null) {
811+
// For a method reference or lambda, we get generic type arguments from the javac's
812+
// inferred type for the tree, which seems to properly preserve type-use annotations
813813
paramNullness =
814814
GenericsChecks.getGenericMethodParameterNullness(
815-
i, overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config);
815+
i,
816+
overriddenMethod,
817+
ASTHelpers.getType(
818+
memberReferenceTree != null ? memberReferenceTree : lambdaExpressionTree),
819+
state,
820+
config);
816821
} else {
817822
// Use the enclosing class of the overriding method to find generic type arguments
818823
paramNullness =

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

+11-5
Original file line numberDiff line numberDiff line change
@@ -820,16 +820,22 @@ public static Nullness getGenericMethodReturnTypeNullness(
820820
private static @Nullable Type getTypeForSymbol(Symbol symbol, VisitorState state, Config config) {
821821
if (symbol.isAnonymous()) {
822822
// For anonymous classes, symbol.type does not contain annotations on generic type parameters.
823-
// So, we get a correct type from the enclosing NewClassTree.
823+
// So, we get a correct type from the enclosing NewClassTree representing the anonymous class.
824824
TreePath path = state.getPath();
825-
NewClassTree newClassTree = ASTHelpers.findEnclosingNode(path, NewClassTree.class);
826-
if (newClassTree == null) {
825+
path = ASTHelpers.findPathFromEnclosingNodeToTopLevel(path, NewClassTree.class);
826+
NewClassTree newClassTree = (NewClassTree) path.getLeaf();
827+
if (newClassTree == null || newClassTree.getClassBody() == null) {
827828
throw new RuntimeException(
828-
"method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf()));
829+
"method should be directly inside an anonymous NewClassTree "
830+
+ state.getSourceForNode(path.getLeaf()));
829831
}
830832
Type typeFromTree = getTreeType(newClassTree, config);
831833
if (typeFromTree != null) {
832-
verify(state.getTypes().isAssignable(symbol.type, typeFromTree));
834+
verify(
835+
state.getTypes().isAssignable(symbol.type, typeFromTree),
836+
"%s is not assignable to %s",
837+
symbol.type,
838+
typeFromTree);
833839
}
834840
return typeFromTree;
835841
} else {

nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java

+55
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,32 @@ public void testForMethodReferenceInAnAssignment() {
507507
.doTest();
508508
}
509509

510+
@Test
511+
public void testForLambdaInAssignment() {
512+
makeHelper()
513+
.addSourceLines(
514+
"Test.java",
515+
"package com.uber;",
516+
"import org.jspecify.annotations.Nullable;",
517+
"import java.util.function.Supplier;",
518+
"class Test {",
519+
" interface A<T1 extends @Nullable Object> {",
520+
" String function(T1 o);",
521+
" }",
522+
" static String foo(Object o) {",
523+
" return o.toString();",
524+
" }",
525+
" static void testPositive() {",
526+
" // BUG: Diagnostic contains: dereferenced expression x is @Nullable",
527+
" A<@Nullable Object> p = x -> x.toString();",
528+
" }",
529+
" static void testNegative() {",
530+
" A<Object> p = x -> \"hello\";",
531+
" }",
532+
"}")
533+
.doTest();
534+
}
535+
510536
@Test
511537
public void testForMethodReferenceForClassFieldAssignment() {
512538
makeHelper()
@@ -2298,6 +2324,35 @@ public void issue1129() {
22982324
.doTest();
22992325
}
23002326

2327+
@Test
2328+
public void issue1156() {
2329+
makeHelper()
2330+
.addSourceLines(
2331+
"Foo.java",
2332+
"import org.jspecify.annotations.NullMarked;",
2333+
"import java.util.function.Function;",
2334+
"import java.util.function.Supplier;",
2335+
"@NullMarked",
2336+
"public class Foo implements Supplier<Integer> {",
2337+
" public Foo(Function<Integer, Integer> func) {",
2338+
" }",
2339+
" @Override",
2340+
" public Integer get() {",
2341+
" return 0;",
2342+
" }",
2343+
" public static void test() {",
2344+
" new Supplier<Boolean>() {",
2345+
" @Override",
2346+
" public Boolean get() {",
2347+
" Foo foo = new Foo(x -> 1);",
2348+
" return true;",
2349+
" }",
2350+
" };",
2351+
" }",
2352+
"}")
2353+
.doTest();
2354+
}
2355+
23012356
private CompilationTestHelper makeHelper() {
23022357
return makeTestHelperWithArgs(
23032358
Arrays.asList(

0 commit comments

Comments
 (0)