From b3eba82814944363811dfd2fd82d16690b09ab1a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 5 Nov 2024 13:08:39 -0800 Subject: [PATCH 01/55] fix and test --- .../main/java/com/uber/nullaway/NullAway.java | 2 +- .../nullaway/generics/GenericsChecks.java | 30 ++++++++++++++----- .../uber/nullaway/jspecify/GenericsTests.java | 21 +++++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index d4b1c81eb2..e2a11e1893 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1852,7 +1852,7 @@ private Description handleInvocation( } if (config.isJSpecifyMode()) { GenericsChecks.compareGenericTypeParameterNullabilityForCall( - formalParams, actualParams, varArgsMethod, this, state); + methodSymbol, tree, actualParams, varArgsMethod, this, state); if (!methodSymbol.getTypeParameters().isEmpty()) { GenericsChecks.checkGenericMethodCallTypeArguments(tree, state, this, config, handler); } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 7f39ca2375..9a39080d74 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -595,14 +595,16 @@ public static void checkTypeParameterNullnessForConditionalExpression( * Checks that for each parameter p at a call, the type parameter nullability for p's type matches * that of the corresponding formal parameter. If a mismatch is found, report an error. * - * @param formalParams the formal parameters - * @param actualParams the actual parameters + * @param methodSymbol the symbol for the method being called + * @param tree the tree representing the method call + * @param actualParams the actual parameters at the call * @param isVarArgs true if the call is to a varargs method * @param analysis the analysis object * @param state the visitor state */ public static void compareGenericTypeParameterNullabilityForCall( - List formalParams, + Symbol.MethodSymbol methodSymbol, + Tree tree, List actualParams, boolean isVarArgs, NullAway analysis, @@ -610,14 +612,26 @@ public static void compareGenericTypeParameterNullabilityForCall( if (!analysis.getConfig().isJSpecifyMode()) { return; } - int n = formalParams.size(); + Type invokedMethodType = methodSymbol.type; + if (!methodSymbol.isStatic() && tree instanceof MethodInvocationTree) { + // TODO what if the receiver is `this`? + Type enclosingType = + getTreeType( + ((MemberSelectTree) ((MethodInvocationTree) tree).getMethodSelect()).getExpression(), + state); + if (enclosingType != null) { + invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol); + } + } + List formalParamTypes = invokedMethodType.getParameterTypes(); + int n = formalParamTypes.size(); if (isVarArgs) { // If the last argument is var args, don't check it now, it will be checked against // all remaining actual arguments in the next loop. n = n - 1; } for (int i = 0; i < n; i++) { - Type formalParameter = formalParams.get(i).type; + Type formalParameter = formalParamTypes.get(i); if (formalParameter.isRaw()) { // bail out of any checking involving raw types for now return; @@ -630,11 +644,11 @@ public static void compareGenericTypeParameterNullabilityForCall( } } } - if (isVarArgs && !formalParams.isEmpty()) { + if (isVarArgs && !formalParamTypes.isEmpty()) { Type.ArrayType varargsArrayType = - (Type.ArrayType) formalParams.get(formalParams.size() - 1).type; + (Type.ArrayType) formalParamTypes.get(formalParamTypes.size() - 1); Type varargsElementType = varargsArrayType.elemtype; - for (int i = formalParams.size() - 1; i < actualParams.size(); i++) { + for (int i = formalParamTypes.size() - 1; i < actualParams.size(); i++) { Type actualParameterType = getTreeType(actualParams.get(i), state); // If the actual parameter type is assignable to the varargs array type, then the call site // is passing the varargs directly in an array, and we should skip our check. diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index c686b25f0c..a75c43864e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -938,6 +938,27 @@ public void parameterPassing() { .doTest(); } + @Test + public void parameterPassingInstanceMethods() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class A {", + " void foo(A a) {}", + " }", + " static void test(A<@Nullable String> p, A q) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type", + " p.foo(q);", + " // this one is fine", + " p.foo(p);", + " }", + "}") + .doTest(); + } + @Test public void varargsParameter() { makeHelper() From bc9e5fb9681a8f1bbaf6dd7f52cabe0827c99f4f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 5 Nov 2024 13:25:43 -0800 Subject: [PATCH 02/55] another fix and test --- .../com/uber/nullaway/generics/GenericsChecks.java | 13 ++++++++----- .../com/uber/nullaway/jspecify/GenericsTests.java | 1 + 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 9a39080d74..1a5f1ee73c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -614,11 +614,14 @@ public static void compareGenericTypeParameterNullabilityForCall( } Type invokedMethodType = methodSymbol.type; if (!methodSymbol.isStatic() && tree instanceof MethodInvocationTree) { - // TODO what if the receiver is `this`? - Type enclosingType = - getTreeType( - ((MemberSelectTree) ((MethodInvocationTree) tree).getMethodSelect()).getExpression(), - state); + ExpressionTree methodSelect = ((MethodInvocationTree) tree).getMethodSelect(); + Type enclosingType; + if (methodSelect instanceof MemberSelectTree) { + enclosingType = getTreeType(((MemberSelectTree) methodSelect).getExpression(), state); + } else { + // implicit this parameter + enclosingType = methodSymbol.owner.type; + } if (enclosingType != null) { invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol); } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index a75c43864e..74a7ef481f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -948,6 +948,7 @@ public void parameterPassingInstanceMethods() { "class Test {", " static class A {", " void foo(A a) {}", + " void bar(A a) { foo(a); this.foo(a); }", " }", " static void test(A<@Nullable String> p, A q) {", " // BUG: Diagnostic contains: Cannot pass parameter of type", From 174b26231ee87953824e6bac391701aeb158c48a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 5 Nov 2024 14:29:06 -0800 Subject: [PATCH 03/55] add TODO --- .../src/main/java/com/uber/nullaway/generics/GenericsChecks.java | 1 + 1 file changed, 1 insertion(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 1a5f1ee73c..3810e2475f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -626,6 +626,7 @@ public static void compareGenericTypeParameterNullabilityForCall( invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol); } } + // TODO handle generic methods, by calling state.getTypes().subst on invokedMethodType List formalParamTypes = invokedMethodType.getParameterTypes(); int n = formalParamTypes.size(); if (isVarArgs) { From c94dc28110a80ec017b85feb041502f20fa43c21 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 11 Nov 2024 01:24:08 -0800 Subject: [PATCH 04/55] check explicit type arguments when comparing generic type param nullability at call --- .../com/uber/nullaway/generics/GenericsChecks.java | 11 +++++++++++ .../uber/nullaway/jspecify/GenericMethodTests.java | 3 +-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 3810e2475f..36b656c3d4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -628,6 +628,17 @@ public static void compareGenericTypeParameterNullabilityForCall( } // TODO handle generic methods, by calling state.getTypes().subst on invokedMethodType List formalParamTypes = invokedMethodType.getParameterTypes(); + if (tree instanceof MethodInvocationTree && methodSymbol.type instanceof Type.ForAll) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree; + + List typeArgumentTrees = methodInvocationTree.getTypeArguments(); + com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); + + Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; + Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; + formalParamTypes = + state.getTypes().subst(underlyingMethodType.argtypes, forAllType.tvars, explicitTypeArgs); + } int n = formalParamTypes.size(); if (isVarArgs) { // If the last argument is var args, don't check it now, it will be checked against diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 6b9d409ba2..bd3bfb4748 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -105,7 +105,6 @@ public void genericInstanceMethods() { } @Test - @Ignore("requires generic method support") public void genericMethodAndVoidType() { makeHelper() .addSourceLines( @@ -127,7 +126,7 @@ public void genericMethodAndVoidType() { " }", " static void test(Foo f) {", " // this is safe", - " f.foo(null, new MyVisitor());", + " f.<@Nullable Void>foo(null, new MyVisitor());", " }", "}") .doTest(); From 32b87ab42b9244558e6c4e43e4c4a5fdd6e9b23b Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 11 Nov 2024 01:28:55 -0800 Subject: [PATCH 05/55] formatting --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 2 +- .../java/com/uber/nullaway/jspecify/GenericMethodTests.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 36b656c3d4..bac3061479 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -637,7 +637,7 @@ public static void compareGenericTypeParameterNullabilityForCall( Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; formalParamTypes = - state.getTypes().subst(underlyingMethodType.argtypes, forAllType.tvars, explicitTypeArgs); + state.getTypes().subst(underlyingMethodType.argtypes, forAllType.tvars, explicitTypeArgs); } int n = formalParamTypes.size(); if (isVarArgs) { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index bd3bfb4748..70b9973a76 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -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 GenericMethodTests extends NullAwayTestsBase { From e1dfa8a00e110c134082bb8cecc5dc3b7bd58a57 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 11 Nov 2024 08:50:54 -0800 Subject: [PATCH 06/55] add test case for explicit type arguments --- .../nullaway/jspecify/GenericMethodTests.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 70b9973a76..40a356925b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -3,6 +3,7 @@ import com.google.errorprone.CompilationTestHelper; import com.uber.nullaway.NullAwayTestsBase; import java.util.Arrays; +import org.junit.Ignore; import org.junit.Test; public class GenericMethodTests extends NullAwayTestsBase { @@ -131,6 +132,35 @@ public void genericMethodAndVoidType() { .doTest(); } + @Test + @Ignore("requires generic method support") + public void genericMethodAndVoidTypeWithInference() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class Foo {", + " void foo(C c, Visitor visitor) {", + " visitor.visit(this, c);", + " }", + " }", + " static abstract class Visitor {", + " abstract void visit(Foo foo, C c);", + " }", + " static class MyVisitor extends Visitor<@Nullable Void> {", + " @Override", + " void visit(Foo foo, @Nullable Void c) {}", + " }", + " static void test(Foo f) {", + " // this is safe", + " f.foo(null, new MyVisitor());", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 24ca5a1f05672e1bd7c58d07015d777fb8f8b34d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 11 Nov 2024 09:17:01 -0800 Subject: [PATCH 07/55] apply substitution to method type --- .../java/com/uber/nullaway/generics/GenericsChecks.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index bac3061479..2093599823 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -626,8 +626,7 @@ public static void compareGenericTypeParameterNullabilityForCall( invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol); } } - // TODO handle generic methods, by calling state.getTypes().subst on invokedMethodType - List formalParamTypes = invokedMethodType.getParameterTypes(); + // Handle generic methods if (tree instanceof MethodInvocationTree && methodSymbol.type instanceof Type.ForAll) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree; @@ -636,9 +635,10 @@ public static void compareGenericTypeParameterNullabilityForCall( Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; - formalParamTypes = - state.getTypes().subst(underlyingMethodType.argtypes, forAllType.tvars, explicitTypeArgs); + invokedMethodType = + state.getTypes().subst(underlyingMethodType, forAllType.tvars, explicitTypeArgs); } + List formalParamTypes = invokedMethodType.getParameterTypes(); int n = formalParamTypes.size(); if (isVarArgs) { // If the last argument is var args, don't check it now, it will be checked against From 3f3ebf282b972eef0de96a72391828405c8cf353 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Tue, 12 Nov 2024 13:48:15 -0800 Subject: [PATCH 08/55] make function that substitutes explicit type arguments --- .../nullaway/generics/GenericsChecks.java | 50 +++++++------------ 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 2093599823..d95ebec3ee 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -628,15 +628,8 @@ public static void compareGenericTypeParameterNullabilityForCall( } // Handle generic methods if (tree instanceof MethodInvocationTree && methodSymbol.type instanceof Type.ForAll) { - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree; - - List typeArgumentTrees = methodInvocationTree.getTypeArguments(); - com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); - - Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; - Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; invokedMethodType = - state.getTypes().subst(underlyingMethodType, forAllType.tvars, explicitTypeArgs); + substituteGenericTypeArgsToExplicit((MethodInvocationTree) tree, methodSymbol, state); } List formalParamTypes = invokedMethodType.getParameterTypes(); int n = formalParamTypes.size(); @@ -825,19 +818,9 @@ public static Nullness getGenericReturnNullnessAtInvocation( Config config) { // If generic method invocation if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { - List typeArgumentTrees = tree.getTypeArguments(); - com.sun.tools.javac.util.List explicitTypeArgs = - convertTreesToTypes(typeArgumentTrees); // Convert to Type objects - Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type; - // Extract the underlying MethodType (the actual signature) - Type.MethodType methodTypeInsideForAll = (Type.MethodType) forAllType.asMethodType(); // Substitute type arguments inside the return type - // NOTE: if the return type it not a type variable of the method itself, or if - // explicitTypeArgs is empty, this is a noop. Type substitutedReturnType = - state - .getTypes() - .subst(methodTypeInsideForAll.restype, forAllType.tvars, explicitTypeArgs); + substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getReturnType(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class if (substitutedReturnType != null @@ -871,6 +854,20 @@ private static com.sun.tools.javac.util.List convertTreesToTypes( return com.sun.tools.javac.util.List.from(types); } + private static Type substituteGenericTypeArgsToExplicit( + MethodInvocationTree methodInvocationTreetree, + Symbol.MethodSymbol methodSymbol, + VisitorState state) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) methodInvocationTreetree; + + List typeArgumentTrees = methodInvocationTree.getTypeArguments(); + com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); + + Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; + Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; + return state.getTypes().subst(underlyingMethodType, forAllType.tvars, explicitTypeArgs); + } + /** * Computes the nullness of a formal parameter of a generic method at an invocation, in the * context of the declared type of its receiver argument. If the formal parameter's type is a type @@ -913,23 +910,10 @@ public static Nullness getGenericParameterNullnessAtInvocation( Config config) { // If generic method invocation if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { - List typeArgumentTrees = tree.getTypeArguments(); - com.sun.tools.javac.util.List explicitTypeArgs = - convertTreesToTypes(typeArgumentTrees); // Convert to Type objects - - Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type; - // Extract the underlying MethodType (the actual signature) - Type.MethodType methodTypeInsideForAll = (Type.MethodType) forAllType.qtype; // Substitute the argument types within the MethodType // NOTE: if explicitTypeArgs is empty, this is a noop List substitutedParamTypes = - state - .getTypes() - .subst( - methodTypeInsideForAll.argtypes, - forAllType.tvars, // The type variables from the ForAll - explicitTypeArgs // The actual type arguments from the method invocation - ); + substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class if (substitutedParamTypes != null From 7004926383b77817170a6af1f4bbc1176e6edaf4 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 18 Nov 2024 11:19:57 -0800 Subject: [PATCH 09/55] test case --- .../nullaway/jspecify/GenericMethodTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 40a356925b..68e21d1cb5 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -161,6 +161,30 @@ public void genericMethodAndVoidTypeWithInference() { .doTest(); } + @Test + public void genericInferenceOnAssignments() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + " class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo make(U u) {", + " return new Foo<>(u);", + " }", + " }", + " static void testLocalAssign() {", + " // legal", + " Foo<@Nullable Object> f = Foo.make(null);", + " // illegal", + " Foo f2 = Foo.make(null);", + " }", + " }") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From aaba869eb71cab035e3d51fcfa6962e7d06a9841 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sat, 7 Dec 2024 15:04:55 -0800 Subject: [PATCH 10/55] add test for multiple generics --- .../com/uber/nullaway/jspecify/GenericMethodTests.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 68e21d1cb5..fec0d4afce 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -175,11 +175,19 @@ public void genericInferenceOnAssignments() { " return new Foo<>(u);", " }", " }", + " static class Bar {", + " Bar(S s, Z z) {}", + " static Bar make(U a, B b) {", + " return new Bar<>(a, b);", + " }", + " }", " static void testLocalAssign() {", " // legal", " Foo<@Nullable Object> f = Foo.make(null);", - " // illegal", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f2 = Foo.make(null);", + " // legal", + " Bar<@Nullable Object, Object> b = Bar.make(null, new Object());", " }", " }") .doTest(); From 37ef4d11c15372dcf2e2be44035d2242dd06d625 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sat, 7 Dec 2024 15:05:48 -0800 Subject: [PATCH 11/55] temp structure of inferred_types cache - need to change map key type --- .../main/java/com/uber/nullaway/NullAway.java | 63 ++++++++++- .../nullaway/generics/GenericsChecks.java | 105 ++++++++++++++++++ 2 files changed, 166 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 38b1b1d6dd..15577431e1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -106,6 +106,7 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Target; import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -275,6 +276,9 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); +// private final Map inferred_types = new HashMap<>(); + private final Map inferred_types = new HashMap<>(); + /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the * constructor taking an ErrorProneFlags object. This constructor should not be used anywhere @@ -1478,9 +1482,33 @@ public Description matchVariable(VariableTree tree, VisitorState state) { if (!withinAnnotatedCode(state)) { return Description.NO_MATCH; } + Tree rhsTree = tree.getInitializer(); + if(rhsTree instanceof MethodInvocationTree) { + MethodInvocationTree methodTree = (MethodInvocationTree) rhsTree; + Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(methodTree); + if(methodSymbol.type instanceof Type.ForAll + && methodTree.getTypeArguments().isEmpty()) { + Tree lhsTree = tree.getType(); + if(lhsTree instanceof ParameterizedTypeTree) { + List typeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); + Type baseType = methodSymbol.asType(); + List baseTypeVariables = baseType.getTypeArguments(); + if(!typeArguments.isEmpty()) { + for(int i=0; i actualParams) { List formalParams = methodSymbol.getParameters(); + // if method invocation is in a variable declaration and it doesn't have explicit type arguments + if (tree instanceof MethodInvocationTree) { + MethodInvocationTree methodTree = (MethodInvocationTree) tree; + if (methodSymbol.type instanceof Type.ForAll + && methodTree.getTypeArguments().isEmpty()) { // if generic method && no explicit type arguments + TreePath parentPath = state.getPath().getParentPath(); + Tree parentTree = parentPath.getLeaf(); + if (parentTree instanceof VariableTree) { + VariableTree varTree = (VariableTree) parentTree; // the declaration statement tree +// var i = varTree.getInitializer(); // RHS function call : Foo.make(null) +// var m = varTree.getModifiers(); // "" +// var n = varTree.getName(); // declared variable name : "f" +// var ne = varTree.getNameExpression(); // null in this case (don't need for this case) : null + Tree t = varTree.getType(); // the inferred type : Foo<@Nullable Object> + if (t instanceof ParameterizedTypeTree) { + List typeArguments = ((ParameterizedTypeTree) t).getTypeArguments(); + Type baseType = methodSymbol.asType(); + List baseTypeVariables = baseType.getTypeArguments(); + if (!typeArguments.isEmpty()) { + for (int i = 0; i < baseTypeVariables.size(); i++) { + var tmp = typeArguments.get(i); + var tttt = tmp.getKind(); + if (tttt == null) {} + inferred_types.put(baseTypeVariables.get(i), typeArguments.get(i)); + } + } + } + } + } + } + boolean varArgsMethod = methodSymbol.isVarArgs(); if (formalParams.size() != actualParams.size() && !varArgsMethod @@ -1846,7 +1905,7 @@ private Description handleInvocation( ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? GenericsChecks.getGenericParameterNullnessAtInvocation( - i, methodSymbol, (MethodInvocationTree) tree, state, config) + i, methodSymbol, (MethodInvocationTree) tree, state, config, inferred_types) : Nullness.NONNULL); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index d95ebec3ee..9c2c660e8c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -441,6 +441,57 @@ public static void checkTypeParameterNullnessForAssignability( } } + public static void checkTypeParameterNullnessForAssignability( + Tree tree, NullAway analysis, VisitorState state, Map inferred_types) { + if (!analysis.getConfig().isJSpecifyMode()) { + return; + } + Type lhsType = getTreeType(tree, state); + Tree rhsTree; + if (tree instanceof VariableTree) { + VariableTree varTree = (VariableTree) tree; + rhsTree = varTree.getInitializer(); + } else { + AssignmentTree assignmentTree = (AssignmentTree) tree; + rhsTree = assignmentTree.getExpression(); + } + // rhsTree can be null for a VariableTree. Also, we don't need to do a check + // if rhsTree is the null literal + if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { + return; + } + Type rhsType = getTreeType(rhsTree, state); + + if(rhsType != null && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types TODO: let's see if there is other ways to check this + if(rhsTree instanceof MethodInvocationTree) { + Symbol.MethodSymbol invokedMethodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); + + List typeParam = invokedMethodSymbol.getTypeParameters(); + List newTypeArgument = new ArrayList<>(); + for (int i = 0; i < typeParam.size(); i++) { + if (inferred_types.containsKey(typeParam.get(i).type)) { + var pType = typeParam.get(i).type; + Tree inferred_tree = (Tree) inferred_types.get(pType); + newTypeArgument.add(getTreeType(inferred_tree, state)); + } + } + Type.ClassType classType = (Type.ClassType) rhsType; + rhsType = new Type.ClassType( + classType.getEnclosingType(), + com.sun.tools.javac.util.List.from(newTypeArgument), + classType.tsym + ); + } + } + + if (lhsType != null && rhsType != null) { + boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state); + if (!isAssignmentValid) { + reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); + } + } + } + /** * Checks that the nullability of type parameters for a returned expression matches that of the * type parameters of the enclosing method's return type. @@ -932,6 +983,51 @@ public static Nullness getGenericParameterNullnessAtInvocation( paramIndex, invokedMethodSymbol, enclosingType, state, config); } + public static Nullness getGenericParameterNullnessAtInvocation( + int paramIndex, + Symbol.MethodSymbol invokedMethodSymbol, + MethodInvocationTree tree, + VisitorState state, + Config config, + Map inferred_types) { + // If generic method invocation + if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { + // Substitute the argument types within the MethodType + // NOTE: if explicitTypeArgs is empty, this is a noop + List substitutedParamTypes = + substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); + // If this condition evaluates to false, we fall through to the subsequent logic, to handle + // type variables declared on the enclosing class + if (substitutedParamTypes != null + && Objects.equals( + getTypeNullness(substitutedParamTypes.get(paramIndex), config), Nullness.NULLABLE)) { + return Nullness.NULLABLE; + } + // check nullness of inferred types + List typeParam = invokedMethodSymbol.getTypeParameters(); + for(int i=0; i inferred_types) { +// boolean hasNullableAnnotation = +// Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config); +// if (hasNullableAnnotation) { +// return Nullness.NULLABLE; +// } +// return Nullness.NONNULL; +// } + /** * Returns a pretty-printed representation of type suitable for error messages. The representation * uses simple names rather than fully-qualified names, and retains all type-use annotations. From c6a62448a938b92d60d876affc6f076e993920e6 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 13 Jan 2025 10:00:41 -0800 Subject: [PATCH 12/55] infer on assignments draft --- .../main/java/com/uber/nullaway/NullAway.java | 120 +++++++++--------- .../nullaway/generics/GenericsChecks.java | 88 +++++++++---- .../nullaway/jspecify/GenericMethodTests.java | 8 +- 3 files changed, 127 insertions(+), 89 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 15577431e1..205e59084f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -276,8 +276,8 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); -// private final Map inferred_types = new HashMap<>(); - private final Map inferred_types = new HashMap<>(); + private final Map> inferredTypes = new HashMap<>(); +// private final Map inferredTypes = new HashMap<>(); /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the @@ -1482,33 +1482,37 @@ public Description matchVariable(VariableTree tree, VisitorState state) { if (!withinAnnotatedCode(state)) { return Description.NO_MATCH; } - Tree rhsTree = tree.getInitializer(); - if(rhsTree instanceof MethodInvocationTree) { - MethodInvocationTree methodTree = (MethodInvocationTree) rhsTree; - Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(methodTree); - if(methodSymbol.type instanceof Type.ForAll - && methodTree.getTypeArguments().isEmpty()) { - Tree lhsTree = tree.getType(); - if(lhsTree instanceof ParameterizedTypeTree) { - List typeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); - Type baseType = methodSymbol.asType(); - List baseTypeVariables = baseType.getTypeArguments(); - if(!typeArguments.isEmpty()) { - for(int i=0; i typeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); +// Type baseType = methodSymbol.asType(); +// List baseTypeVariables = baseType.getTypeArguments(); +// if(!typeArguments.isEmpty()) { +// for(int i=0; i actualParams) { List formalParams = methodSymbol.getParameters(); - // if method invocation is in a variable declaration and it doesn't have explicit type arguments - if (tree instanceof MethodInvocationTree) { - MethodInvocationTree methodTree = (MethodInvocationTree) tree; - if (methodSymbol.type instanceof Type.ForAll - && methodTree.getTypeArguments().isEmpty()) { // if generic method && no explicit type arguments - TreePath parentPath = state.getPath().getParentPath(); - Tree parentTree = parentPath.getLeaf(); - if (parentTree instanceof VariableTree) { - VariableTree varTree = (VariableTree) parentTree; // the declaration statement tree -// var i = varTree.getInitializer(); // RHS function call : Foo.make(null) -// var m = varTree.getModifiers(); // "" -// var n = varTree.getName(); // declared variable name : "f" -// var ne = varTree.getNameExpression(); // null in this case (don't need for this case) : null - Tree t = varTree.getType(); // the inferred type : Foo<@Nullable Object> - if (t instanceof ParameterizedTypeTree) { - List typeArguments = ((ParameterizedTypeTree) t).getTypeArguments(); - Type baseType = methodSymbol.asType(); - List baseTypeVariables = baseType.getTypeArguments(); - if (!typeArguments.isEmpty()) { - for (int i = 0; i < baseTypeVariables.size(); i++) { - var tmp = typeArguments.get(i); - var tttt = tmp.getKind(); - if (tttt == null) {} - inferred_types.put(baseTypeVariables.get(i), typeArguments.get(i)); - } - } - } - } - } - } +// // if method invocation is in a variable declaration and it doesn't have explicit type arguments +// if (tree instanceof MethodInvocationTree) { +// MethodInvocationTree methodTree = (MethodInvocationTree) tree; +// if (methodSymbol.type instanceof Type.ForAll +// && methodTree.getTypeArguments().isEmpty()) { // if generic method && no explicit type arguments +// TreePath parentPath = state.getPath().getParentPath(); +// Tree parentTree = parentPath.getLeaf(); +// if (parentTree instanceof VariableTree) { +// VariableTree varTree = (VariableTree) parentTree; // the declaration statement tree +//// var i = varTree.getInitializer(); // RHS function call : Foo.make(null) +//// var m = varTree.getModifiers(); // "" +//// var n = varTree.getName(); // declared variable name : "f" +//// var ne = varTree.getNameExpression(); // null in this case (don't need for this case) : null +// Tree t = varTree.getType(); // the inferred type : Foo<@Nullable Object> +// if (t instanceof ParameterizedTypeTree) { // has a generic type in it +// List typeArguments = ((ParameterizedTypeTree) t).getTypeArguments(); +// Type baseType = methodSymbol.asType(); +// List baseTypeVariables = baseType.getTypeArguments(); +// if (!typeArguments.isEmpty()) { +// for (int i = 0; i < baseTypeVariables.size(); i++) { +//// var tmp = typeArguments.get(i); +//// var tttt = tmp.getKind(); +//// if (tttt == null) {} +//// inferredTypes.put(baseTypeVariables.get(i), ASTHelpers.getType(typeArguments.get(i))); +// } +// } +// } +// } +// } +// } boolean varArgsMethod = methodSymbol.isVarArgs(); if (formalParams.size() != actualParams.size() @@ -1905,7 +1909,7 @@ private Description handleInvocation( ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? GenericsChecks.getGenericParameterNullnessAtInvocation( - i, methodSymbol, (MethodInvocationTree) tree, state, config, inferred_types) + i, methodSymbol, (MethodInvocationTree) tree, state, config, inferredTypes) : Nullness.NONNULL); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 9c2c660e8c..86b9d59ee3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -442,15 +442,45 @@ public static void checkTypeParameterNullnessForAssignability( } public static void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state, Map inferred_types) { + Tree tree, NullAway analysis, VisitorState state, Map> inferredTypes) { if (!analysis.getConfig().isJSpecifyMode()) { return; } Type lhsType = getTreeType(tree, state); Tree rhsTree; +// VariableTree varTree = null; if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); + // rhs is a method call + if(rhsTree instanceof MethodInvocationTree) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); + // generic method, no explicit generic arguments + if(methodSymbol.type instanceof Type.ForAll && methodInvocationTree.getTypeArguments().isEmpty()) { + Tree lhsTree = varTree.getType(); // Foo<@Nullable Object> + // lhs has generic types + if(lhsTree instanceof ParameterizedTypeTree) { + List lhsTypeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); // @Nullable Object + Type returnType = methodSymbol.getReturnType(); // com.uber.Test.Foo + // method call has a return type of class type + if(returnType instanceof Type.ClassType) { + Type.ClassType classType = (Type.ClassType) returnType; + List rhsTypeArguments = classType.getTypeArguments(); // "U" + // no explicit generic type given + if(!rhsTypeArguments.isEmpty()) { + Map genericNullness = new HashMap<>(); + for(int i=0; i @Nullable Object + } + inferredTypes.put(tree, genericNullness); + } + } + } + } + + } } else { AssignmentTree assignmentTree = (AssignmentTree) tree; rhsTree = assignmentTree.getExpression(); @@ -462,20 +492,25 @@ public static void checkTypeParameterNullnessForAssignability( } Type rhsType = getTreeType(rhsTree, state); - if(rhsType != null && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types TODO: let's see if there is other ways to check this + if(rhsType != null && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types if(rhsTree instanceof MethodInvocationTree) { Symbol.MethodSymbol invokedMethodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); List typeParam = invokedMethodSymbol.getTypeParameters(); List newTypeArgument = new ArrayList<>(); - for (int i = 0; i < typeParam.size(); i++) { - if (inferred_types.containsKey(typeParam.get(i).type)) { - var pType = typeParam.get(i).type; - Tree inferred_tree = (Tree) inferred_types.get(pType); - newTypeArgument.add(getTreeType(inferred_tree, state)); + + if(inferredTypes.containsKey(tree)) { + Map genericNullness = inferredTypes.get(tree); + for (int i = 0; i < typeParam.size(); i++) { + if (genericNullness.containsKey(typeParam.get(i).type)) { + var pType = typeParam.get(i).type; +// Tree inferred_tree = (Tree) inferredTypes.get(pType); + newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types + } } } Type.ClassType classType = (Type.ClassType) rhsType; + // create a new Type.ClassType that has inferred types rhsType = new Type.ClassType( classType.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgument), @@ -989,7 +1024,7 @@ public static Nullness getGenericParameterNullnessAtInvocation( MethodInvocationTree tree, VisitorState state, Config config, - Map inferred_types) { + Map> inferredTypes) { // If generic method invocation if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { // Substitute the argument types within the MethodType @@ -1004,18 +1039,24 @@ public static Nullness getGenericParameterNullnessAtInvocation( return Nullness.NULLABLE; } // check nullness of inferred types - List typeParam = invokedMethodSymbol.getTypeParameters(); - for(int i=0; i genericNullness = inferredTypes.get(parentTree); + List typeParam = invokedMethodSymbol.getTypeParameters(); +// for (int i = 0; i < typeParam.size(); i++) { + if (genericNullness.containsKey(typeParam.get(paramIndex).type)) { + var pType = typeParam.get(paramIndex).type; +// Tree inferred_tree = (Tree) inferredTypes.get(pType); + Type in_type = genericNullness.get(pType); + if (Objects.equals(getTypeNullness(in_type, config), Nullness.NULLABLE)) { + return Nullness.NULLABLE; + } else { + return Nullness.NONNULL; + } } - } + +// } } } @@ -1173,15 +1214,6 @@ private static Nullness getTypeNullness(Type type, Config config) { return Nullness.NONNULL; } -// private static Nullness getTypeNullness(Type type, Config config, Map inferred_types) { -// boolean hasNullableAnnotation = -// Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config); -// if (hasNullableAnnotation) { -// return Nullness.NULLABLE; -// } -// return Nullness.NONNULL; -// } - /** * Returns a pretty-printed representation of type suitable for error messages. The representation * uses simple names rather than fully-qualified names, and retains all type-use annotations. diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index fec0d4afce..5e68120a3e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -171,7 +171,7 @@ public void genericInferenceOnAssignments() { " class Test {", " static class Foo {", " Foo(T t) {}", - " static Foo make(U u) {", + " static Foo make(U u) {", // use return type for inference " return new Foo<>(u);", " }", " }", @@ -182,12 +182,14 @@ public void genericInferenceOnAssignments() { " }", " }", " static void testLocalAssign() {", - " // legal", - " Foo<@Nullable Object> f = Foo.make(null);", + " // legal", // [Foo.make(null) -> [U -> @Nullable Object, T -> …] ] ==> [Foo<@Nullable Object> f = Foo.make(null) -> [U -> @Nullable Object, T -> …] ] + " Foo<@Nullable Object> f = Foo.make(null);", // the context we need to use is that it is an assignment " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f2 = Foo.make(null);", " // legal", " Bar<@Nullable Object, Object> b = Bar.make(null, new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", " }", " }") .doTest(); From 277e2b6b815cbf1cedbdf1239c720b8a49f1ddac Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 13 Jan 2025 11:04:40 -0800 Subject: [PATCH 13/55] new test case for inference on assignments --- .../nullaway/jspecify/GenericMethodTests.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 5e68120a3e..79ac9731f8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -195,6 +195,40 @@ public void genericInferenceOnAssignments() { .doTest(); } + @Test + public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + " class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo make(U u) {", // use return type for inference + " return new Foo<>(u);", + " }", + " }", +// " static class Bar {", +// " Bar(S s, Z z) {}", +// " static Bar make(U a, B b) {", +// " return new Bar<>(a, b);", +// " }", +// " }", + " static void testLocalAssign() {", + " // legal", + " var f1 = Foo.make(new String());", + " // legal: infers Foo<@Nullable Object>", + " var f2 = Foo.make(null);", +// " // legal", +// " Bar<@Nullable Object, Object> b = Bar.make(null, new Object());", +// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", +// " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", + " }", + " }") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From e7346d304602d73748ae13a2d9116e6387caa779 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 13 Jan 2025 11:07:49 -0800 Subject: [PATCH 14/55] update test case --- .../nullaway/jspecify/GenericMethodTests.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 79ac9731f8..a3fe39a56f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -209,21 +209,21 @@ public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { " return new Foo<>(u);", " }", " }", -// " static class Bar {", -// " Bar(S s, Z z) {}", -// " static Bar make(U a, B b) {", -// " return new Bar<>(a, b);", -// " }", -// " }", + " static class Bar {", + " Bar(S s, Z z) {}", + " static Bar make(U a, B b) {", + " return new Bar<>(a, b);", + " }", + " }", " static void testLocalAssign() {", " // legal", " var f1 = Foo.make(new String());", " // legal: infers Foo<@Nullable Object>", " var f2 = Foo.make(null);", -// " // legal", -// " Bar<@Nullable Object, Object> b = Bar.make(null, new Object());", -// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", -// " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", + " // legal", + " var b1 = Bar.make(new String(), new Object());", + " var b2 = Bar.make(new String(), null);", + " var b3 = Bar.make(null, null);", " }", " }") .doTest(); From bca1d7d55aa3f273db3494a6e3f731a411983d42 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 15 Jan 2025 14:09:15 -0800 Subject: [PATCH 15/55] simple cleanup --- .../main/java/com/uber/nullaway/NullAway.java | 61 +--------- .../nullaway/generics/GenericsChecks.java | 108 +++--------------- .../nullaway/jspecify/GenericMethodTests.java | 9 +- 3 files changed, 25 insertions(+), 153 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 205e59084f..a8902ff70f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -491,7 +491,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } // generics check if (lhsType != null && config.isJSpecifyMode()) { - GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); + GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, inferredTypes); } if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { @@ -1482,34 +1482,6 @@ public Description matchVariable(VariableTree tree, VisitorState state) { if (!withinAnnotatedCode(state)) { return Description.NO_MATCH; } -// Tree rhsTree = tree.getInitializer(); -// if(rhsTree instanceof MethodInvocationTree) { -// MethodInvocationTree methodTree = (MethodInvocationTree) rhsTree; -// Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(methodTree); -// if(methodSymbol.type instanceof Type.ForAll -// && methodTree.getTypeArguments().isEmpty()) { -// Tree lhsTree = tree.getType(); -// if(lhsTree instanceof ParameterizedTypeTree) { -// List typeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); -// Type baseType = methodSymbol.asType(); -// List baseTypeVariables = baseType.getTypeArguments(); -// if(!typeArguments.isEmpty()) { -// for(int i=0; i actualParams) { List formalParams = methodSymbol.getParameters(); -// // if method invocation is in a variable declaration and it doesn't have explicit type arguments -// if (tree instanceof MethodInvocationTree) { -// MethodInvocationTree methodTree = (MethodInvocationTree) tree; -// if (methodSymbol.type instanceof Type.ForAll -// && methodTree.getTypeArguments().isEmpty()) { // if generic method && no explicit type arguments -// TreePath parentPath = state.getPath().getParentPath(); -// Tree parentTree = parentPath.getLeaf(); -// if (parentTree instanceof VariableTree) { -// VariableTree varTree = (VariableTree) parentTree; // the declaration statement tree -//// var i = varTree.getInitializer(); // RHS function call : Foo.make(null) -//// var m = varTree.getModifiers(); // "" -//// var n = varTree.getName(); // declared variable name : "f" -//// var ne = varTree.getNameExpression(); // null in this case (don't need for this case) : null -// Tree t = varTree.getType(); // the inferred type : Foo<@Nullable Object> -// if (t instanceof ParameterizedTypeTree) { // has a generic type in it -// List typeArguments = ((ParameterizedTypeTree) t).getTypeArguments(); -// Type baseType = methodSymbol.asType(); -// List baseTypeVariables = baseType.getTypeArguments(); -// if (!typeArguments.isEmpty()) { -// for (int i = 0; i < baseTypeVariables.size(); i++) { -//// var tmp = typeArguments.get(i); -//// var tttt = tmp.getKind(); -//// if (tttt == null) {} -//// inferredTypes.put(baseTypeVariables.get(i), ASTHelpers.getType(typeArguments.get(i))); -// } -// } -// } -// } -// } -// } - boolean varArgsMethod = methodSymbol.isVarArgs(); if (formalParams.size() != actualParams.size() && !varArgsMethod diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 86b9d59ee3..624e4f9717 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -412,35 +412,6 @@ private static void reportInvalidOverridingMethodParamTypeError( * @param analysis the analysis object * @param state the visitor state */ - public static void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state) { - if (!analysis.getConfig().isJSpecifyMode()) { - return; - } - Type lhsType = getTreeType(tree, state); - Tree rhsTree; - if (tree instanceof VariableTree) { - VariableTree varTree = (VariableTree) tree; - rhsTree = varTree.getInitializer(); - } else { - AssignmentTree assignmentTree = (AssignmentTree) tree; - rhsTree = assignmentTree.getExpression(); - } - // rhsTree can be null for a VariableTree. Also, we don't need to do a check - // if rhsTree is the null literal - if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { - return; - } - Type rhsType = getTreeType(rhsTree, state); - - if (lhsType != null && rhsType != null) { - boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state); - if (!isAssignmentValid) { - reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); - } - } - } - public static void checkTypeParameterNullnessForAssignability( Tree tree, NullAway analysis, VisitorState state, Map> inferredTypes) { if (!analysis.getConfig().isJSpecifyMode()) { @@ -448,38 +419,33 @@ public static void checkTypeParameterNullnessForAssignability( } Type lhsType = getTreeType(tree, state); Tree rhsTree; -// VariableTree varTree = null; + // update inferredTypes cache for assignments if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); - // rhs is a method call + Tree lhsTree = varTree.getType(); + if(rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); - // generic method, no explicit generic arguments - if(methodSymbol.type instanceof Type.ForAll && methodInvocationTree.getTypeArguments().isEmpty()) { - Tree lhsTree = varTree.getType(); // Foo<@Nullable Object> - // lhs has generic types - if(lhsTree instanceof ParameterizedTypeTree) { - List lhsTypeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); // @Nullable Object - Type returnType = methodSymbol.getReturnType(); // com.uber.Test.Foo + // rhs is generic method call, no explicit generic arguments, lhs type has generic + if(methodSymbol.type instanceof Type.ForAll && methodInvocationTree.getTypeArguments().isEmpty() && lhsTree instanceof ParameterizedTypeTree) { + List lhsTypeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); // method call has a return type of class type - if(returnType instanceof Type.ClassType) { - Type.ClassType classType = (Type.ClassType) returnType; - List rhsTypeArguments = classType.getTypeArguments(); // "U" - // no explicit generic type given + if(methodSymbol.getReturnType() instanceof Type.ClassType) { + Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); + List rhsTypeArguments = returnType.getTypeArguments(); + // if generic type in return type if(!rhsTypeArguments.isEmpty()) { Map genericNullness = new HashMap<>(); for(int i=0; i @Nullable Object + genericNullness.put(rhsTypeArguments.get(i), lhsInferredType); } - inferredTypes.put(tree, genericNullness); + inferredTypes.put(rhsTree, genericNullness); } } - } } - } } else { AssignmentTree assignmentTree = (AssignmentTree) tree; @@ -988,36 +954,6 @@ private static Type substituteGenericTypeArgsToExplicit( * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not * invoke an instance method */ - public static Nullness getGenericParameterNullnessAtInvocation( - int paramIndex, - Symbol.MethodSymbol invokedMethodSymbol, - MethodInvocationTree tree, - VisitorState state, - Config config) { - // If generic method invocation - if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { - // Substitute the argument types within the MethodType - // NOTE: if explicitTypeArgs is empty, this is a noop - List substitutedParamTypes = - substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); - // If this condition evaluates to false, we fall through to the subsequent logic, to handle - // type variables declared on the enclosing class - if (substitutedParamTypes != null - && Objects.equals( - getTypeNullness(substitutedParamTypes.get(paramIndex), config), Nullness.NULLABLE)) { - return Nullness.NULLABLE; - } - } - - if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { - return Nullness.NONNULL; - } - Type enclosingType = - getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); - return getGenericMethodParameterNullness( - paramIndex, invokedMethodSymbol, enclosingType, state, config); - } - public static Nullness getGenericParameterNullnessAtInvocation( int paramIndex, Symbol.MethodSymbol invokedMethodSymbol, @@ -1039,24 +975,18 @@ public static Nullness getGenericParameterNullnessAtInvocation( return Nullness.NULLABLE; } // check nullness of inferred types - TreePath parentPath = state.getPath().getParentPath(); - Tree parentTree = parentPath.getLeaf(); - if(parentTree instanceof VariableTree && inferredTypes.containsKey(parentTree)) { - Map genericNullness = inferredTypes.get(parentTree); - List typeParam = invokedMethodSymbol.getTypeParameters(); -// for (int i = 0; i < typeParam.size(); i++) { - if (genericNullness.containsKey(typeParam.get(paramIndex).type)) { - var pType = typeParam.get(paramIndex).type; -// Tree inferred_tree = (Tree) inferredTypes.get(pType); - Type in_type = genericNullness.get(pType); - if (Objects.equals(getTypeNullness(in_type, config), Nullness.NULLABLE)) { + if(inferredTypes.containsKey(tree)) { // if in cache + Map genericNullness = inferredTypes.get(tree); + List typeParameters = invokedMethodSymbol.getTypeParameters(); // + if (genericNullness.containsKey(typeParameters.get(paramIndex).type)) { // get the inferred types + Type genericType = typeParameters.get(paramIndex).type; + Type inferredType = genericNullness.get(genericType); + if (Objects.equals(getTypeNullness(inferredType, config), Nullness.NULLABLE)) { return Nullness.NULLABLE; } else { return Nullness.NONNULL; } } - -// } } } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index a3fe39a56f..a85a681d1c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -171,7 +171,7 @@ public void genericInferenceOnAssignments() { " class Test {", " static class Foo {", " Foo(T t) {}", - " static Foo make(U u) {", // use return type for inference + " static Foo make(U u) {", " return new Foo<>(u);", " }", " }", @@ -182,8 +182,8 @@ public void genericInferenceOnAssignments() { " }", " }", " static void testLocalAssign() {", - " // legal", // [Foo.make(null) -> [U -> @Nullable Object, T -> …] ] ==> [Foo<@Nullable Object> f = Foo.make(null) -> [U -> @Nullable Object, T -> …] ] - " Foo<@Nullable Object> f = Foo.make(null);", // the context we need to use is that it is an assignment + " // legal", // [Foo.make(null) -> [U -> @Nullable Object, T -> …] ] + " Foo<@Nullable Object> f = Foo.make(null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f2 = Foo.make(null);", " // legal", @@ -196,6 +196,7 @@ public void genericInferenceOnAssignments() { } @Test + @Ignore("requires generic method support") public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { makeHelper() .addSourceLines( @@ -205,7 +206,7 @@ public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { " class Test {", " static class Foo {", " Foo(T t) {}", - " static Foo make(U u) {", // use return type for inference + " static Foo make(U u) {", " return new Foo<>(u);", " }", " }", From 453623ad7c6724e4f58948889f4c57ab808d2500 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 15 Jan 2025 14:10:05 -0800 Subject: [PATCH 16/55] formatting --- .../main/java/com/uber/nullaway/NullAway.java | 10 +- .../nullaway/generics/GenericsChecks.java | 95 ++++++++++--------- .../nullaway/jspecify/GenericMethodTests.java | 58 +++++------ 3 files changed, 88 insertions(+), 75 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index a8902ff70f..e506eec18c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -277,7 +277,8 @@ public Config getConfig() { private final Map computedNullnessMap = new LinkedHashMap<>(); private final Map> inferredTypes = new HashMap<>(); -// private final Map inferredTypes = new HashMap<>(); + + // private final Map inferredTypes = new HashMap<>(); /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the @@ -1850,7 +1851,12 @@ private Description handleInvocation( ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? GenericsChecks.getGenericParameterNullnessAtInvocation( - i, methodSymbol, (MethodInvocationTree) tree, state, config, inferredTypes) + i, + methodSymbol, + (MethodInvocationTree) tree, + state, + config, + inferredTypes) : Nullness.NONNULL); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 624e4f9717..748ecbed7f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -413,7 +413,7 @@ private static void reportInvalidOverridingMethodParamTypeError( * @param state the visitor state */ public static void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state, Map> inferredTypes) { + Tree tree, NullAway analysis, VisitorState state, Map> inferredTypes) { if (!analysis.getConfig().isJSpecifyMode()) { return; } @@ -425,26 +425,29 @@ public static void checkTypeParameterNullnessForAssignability( rhsTree = varTree.getInitializer(); Tree lhsTree = varTree.getType(); - if(rhsTree instanceof MethodInvocationTree) { + if (rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); // rhs is generic method call, no explicit generic arguments, lhs type has generic - if(methodSymbol.type instanceof Type.ForAll && methodInvocationTree.getTypeArguments().isEmpty() && lhsTree instanceof ParameterizedTypeTree) { - List lhsTypeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); - // method call has a return type of class type - if(methodSymbol.getReturnType() instanceof Type.ClassType) { - Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); - List rhsTypeArguments = returnType.getTypeArguments(); - // if generic type in return type - if(!rhsTypeArguments.isEmpty()) { - Map genericNullness = new HashMap<>(); - for(int i=0; i lhsTypeArguments = + ((ParameterizedTypeTree) lhsTree).getTypeArguments(); + // method call has a return type of class type + if (methodSymbol.getReturnType() instanceof Type.ClassType) { + Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); + List rhsTypeArguments = returnType.getTypeArguments(); + // if generic type in return type + if (!rhsTypeArguments.isEmpty()) { + Map genericNullness = new HashMap<>(); + for (int i = 0; i < rhsTypeArguments.size(); i++) { + Type lhsInferredType = ASTHelpers.getType(lhsTypeArguments.get(i)); + genericNullness.put(rhsTypeArguments.get(i), lhsInferredType); } + inferredTypes.put(rhsTree, genericNullness); } + } } } } else { @@ -458,30 +461,32 @@ public static void checkTypeParameterNullnessForAssignability( } Type rhsType = getTreeType(rhsTree, state); - if(rhsType != null && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types - if(rhsTree instanceof MethodInvocationTree) { - Symbol.MethodSymbol invokedMethodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); + if (rhsType != null + && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types + if (rhsTree instanceof MethodInvocationTree) { + Symbol.MethodSymbol invokedMethodSymbol = + ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); List typeParam = invokedMethodSymbol.getTypeParameters(); List newTypeArgument = new ArrayList<>(); - if(inferredTypes.containsKey(tree)) { + if (inferredTypes.containsKey(tree)) { Map genericNullness = inferredTypes.get(tree); for (int i = 0; i < typeParam.size(); i++) { if (genericNullness.containsKey(typeParam.get(i).type)) { var pType = typeParam.get(i).type; -// Tree inferred_tree = (Tree) inferredTypes.get(pType); + // Tree inferred_tree = (Tree) inferredTypes.get(pType); newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types } } } Type.ClassType classType = (Type.ClassType) rhsType; // create a new Type.ClassType that has inferred types - rhsType = new Type.ClassType( + rhsType = + new Type.ClassType( classType.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgument), - classType.tsym - ); + classType.tsym); } } @@ -955,38 +960,40 @@ private static Type substituteGenericTypeArgsToExplicit( * invoke an instance method */ public static Nullness getGenericParameterNullnessAtInvocation( - int paramIndex, - Symbol.MethodSymbol invokedMethodSymbol, - MethodInvocationTree tree, - VisitorState state, - Config config, - Map> inferredTypes) { + int paramIndex, + Symbol.MethodSymbol invokedMethodSymbol, + MethodInvocationTree tree, + VisitorState state, + Config config, + Map> inferredTypes) { // If generic method invocation if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { // Substitute the argument types within the MethodType // NOTE: if explicitTypeArgs is empty, this is a noop List substitutedParamTypes = - substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); + substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class if (substitutedParamTypes != null - && Objects.equals( + && Objects.equals( getTypeNullness(substitutedParamTypes.get(paramIndex), config), Nullness.NULLABLE)) { return Nullness.NULLABLE; } // check nullness of inferred types - if(inferredTypes.containsKey(tree)) { // if in cache + if (inferredTypes.containsKey(tree)) { // if in cache Map genericNullness = inferredTypes.get(tree); - List typeParameters = invokedMethodSymbol.getTypeParameters(); // - if (genericNullness.containsKey(typeParameters.get(paramIndex).type)) { // get the inferred types - Type genericType = typeParameters.get(paramIndex).type; - Type inferredType = genericNullness.get(genericType); - if (Objects.equals(getTypeNullness(inferredType, config), Nullness.NULLABLE)) { - return Nullness.NULLABLE; - } else { - return Nullness.NONNULL; - } + List typeParameters = + invokedMethodSymbol.getTypeParameters(); // + if (genericNullness.containsKey( + typeParameters.get(paramIndex).type)) { // get the inferred types + Type genericType = typeParameters.get(paramIndex).type; + Type inferredType = genericNullness.get(genericType); + if (Objects.equals(getTypeNullness(inferredType, config), Nullness.NULLABLE)) { + return Nullness.NULLABLE; + } else { + return Nullness.NONNULL; } + } } } @@ -994,9 +1001,9 @@ public static Nullness getGenericParameterNullnessAtInvocation( return Nullness.NONNULL; } Type enclosingType = - getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); return getGenericMethodParameterNullness( - paramIndex, invokedMethodSymbol, enclosingType, state, config); + paramIndex, invokedMethodSymbol, enclosingType, state, config); } /** diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index a85a681d1c..09e359b9a9 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -199,35 +199,35 @@ public void genericInferenceOnAssignments() { @Ignore("requires generic method support") public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - " class Test {", - " static class Foo {", - " Foo(T t) {}", - " static Foo make(U u) {", - " return new Foo<>(u);", - " }", - " }", - " static class Bar {", - " Bar(S s, Z z) {}", - " static Bar make(U a, B b) {", - " return new Bar<>(a, b);", - " }", - " }", - " static void testLocalAssign() {", - " // legal", - " var f1 = Foo.make(new String());", - " // legal: infers Foo<@Nullable Object>", - " var f2 = Foo.make(null);", - " // legal", - " var b1 = Bar.make(new String(), new Object());", - " var b2 = Bar.make(new String(), null);", - " var b3 = Bar.make(null, null);", - " }", - " }") - .doTest(); + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + " class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo make(U u) {", + " return new Foo<>(u);", + " }", + " }", + " static class Bar {", + " Bar(S s, Z z) {}", + " static Bar make(U a, B b) {", + " return new Bar<>(a, b);", + " }", + " }", + " static void testLocalAssign() {", + " // legal", + " var f1 = Foo.make(new String());", + " // legal: infers Foo<@Nullable Object>", + " var f2 = Foo.make(null);", + " // legal", + " var b1 = Bar.make(new String(), new Object());", + " var b2 = Bar.make(new String(), null);", + " var b3 = Bar.make(null, null);", + " }", + " }") + .doTest(); } private CompilationTestHelper makeHelper() { From db0b6ce913bac0014df7cae8dc42e45dee4c26e5 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 15 Jan 2025 14:49:13 -0800 Subject: [PATCH 17/55] minor changes --- .../main/java/com/uber/nullaway/NullAway.java | 2 -- .../nullaway/generics/GenericsChecks.java | 8 ++--- .../nullaway/jspecify/GenericMethodTests.java | 35 ------------------- 3 files changed, 3 insertions(+), 42 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 7d0cc143cd..dd79d41a9d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -280,8 +280,6 @@ public Config getConfig() { private final Map> inferredTypes = new HashMap<>(); - // private final Map inferredTypes = new HashMap<>(); - /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the * constructor taking an ErrorProneFlags object. This constructor should not be used anywhere diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index d3e323403f..991be102f9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -475,7 +475,6 @@ public static void checkTypeParameterNullnessForAssignability( for (int i = 0; i < typeParam.size(); i++) { if (genericNullness.containsKey(typeParam.get(i).type)) { var pType = typeParam.get(i).type; - // Tree inferred_tree = (Tree) inferredTypes.get(pType); newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types } } @@ -995,13 +994,12 @@ public static Nullness getGenericParameterNullnessAtInvocation( // check nullness of inferred types if (inferredTypes.containsKey(tree)) { // if in cache Map genericNullness = inferredTypes.get(tree); - List typeParameters = - invokedMethodSymbol.getTypeParameters(); // + List typeParameters = invokedMethodSymbol.getTypeParameters(); if (genericNullness.containsKey( typeParameters.get(paramIndex).type)) { // get the inferred types Type genericType = typeParameters.get(paramIndex).type; - Type inferredType = genericNullness.get(genericType); - if (Objects.equals(getTypeNullness(inferredType, config), Nullness.NULLABLE)) { + Type inferredGenericType = genericNullness.get(genericType); + if (Objects.equals(getTypeNullness(inferredGenericType, config), Nullness.NULLABLE)) { return Nullness.NULLABLE; } else { return Nullness.NONNULL; diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 68f16b9e94..92ac80509e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -195,41 +195,6 @@ public void genericInferenceOnAssignments() { .doTest(); } - @Test - @Ignore("requires generic method support") - public void genericInferenceOnAssignmentsWithLocalVarTypeInference() { - makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - " class Test {", - " static class Foo {", - " Foo(T t) {}", - " static Foo make(U u) {", - " return new Foo<>(u);", - " }", - " }", - " static class Bar {", - " Bar(S s, Z z) {}", - " static Bar make(U a, B b) {", - " return new Bar<>(a, b);", - " }", - " }", - " static void testLocalAssign() {", - " // legal", - " var f1 = Foo.make(new String());", - " // legal: infers Foo<@Nullable Object>", - " var f2 = Foo.make(null);", - " // legal", - " var b1 = Bar.make(new String(), new Object());", - " var b2 = Bar.make(new String(), null);", - " var b3 = Bar.make(null, null);", - " }", - " }") - .doTest(); - } - @Test public void issue1035() { makeHelper() From 35b18cff5c03fd56f64f6400dacc30083a7e5403 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 15 Jan 2025 15:45:03 -0800 Subject: [PATCH 18/55] move cache into GenericsChecks and make related methods nonstatic --- .../src/main/java/com/uber/nullaway/NullAway.java | 12 ++++++------ .../com/uber/nullaway/generics/GenericsChecks.java | 13 +++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index dd79d41a9d..ad031fdf73 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -278,7 +278,8 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); - private final Map> inferredTypes = new HashMap<>(); +// private final Map> inferredTypes = new HashMap<>(); + private GenericsChecks genericsChecks = new GenericsChecks(); /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the @@ -503,7 +504,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } // generics check if (lhsType != null && config.isJSpecifyMode()) { - GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, inferredTypes); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { @@ -1497,7 +1498,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } VarSymbol symbol = ASTHelpers.getSymbol(tree); if (tree.getInitializer() != null && config.isJSpecifyMode()) { - GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, inferredTypes); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } if (!config.isLegacyAnnotationLocation()) { checkNullableAnnotationPositionInType( @@ -1883,13 +1884,12 @@ private Description handleInvocation( Nullness.paramHasNullableAnnotation(methodSymbol, i, config) ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) - ? GenericsChecks.getGenericParameterNullnessAtInvocation( + ? genericsChecks.getGenericParameterNullnessAtInvocation( i, methodSymbol, (MethodInvocationTree) tree, state, - config, - inferredTypes) + config) : Nullness.NONNULL); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 991be102f9..87a1a0b387 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -55,8 +55,10 @@ public final class GenericsChecks { static final Supplier JSPECIFY_NULLABLE_TYPE_SUPPLIER = Suppliers.typeFromString("org.jspecify.annotations.Nullable"); + private final Map> inferredTypes = new HashMap<>(); + /** Do not instantiate; all methods should be static */ - private GenericsChecks() {} + public GenericsChecks() {} /** * Checks that for an instantiated generic type, {@code @Nullable} types are only used for type @@ -412,8 +414,8 @@ private static void reportInvalidOverridingMethodParamTypeError( * @param analysis the analysis object * @param state the visitor state */ - public static void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state, Map> inferredTypes) { + public void checkTypeParameterNullnessForAssignability( + Tree tree, NullAway analysis, VisitorState state) { if (!analysis.getConfig().isJSpecifyMode()) { return; } @@ -970,13 +972,12 @@ private static Type substituteTypeArgsInGenericMethodType( * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not * invoke an instance method */ - public static Nullness getGenericParameterNullnessAtInvocation( + public Nullness getGenericParameterNullnessAtInvocation( int paramIndex, Symbol.MethodSymbol invokedMethodSymbol, MethodInvocationTree tree, VisitorState state, - Config config, - Map> inferredTypes) { + Config config) { // If generic method invocation if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { // Substitute the argument types within the MethodType From f5ece942f462115b4f5b4d284650f2e321d14a4b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 26 Jan 2025 13:11:22 -0800 Subject: [PATCH 19/55] formatting --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index ad031fdf73..cd593db14b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -107,7 +107,6 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Target; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -278,7 +277,7 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); -// private final Map> inferredTypes = new HashMap<>(); + // private final Map> inferredTypes = new HashMap<>(); private GenericsChecks genericsChecks = new GenericsChecks(); /** @@ -1885,11 +1884,7 @@ private Description handleInvocation( ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? genericsChecks.getGenericParameterNullnessAtInvocation( - i, - methodSymbol, - (MethodInvocationTree) tree, - state, - config) + i, methodSymbol, (MethodInvocationTree) tree, state, config) : Nullness.NONNULL); } } From f89fef03ddf09846969ad2df44df8e3d91c55d96 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Tue, 28 Jan 2025 13:23:16 -0800 Subject: [PATCH 20/55] fix CI Job failure # Conflicts: # nullaway/src/main/java/com/uber/nullaway/NullAway.java --- .../nullaway/generics/GenericsChecks.java | 12 ++-- .../nullaway/jspecify/GenericMethodTests.java | 62 +++++++++++++++---- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 87a1a0b387..6fdb51bedc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -993,14 +993,14 @@ public Nullness getGenericParameterNullnessAtInvocation( return Nullness.NULLABLE; } // check nullness of inferred types - if (inferredTypes.containsKey(tree)) { // if in cache + if (inferredTypes.containsKey(tree)) { Map genericNullness = inferredTypes.get(tree); - List typeParameters = invokedMethodSymbol.getTypeParameters(); - if (genericNullness.containsKey( - typeParameters.get(paramIndex).type)) { // get the inferred types - Type genericType = typeParameters.get(paramIndex).type; + List parameters = invokedMethodSymbol.getParameters(); + if (genericNullness.containsKey(parameters.get(paramIndex).type)) { + Type genericType = parameters.get(paramIndex).type; Type inferredGenericType = genericNullness.get(genericType); - if (Objects.equals(getTypeNullness(inferredGenericType, config), Nullness.NULLABLE)) { + if (inferredGenericType != null + && Objects.equals(getTypeNullness(inferredGenericType, config), Nullness.NULLABLE)) { return Nullness.NULLABLE; } else { return Nullness.NONNULL; diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 92ac80509e..a02f26ee67 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -175,26 +175,66 @@ public void genericInferenceOnAssignments() { " return new Foo<>(u);", " }", " }", - " static class Bar {", - " Bar(S s, Z z) {}", - " static Bar make(U a, B b) {", - " return new Bar<>(a, b);", - " }", - " }", " static void testLocalAssign() {", - " // legal", // [Foo.make(null) -> [U -> @Nullable Object, T -> …] ] + " // legal", " Foo<@Nullable Object> f = Foo.make(null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f2 = Foo.make(null);", - " // legal", - " Bar<@Nullable Object, Object> b = Bar.make(null, new Object());", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", " }", " }") .doTest(); } + @Test + public void genericInferenceOnAssignmentsMultipleParams() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class Foo {", + " Foo(T t) {}", + " public Foo make(U u, @Nullable String s) {", + " return new Foo<>(u);", + " }", + " }", + " static class Bar {", + " Bar(S s, Z z) {}", + " static Bar make(U u, B b) {", + " return new Bar<>(u, b);", + " }", + " }", + " static class Baz {", + " Baz(S s, Z z) {}", + " static Baz make(U u, B b) {", + " return new Baz<>(u, b);", + " }", + " }", + " public void run(Foo<@Nullable String> foo) {", + " // legal", + " Foo<@Nullable Object> f1 = foo.make(null, new String());", + " Foo<@Nullable Object> f2 = foo.make(null, null);", + " Foo f3 = foo.make(new Object(), null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo f4 = foo.make(null, null);", + " // legal", + " Bar<@Nullable Object, Object> b1 = Bar.make(null, new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", + " // legal", + " Baz baz1 = Baz.make(new String(), new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Baz baz2 = Baz.make(null, new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Baz baz3 = Baz.make(new String(), null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Baz baz4 = Baz.make(null, null);", + " }", + "}") + .doTest(); + } + @Test public void issue1035() { makeHelper() From 0c577aef1ed78a0e523598f78b990ebc764d5530 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Tue, 28 Jan 2025 13:30:23 -0800 Subject: [PATCH 21/55] remove unused code --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 1 - 1 file changed, 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index cd593db14b..f05042a527 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -277,7 +277,6 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); - // private final Map> inferredTypes = new HashMap<>(); private GenericsChecks genericsChecks = new GenericsChecks(); /** From 61e4f3e78f85b6996bb4206cb52b18e859fad7b0 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 3 Feb 2025 12:14:39 -0800 Subject: [PATCH 22/55] debugged for testJdk23 --- .../nullaway/generics/GenericsChecks.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 6fdb51bedc..ae70a88b6c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -472,22 +472,23 @@ public void checkTypeParameterNullnessForAssignability( List typeParam = invokedMethodSymbol.getTypeParameters(); List newTypeArgument = new ArrayList<>(); - if (inferredTypes.containsKey(tree)) { - Map genericNullness = inferredTypes.get(tree); + if (inferredTypes.containsKey(rhsTree)) { + Map genericNullness = inferredTypes.get(rhsTree); for (int i = 0; i < typeParam.size(); i++) { if (genericNullness.containsKey(typeParam.get(i).type)) { var pType = typeParam.get(i).type; newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types } } + + Type.ClassType classType = (Type.ClassType) rhsType; + // create a new Type.ClassType that has inferred types + rhsType = + new Type.ClassType( + classType.getEnclosingType(), + com.sun.tools.javac.util.List.from(newTypeArgument), + classType.tsym); } - Type.ClassType classType = (Type.ClassType) rhsType; - // create a new Type.ClassType that has inferred types - rhsType = - new Type.ClassType( - classType.getEnclosingType(), - com.sun.tools.javac.util.List.from(newTypeArgument), - classType.tsym); } } From 9f6c91b94f3c584d3cc2ac5347a81bc84a183431 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 3 Feb 2025 12:20:00 -0800 Subject: [PATCH 23/55] fetch and merge --- .../java/com/uber/nullaway/generics/GenericsChecks.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index ae70a88b6c..73151888f2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -484,10 +484,10 @@ public void checkTypeParameterNullnessForAssignability( Type.ClassType classType = (Type.ClassType) rhsType; // create a new Type.ClassType that has inferred types rhsType = - new Type.ClassType( - classType.getEnclosingType(), - com.sun.tools.javac.util.List.from(newTypeArgument), - classType.tsym); + new Type.ClassType( + classType.getEnclosingType(), + com.sun.tools.javac.util.List.from(newTypeArgument), + classType.tsym); } } } From 4f4ca895099cd357b6bf213a93d293877fb56d5c Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 3 Feb 2025 17:42:37 -0800 Subject: [PATCH 24/55] recreated :caffeine:compileTestJava error --- .../nullaway/jspecify/GenericMethodTests.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 9b1c3264e1..8574165dd8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -235,6 +235,24 @@ public void genericInferenceOnAssignmentsMultipleParams() { .doTest(); } + @Test + public void genericsUsedForGenericClasses() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.ArrayList;", + "class Test {", + " abstract class Foo {", + " abstract Foo> asFoo();", + " }", + " static void test(Foo f) {", + " Foo> foo = f.asFoo();", + " }", + "}") + .doTest(); + } + @Test public void issue1035() { makeHelper() From d96769a2db286cc1ee12733c5d0784d74cabae6a Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 3 Feb 2025 22:54:34 -0800 Subject: [PATCH 25/55] update test case --- .../java/com/uber/nullaway/jspecify/GenericMethodTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 8574165dd8..1f8943abd7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -241,13 +241,15 @@ public void genericsUsedForGenericClasses() { .addSourceLines( "Test.java", "package com.uber;", + "import org.jspecify.annotations.Nullable;", "import java.util.ArrayList;", "class Test {", " abstract class Foo {", - " abstract Foo> asFoo();", + " abstract Foo> asFoo();", " }", " static void test(Foo f) {", " Foo> foo = f.asFoo();", + " Foo> foo = f.asFoo();", " }", "}") .doTest(); From b3dda68a703522d3769ecc0b64619d8798f4dea7 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Tue, 4 Feb 2025 22:17:33 -0800 Subject: [PATCH 26/55] add scenarios to test cases --- .../nullaway/jspecify/GenericMethodTests.java | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 1f8943abd7..5fa6fe44f5 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -171,15 +171,22 @@ public void genericInferenceOnAssignments() { " class Test {", " static class Foo {", " Foo(T t) {}", - " static Foo make(U u) {", + " static Foo makeNull(U u) {", + " return new Foo<>(u);", + " }", + " static Foo makeNonNull(U u) {", " return new Foo<>(u);", " }", " }", " static void testLocalAssign() {", " // legal", - " Foo<@Nullable Object> f = Foo.make(null);", + " Foo<@Nullable Object> f1 = Foo.makeNull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo f2 = Foo.makeNull(null);", + " // ILLEGAL: U does not have a @Nullable upper bound", + " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " Foo f2 = Foo.make(null);", + " Foo f4 = Foo.makeNonNull(null);", " }", " }") .doTest(); @@ -245,11 +252,14 @@ public void genericsUsedForGenericClasses() { "import java.util.ArrayList;", "class Test {", " abstract class Foo {", - " abstract Foo> asFoo();", + " abstract Foo> nonNullTest();", + " abstract Foo> nullTest();", " }", " static void test(Foo f) {", - " Foo> foo = f.asFoo();", - " Foo> foo = f.asFoo();", + " Foo> fooNonNull_1 = f.nonNullTest();", + " Foo> fooNonNull_2 = f.nonNullTest();", // error message + " Foo> fooNull_1 = f.nullTest();", + " Foo> fooNull_2 = f.nullTest();", // error message " }", "}") .doTest(); From 5f1e08fcf1bd1c05981c62d03b35414733f141d8 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 5 Feb 2025 16:49:48 -0800 Subject: [PATCH 27/55] add expected errors for testcase --- .../test/java/com/uber/nullaway/jspecify/GenericMethodTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 5fa6fe44f5..f9b541b465 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -184,6 +184,7 @@ public void genericInferenceOnAssignments() { " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f2 = Foo.makeNull(null);", " // ILLEGAL: U does not have a @Nullable upper bound", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo f4 = Foo.makeNonNull(null);", From 1e9699a10593cbcca2c6e4b44f85e9f98adf15b7 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Thu, 6 Feb 2025 10:20:47 -0800 Subject: [PATCH 28/55] add upper bound checks --- .../nullaway/generics/GenericsChecks.java | 51 ++++++++++++++++--- .../nullaway/jspecify/GenericMethodTests.java | 19 +++---- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 75ff8f7671..68ceca9827 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2,6 +2,7 @@ import static com.google.common.base.Verify.verify; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static com.uber.nullaway.NullabilityUtil.findEnclosingMethodOrLambdaOrInitializer; import com.google.errorprone.VisitorState; import com.google.errorprone.suppliers.Supplier; @@ -40,6 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import javax.lang.model.type.ExecutableType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeVariable; @@ -426,7 +428,7 @@ private static void reportInvalidOverridingMethodParamTypeError( * @param state the visitor state */ public void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state) { + Tree tree, NullAway analysis, VisitorState state, Config config) { if (!analysis.getConfig().isJSpecifyMode()) { return; } @@ -450,13 +452,20 @@ public void checkTypeParameterNullnessForAssignability( // method call has a return type of class type if (methodSymbol.getReturnType() instanceof Type.ClassType) { Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); - List rhsTypeArguments = returnType.getTypeArguments(); + List typeParam = methodSymbol.getTypeParameters(); + List returnTypeTypeArg = returnType.getTypeArguments(); + // if generic type in return type - if (!rhsTypeArguments.isEmpty()) { + if (!typeParam.isEmpty()) { Map genericNullness = new HashMap<>(); - for (int i = 0; i < rhsTypeArguments.size(); i++) { - Type lhsInferredType = ASTHelpers.getType(lhsTypeArguments.get(i)); - genericNullness.put(rhsTypeArguments.get(i), lhsInferredType); + for (int i = 0; i < typeParam.size(); i++) { + Type upperBound = typeParam.get(i).type.getUpperBound(); + if(getTypeNullness(upperBound, config) == Nullness.NULLABLE) { // generic has nullable upperbound + Type lhsInferredType = inferMethodTypeArgument(typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); + if (lhsInferredType != null) { // && has a nullable upperbound + genericNullness.put(typeParam.get(i).type, lhsInferredType); + } + } } inferredTypes.put(rhsTree, genericNullness); } @@ -485,10 +494,13 @@ public void checkTypeParameterNullnessForAssignability( if (inferredTypes.containsKey(rhsTree)) { Map genericNullness = inferredTypes.get(rhsTree); + List parameterTypes = rhsType.getTypeArguments(); for (int i = 0; i < typeParam.size(); i++) { - if (genericNullness.containsKey(typeParam.get(i).type)) { - var pType = typeParam.get(i).type; + Type pType = typeParam.get(i).type; + if (genericNullness.containsKey(pType)) { newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types + } else { + newTypeArgument.add(parameterTypes.get(i)); } } @@ -511,6 +523,29 @@ public void checkTypeParameterNullnessForAssignability( } } + private Type inferMethodTypeArgument(Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { + // base case + if(typeParam == null || lhsTypeArg == null || typeArg == null) { + return null; + } + + // recursive case + Type inferType = null; + for(int i=0; i maybe the base case makes it unnecessary + inferType = inferMethodTypeArgument(typeParam, ((ParameterizedTypeTree) lhsTypeArg.get(i)).getTypeArguments(), typeArg.get(i).getTypeArguments(), state); + if(inferType != null) { + return inferType; + } + } + } + return inferType; + } + /** * Checks that the nullability of type parameters for a returned expression matches that of the * type parameters of the enclosing method's return type. diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index f9b541b465..27b030f098 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -179,15 +179,16 @@ public void genericInferenceOnAssignments() { " }", " }", " static void testLocalAssign() {", - " // legal", - " Foo<@Nullable Object> f1 = Foo.makeNull(null);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " Foo f2 = Foo.makeNull(null);", - " // ILLEGAL: U does not have a @Nullable upper bound", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " Foo f4 = Foo.makeNonNull(null);", +// " // legal", +// " Foo<@Nullable Object> f1 = Foo.makeNull(null);", +// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", +// " Foo f2 = Foo.makeNull(null);", +// " // ILLEGAL: U does not have a @Nullable upper bound", +// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", +// " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", +// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", +// " Foo f4 = Foo.makeNonNull(null);", + " Foo<@Nullable Object> f5 = Foo.makeNonNull(new Object());", " }", " }") .doTest(); From a2489ea1bf74863f4383205335a2c2294ad87dc4 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Thu, 6 Feb 2025 10:24:57 -0800 Subject: [PATCH 29/55] update NullAway.java --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index f05042a527..e338d63d3d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -502,7 +502,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } // generics check if (lhsType != null && config.isJSpecifyMode()) { - genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, config); } if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { @@ -1496,7 +1496,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } VarSymbol symbol = ASTHelpers.getSymbol(tree); if (tree.getInitializer() != null && config.isJSpecifyMode()) { - genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, config); } if (!config.isLegacyAnnotationLocation()) { checkNullableAnnotationPositionInType( From ddf244085de9e820c355d0e94d879bfc99f01006 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 7 Feb 2025 15:02:52 -0800 Subject: [PATCH 30/55] formatting --- .../nullaway/generics/GenericsChecks.java | 34 ++++++----- .../nullaway/jspecify/GenericMethodTests.java | 58 ++++++++++--------- 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 68ceca9827..9aa24140ab 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2,7 +2,6 @@ import static com.google.common.base.Verify.verify; import static com.uber.nullaway.NullabilityUtil.castToNonNull; -import static com.uber.nullaway.NullabilityUtil.findEnclosingMethodOrLambdaOrInitializer; import com.google.errorprone.VisitorState; import com.google.errorprone.suppliers.Supplier; @@ -41,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; import javax.lang.model.type.ExecutableType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeVariable; @@ -452,7 +450,7 @@ public void checkTypeParameterNullnessForAssignability( // method call has a return type of class type if (methodSymbol.getReturnType() instanceof Type.ClassType) { Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); - List typeParam = methodSymbol.getTypeParameters(); + List typeParam = methodSymbol.getTypeParameters(); List returnTypeTypeArg = returnType.getTypeArguments(); // if generic type in return type @@ -460,8 +458,11 @@ public void checkTypeParameterNullnessForAssignability( Map genericNullness = new HashMap<>(); for (int i = 0; i < typeParam.size(); i++) { Type upperBound = typeParam.get(i).type.getUpperBound(); - if(getTypeNullness(upperBound, config) == Nullness.NULLABLE) { // generic has nullable upperbound - Type lhsInferredType = inferMethodTypeArgument(typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); + if (getTypeNullness(upperBound, config) + == Nullness.NULLABLE) { // generic has nullable upperbound + Type lhsInferredType = + inferMethodTypeArgument( + typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); if (lhsInferredType != null) { // && has a nullable upperbound genericNullness.put(typeParam.get(i).type, lhsInferredType); } @@ -523,22 +524,29 @@ public void checkTypeParameterNullnessForAssignability( } } - private Type inferMethodTypeArgument(Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { + private Type inferMethodTypeArgument( + Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { // base case - if(typeParam == null || lhsTypeArg == null || typeArg == null) { + if (typeParam == null || lhsTypeArg == null || typeArg == null) { return null; } // recursive case Type inferType = null; - for(int i=0; i maybe the base case makes it unnecessary - inferType = inferMethodTypeArgument(typeParam, ((ParameterizedTypeTree) lhsTypeArg.get(i)).getTypeArguments(), typeArg.get(i).getTypeArguments(), state); - if(inferType != null) { + } else if (!type.getTypeArguments().isEmpty()) { + // instanceof Type.ForAll TODO: check if the lhsTypeArg is a generic class? -> maybe + // the base case makes it unnecessary + inferType = + inferMethodTypeArgument( + typeParam, + ((ParameterizedTypeTree) lhsTypeArg.get(i)).getTypeArguments(), + typeArg.get(i).getTypeArguments(), + state); + if (inferType != null) { return inferType; } } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 27b030f098..cab5ff8b47 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -179,15 +179,18 @@ public void genericInferenceOnAssignments() { " }", " }", " static void testLocalAssign() {", -// " // legal", -// " Foo<@Nullable Object> f1 = Foo.makeNull(null);", -// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", -// " Foo f2 = Foo.makeNull(null);", -// " // ILLEGAL: U does not have a @Nullable upper bound", -// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", -// " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", -// " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", -// " Foo f4 = Foo.makeNonNull(null);", + // " // legal", + // " Foo<@Nullable Object> f1 = Foo.makeNull(null);", + // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' + // where @NonNull is required", + // " Foo f2 = Foo.makeNull(null);", + // " // ILLEGAL: U does not have a @Nullable upper bound", + // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' + // where @NonNull is required", + // " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", + // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' + // where @NonNull is required", + // " Foo f4 = Foo.makeNonNull(null);", " Foo<@Nullable Object> f5 = Foo.makeNonNull(new Object());", " }", " }") @@ -247,24 +250,25 @@ public void genericInferenceOnAssignmentsMultipleParams() { @Test public void genericsUsedForGenericClasses() { makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "import java.util.ArrayList;", - "class Test {", - " abstract class Foo {", - " abstract Foo> nonNullTest();", - " abstract Foo> nullTest();", - " }", - " static void test(Foo f) {", - " Foo> fooNonNull_1 = f.nonNullTest();", - " Foo> fooNonNull_2 = f.nonNullTest();", // error message - " Foo> fooNull_1 = f.nullTest();", - " Foo> fooNull_2 = f.nullTest();", // error message - " }", - "}") - .doTest(); + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.ArrayList;", + "class Test {", + " abstract class Foo {", + " abstract Foo> nonNullTest();", + " abstract Foo> nullTest();", + " }", + " static void test(Foo f) {", + " Foo> fooNonNull_1 = f.nonNullTest();", + " Foo> fooNonNull_2 = f.nonNullTest();", // error message + " Foo> fooNull_1 = f.nullTest();", + " Foo> fooNull_2 = f.nullTest();", // error + // message + " }", + "}") + .doTest(); } @Test From 24241b5a012190a4ef44cf34c7716c83681e47fe Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 7 Feb 2025 15:14:52 -0800 Subject: [PATCH 31/55] remove unnecessary parameter --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 4 ++-- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e338d63d3d..f05042a527 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -502,7 +502,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } // generics check if (lhsType != null && config.isJSpecifyMode()) { - genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, config); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } if (config.isJSpecifyMode() && tree.getVariable() instanceof ArrayAccessTree) { @@ -1496,7 +1496,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } VarSymbol symbol = ASTHelpers.getSymbol(tree); if (tree.getInitializer() != null && config.isJSpecifyMode()) { - genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state, config); + genericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } if (!config.isLegacyAnnotationLocation()) { checkNullableAnnotationPositionInType( diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 9aa24140ab..69589d5de2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -426,7 +426,7 @@ private static void reportInvalidOverridingMethodParamTypeError( * @param state the visitor state */ public void checkTypeParameterNullnessForAssignability( - Tree tree, NullAway analysis, VisitorState state, Config config) { + Tree tree, NullAway analysis, VisitorState state) { if (!analysis.getConfig().isJSpecifyMode()) { return; } @@ -458,7 +458,7 @@ public void checkTypeParameterNullnessForAssignability( Map genericNullness = new HashMap<>(); for (int i = 0; i < typeParam.size(); i++) { Type upperBound = typeParam.get(i).type.getUpperBound(); - if (getTypeNullness(upperBound, config) + if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { // generic has nullable upperbound Type lhsInferredType = inferMethodTypeArgument( From e07c0205dfc640afc7c425cf51f58de2df314155 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sun, 9 Feb 2025 23:25:29 -0800 Subject: [PATCH 32/55] update test cases --- .../nullaway/jspecify/GenericMethodTests.java | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index cab5ff8b47..ec137617cb 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -179,19 +179,20 @@ public void genericInferenceOnAssignments() { " }", " }", " static void testLocalAssign() {", - // " // legal", - // " Foo<@Nullable Object> f1 = Foo.makeNull(null);", - // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' - // where @NonNull is required", - // " Foo f2 = Foo.makeNull(null);", - // " // ILLEGAL: U does not have a @Nullable upper bound", - // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' - // where @NonNull is required", - // " Foo<@Nullable Object> f3 = Foo.makeNonNull(null);", - // " // BUG: Diagnostic contains: passing @Nullable parameter 'null' - // where @NonNull is required", - // " Foo f4 = Foo.makeNonNull(null);", - " Foo<@Nullable Object> f5 = Foo.makeNonNull(new Object());", + " // legal", + " Foo<@Nullable Object> f1 = Foo.makeNull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo f2 = Foo.makeNull(null);", + " Foo<@Nullable Object> f3 = Foo.makeNull(new Object());", + " Foo f4 = Foo.makeNull(new Object());", + " // ILLEGAL: U does not have a @Nullable upper bound", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo<@Nullable Object> f5 = Foo.makeNonNull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo f6 = Foo.makeNonNull(null);", + " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", + " Foo<@Nullable Object> f7 = Foo.makeNonNull(new Object());", + " Foo f8 = Foo.makeNonNull(new Object());", " }", " }") .doTest(); @@ -234,6 +235,12 @@ public void genericInferenceOnAssignmentsMultipleParams() { " Bar<@Nullable Object, Object> b1 = Bar.make(null, new Object());", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Bar<@Nullable Object, Object> b2 = Bar.make(null, null);", + " Bar<@Nullable Object, @Nullable Object> b3 = Bar.make(null, null);", + " Bar b4 = Bar.make(new Object(), null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Bar b5 = Bar.make(null, null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Bar b6 = Bar.make(null, null);", " // legal", " Baz baz1 = Baz.make(new String(), new Object());", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", @@ -242,6 +249,10 @@ public void genericInferenceOnAssignmentsMultipleParams() { " Baz baz3 = Baz.make(new String(), null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Baz baz4 = Baz.make(null, null);", + " // BUG: Diagnostic contains: Generic type parameter cannot be @Nullable", + " Baz<@Nullable String, Object> baz5 = Baz.make(new String(), new Object());", + " // BUG: Diagnostic contains: Generic type parameter cannot be @Nullable", + " Baz baz6 = Baz.make(new String(), new Object());", " }", "}") .doTest(); @@ -256,16 +267,19 @@ public void genericsUsedForGenericClasses() { "import org.jspecify.annotations.Nullable;", "import java.util.ArrayList;", "class Test {", - " abstract class Foo {", + " abstract class Foo {", " abstract Foo> nonNullTest();", " abstract Foo> nullTest();", " }", " static void test(Foo f) {", " Foo> fooNonNull_1 = f.nonNullTest();", - " Foo> fooNonNull_2 = f.nonNullTest();", // error message + " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", + " Foo> fooNonNull_2 = f.nonNullTest();", + " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", + " Foo<@Nullable Integer, ArrayList> fooNonNull_3 = f.nonNullTest();", " Foo> fooNull_1 = f.nullTest();", - " Foo> fooNull_2 = f.nullTest();", // error - // message + " Foo> fooNull_2 = f.nullTest();", + " Foo<@Nullable Integer, ArrayList> fooNull_3 = f.nullTest();", " }", "}") .doTest(); From e15754a3022e6fcf5f1f3ab9ead9bc17b276cb81 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sun, 9 Feb 2025 23:33:36 -0800 Subject: [PATCH 33/55] version passing all tests --- .../nullaway/generics/GenericsChecks.java | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index d71e9c0897..09daeff946 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -432,10 +432,9 @@ public void checkTypeParameterNullnessForAssignability( if (rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); - // rhs is generic method call, no explicit generic arguments, lhs type has generic - if (methodSymbol.type instanceof Type.ForAll - && methodInvocationTree.getTypeArguments().isEmpty() - && lhsTree instanceof ParameterizedTypeTree) { + if (methodSymbol.type instanceof Type.ForAll // generic method call + && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments + && lhsTree instanceof ParameterizedTypeTree) { // lhs type has generic List lhsTypeArguments = ((ParameterizedTypeTree) lhsTree).getTypeArguments(); // method call has a return type of class type @@ -449,8 +448,8 @@ public void checkTypeParameterNullnessForAssignability( Map genericNullness = new HashMap<>(); for (int i = 0; i < typeParam.size(); i++) { Type upperBound = typeParam.get(i).type.getUpperBound(); - if (getTypeNullness(upperBound, analysis.getConfig()) - == Nullness.NULLABLE) { // generic has nullable upperbound + // generic has nullable upperbound + if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { Type lhsInferredType = inferMethodTypeArgument( typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); @@ -475,34 +474,13 @@ public void checkTypeParameterNullnessForAssignability( } Type rhsType = getTreeType(rhsTree, config); - if (rhsType != null - && !rhsType.getTypeArguments().isEmpty()) { // only when we have inferred types + if (rhsType != null && !rhsType.getTypeArguments().isEmpty()) { if (rhsTree instanceof MethodInvocationTree) { - Symbol.MethodSymbol invokedMethodSymbol = - ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); - - List typeParam = invokedMethodSymbol.getTypeParameters(); - List newTypeArgument = new ArrayList<>(); - + // recreate rhsType using inferred types + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); if (inferredTypes.containsKey(rhsTree)) { Map genericNullness = inferredTypes.get(rhsTree); - List parameterTypes = rhsType.getTypeArguments(); - for (int i = 0; i < typeParam.size(); i++) { - Type pType = typeParam.get(i).type; - if (genericNullness.containsKey(pType)) { - newTypeArgument.add(genericNullness.get(pType)); // replace type to inferred types - } else { - newTypeArgument.add(parameterTypes.get(i)); - } - } - - Type.ClassType classType = (Type.ClassType) rhsType; - // create a new Type.ClassType that has inferred types - rhsType = - new Type.ClassType( - classType.getEnclosingType(), - com.sun.tools.javac.util.List.from(newTypeArgument), - classType.tsym); + rhsType = replaceGenerics(rhsType, methodSymbol.getReturnType(), genericNullness); } } } @@ -529,7 +507,7 @@ private Type inferMethodTypeArgument( if (state.getTypes().isSameType(typeParam, type)) { return ASTHelpers.getType(lhsTypeArg.get(i)); } else if (!type.getTypeArguments().isEmpty()) { - // instanceof Type.ForAll TODO: check if the lhsTypeArg is a generic class? -> maybe + // instanceof Type.ForAll check if the lhsTypeArg is a generic class? -> maybe // the base case makes it unnecessary inferType = inferMethodTypeArgument( @@ -545,6 +523,28 @@ private Type inferMethodTypeArgument( return inferType; } + private Type replaceGenerics( + Type currentType, Type typeWithGenerics, Map genericNullness) { + // base case + if (genericNullness.containsKey(typeWithGenerics)) { // type is generic + return genericNullness.get(typeWithGenerics); + } + if (typeWithGenerics.getTypeArguments().isEmpty()) { // dose not have a generic type argument + return currentType; + } + + // recursive case + List newTypeArgument = new ArrayList<>(); + for (int i = 0; i < typeWithGenerics.getTypeArguments().size(); i++) { + Type newType = replaceGenerics(currentType.getTypeArguments().get(i), typeWithGenerics.getTypeArguments().get(i), genericNullness); + newTypeArgument.add(newType); + } + + Type.ClassType curClassType = (Type.ClassType) currentType; + Type.ClassType updatedType = new Type.ClassType(curClassType.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgument), curClassType.tsym); + return updatedType; + } + /** * Checks that the nullability of type parameters for a returned expression matches that of the * type parameters of the enclosing method's return type. From d5c4e5fc1f073f64fccffeb5393834a783d7cf8a Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sun, 9 Feb 2025 23:47:11 -0800 Subject: [PATCH 34/55] change according to comments --- .../nullaway/generics/GenericsChecks.java | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 09daeff946..48635aa287 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -48,9 +48,6 @@ public final class GenericsChecks { private final Map> inferredTypes = new HashMap<>(); - /** Do not instantiate; all methods should be static */ - public GenericsChecks() {} - /** * Checks that for an instantiated generic type, {@code @Nullable} types are only used for type * variables that have a {@code @Nullable} upper bound. @@ -427,16 +424,16 @@ public void checkTypeParameterNullnessForAssignability( if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); - Tree lhsTree = varTree.getType(); + Tree lhsTypeTree = varTree.getType(); if (rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments - && lhsTree instanceof ParameterizedTypeTree) { // lhs type has generic + && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic List lhsTypeArguments = - ((ParameterizedTypeTree) lhsTree).getTypeArguments(); + ((ParameterizedTypeTree) lhsTypeTree).getTypeArguments(); // method call has a return type of class type if (methodSymbol.getReturnType() instanceof Type.ClassType) { Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); @@ -444,22 +441,20 @@ public void checkTypeParameterNullnessForAssignability( List returnTypeTypeArg = returnType.getTypeArguments(); // if generic type in return type - if (!typeParam.isEmpty()) { - Map genericNullness = new HashMap<>(); - for (int i = 0; i < typeParam.size(); i++) { - Type upperBound = typeParam.get(i).type.getUpperBound(); - // generic has nullable upperbound - if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { - Type lhsInferredType = - inferMethodTypeArgument( - typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); - if (lhsInferredType != null) { // && has a nullable upperbound - genericNullness.put(typeParam.get(i).type, lhsInferredType); - } + Map genericNullness = new HashMap<>(); + for (int i = 0; i < typeParam.size(); i++) { + Type upperBound = typeParam.get(i).type.getUpperBound(); + // generic has nullable upperbound + if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { + Type lhsInferredType = + inferMethodTypeArgument( + typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); + if (lhsInferredType != null) { // && has a nullable upperbound + genericNullness.put(typeParam.get(i).type, lhsInferredType); } } - inferredTypes.put(rhsTree, genericNullness); } + inferredTypes.put(rhsTree, genericNullness); } } } From 6c4430ee4d24ff7f6ef2c700a23460709b5aad29 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sun, 9 Feb 2025 23:57:57 -0800 Subject: [PATCH 35/55] mid status --- .../uber/nullaway/generics/GenericsChecks.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 48635aa287..a8ff83b7ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -420,7 +420,6 @@ public void checkTypeParameterNullnessForAssignability( } Type lhsType = getTreeType(tree, config); Tree rhsTree; - // update inferredTypes cache for assignments if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); @@ -429,6 +428,7 @@ public void checkTypeParameterNullnessForAssignability( if (rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); + // update inferredTypes cache for assignments if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic @@ -449,7 +449,7 @@ public void checkTypeParameterNullnessForAssignability( Type lhsInferredType = inferMethodTypeArgument( typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); - if (lhsInferredType != null) { // && has a nullable upperbound + if (lhsInferredType != null) { genericNullness.put(typeParam.get(i).type, lhsInferredType); } } @@ -531,12 +531,20 @@ private Type replaceGenerics( // recursive case List newTypeArgument = new ArrayList<>(); for (int i = 0; i < typeWithGenerics.getTypeArguments().size(); i++) { - Type newType = replaceGenerics(currentType.getTypeArguments().get(i), typeWithGenerics.getTypeArguments().get(i), genericNullness); + Type newType = + replaceGenerics( + currentType.getTypeArguments().get(i), + typeWithGenerics.getTypeArguments().get(i), + genericNullness); newTypeArgument.add(newType); } Type.ClassType curClassType = (Type.ClassType) currentType; - Type.ClassType updatedType = new Type.ClassType(curClassType.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgument), curClassType.tsym); + Type.ClassType updatedType = + new Type.ClassType( + curClassType.getEnclosingType(), + com.sun.tools.javac.util.List.from(newTypeArgument), + curClassType.tsym); return updatedType; } From e4051ccf993050a5a0d29a71ee6cec0355bcd08e Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 10 Feb 2025 00:36:11 -0800 Subject: [PATCH 36/55] make code more simple --- .../nullaway/generics/GenericsChecks.java | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index a8ff83b7ca..46992c4481 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -432,13 +432,10 @@ public void checkTypeParameterNullnessForAssignability( if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic - List lhsTypeArguments = - ((ParameterizedTypeTree) lhsTypeTree).getTypeArguments(); // method call has a return type of class type if (methodSymbol.getReturnType() instanceof Type.ClassType) { - Type.ClassType returnType = (Type.ClassType) methodSymbol.getReturnType(); List typeParam = methodSymbol.getTypeParameters(); - List returnTypeTypeArg = returnType.getTypeArguments(); + List returnTypeTypeArg = methodSymbol.getReturnType().getTypeArguments(); // if generic type in return type Map genericNullness = new HashMap<>(); @@ -448,7 +445,10 @@ public void checkTypeParameterNullnessForAssignability( if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { Type lhsInferredType = inferMethodTypeArgument( - typeParam.get(i).type, lhsTypeArguments, returnTypeTypeArg, state); + typeParam.get(i).type, + lhsType.getTypeArguments(), + returnTypeTypeArg, + state); if (lhsInferredType != null) { genericNullness.put(typeParam.get(i).type, lhsInferredType); } @@ -489,32 +489,24 @@ public void checkTypeParameterNullnessForAssignability( } private Type inferMethodTypeArgument( - Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { - // base case - if (typeParam == null || lhsTypeArg == null || typeArg == null) { - return null; - } - - // recursive case + Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { Type inferType = null; + for (int i = 0; i < typeArg.size(); i++) { Type type = typeArg.get(i); + // base case: found type to infer to if (state.getTypes().isSameType(typeParam, type)) { - return ASTHelpers.getType(lhsTypeArg.get(i)); - } else if (!type.getTypeArguments().isEmpty()) { - // instanceof Type.ForAll check if the lhsTypeArg is a generic class? -> maybe - // the base case makes it unnecessary + return lhsTypeArg.get(i); + } else if (!type.getTypeArguments().isEmpty()) { // recursive case: generic class type inferType = inferMethodTypeArgument( - typeParam, - ((ParameterizedTypeTree) lhsTypeArg.get(i)).getTypeArguments(), - typeArg.get(i).getTypeArguments(), - state); + typeParam, lhsTypeArg.get(i).getTypeArguments(), type.getTypeArguments(), state); if (inferType != null) { return inferType; } } } + // base case: typeArg doesn't contain typeParam return inferType; } From 82628ca6b5103eef13e6e32f85ed12c06d911989 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 10 Feb 2025 00:42:57 -0800 Subject: [PATCH 37/55] make pass ./gradlew :nullaway:buildWithNullAway --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 46992c4481..8f07b518db 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -433,7 +433,7 @@ public void checkTypeParameterNullnessForAssignability( && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic // method call has a return type of class type - if (methodSymbol.getReturnType() instanceof Type.ClassType) { + if (lhsType != null && methodSymbol.getReturnType() instanceof Type.ClassType) { List typeParam = methodSymbol.getTypeParameters(); List returnTypeTypeArg = methodSymbol.getReturnType().getTypeArguments(); @@ -488,7 +488,7 @@ public void checkTypeParameterNullnessForAssignability( } } - private Type inferMethodTypeArgument( + private @Nullable Type inferMethodTypeArgument( Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { Type inferType = null; From f8c8d07feb8a2b874d0a01d5348123e24f771f51 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 10 Feb 2025 12:55:38 -0800 Subject: [PATCH 38/55] remove always true condition --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 8f07b518db..b97f99d823 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -432,8 +432,7 @@ public void checkTypeParameterNullnessForAssignability( if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic - // method call has a return type of class type - if (lhsType != null && methodSymbol.getReturnType() instanceof Type.ClassType) { + if (lhsType != null) { List typeParam = methodSymbol.getTypeParameters(); List returnTypeTypeArg = methodSymbol.getReturnType().getTypeArguments(); From 6bc1d0c2ce32fa41e4bf42937e6fa5536c75b8a7 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 10 Feb 2025 15:22:22 -0800 Subject: [PATCH 39/55] make method for clearing inferredTypes cache --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 1 + .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index f05042a527..a478fc5cd3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1664,6 +1664,7 @@ public Description matchClass(ClassTree tree, VisitorState state) { class2Entities.clear(); class2ConstructorUninit.clear(); computedNullnessMap.clear(); + genericsChecks.clearCache(); EnclosingEnvironmentNullness.instance(state.context).clear(); } else if (classAnnotationIntroducesPartialMarking(classSymbol)) { // Handle the case where the top-class is unannotated, but there is a @NullMarked annotation diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index b97f99d823..2f8f821b9a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1263,6 +1263,10 @@ public static boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( return callingUnannotated; } + public void clearCache() { + inferredTypes.clear(); + } + public static boolean isNullableAnnotated(Type type, Config config) { return Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config); } From 29255bbe2de10219a7b36b1628b29ccdc4b3a02f Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 17 Feb 2025 20:12:12 -0800 Subject: [PATCH 40/55] change based on comments --- .../nullaway/generics/GenericsChecks.java | 125 +++++++++++------- 1 file changed, 77 insertions(+), 48 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index a2c2c3b3b0..3ba4a76b88 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -46,6 +46,11 @@ /** Methods for performing checks related to generic types and nullability. */ public final class GenericsChecks { + /** + * Maps a MethodInvocationTree to a set of type variables that are mapped to their inferred types. + * Any generic type parameter that are not explicitly stated are inferred and cached in this + * field. + */ private final Map> inferredTypes = new HashMap<>(); /** @@ -423,7 +428,6 @@ public void checkTypeParameterNullnessForAssignability( if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); - Tree lhsTypeTree = varTree.getType(); if (rhsTree instanceof MethodInvocationTree) { MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; @@ -431,7 +435,8 @@ public void checkTypeParameterNullnessForAssignability( // update inferredTypes cache for assignments if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments - && lhsTypeTree instanceof ParameterizedTypeTree) { // lhs type has generic + && lhsType != null + && !lhsType.getTypeArguments().isEmpty()) { // lhs type has generic if (lhsType != null) { List typeParam = methodSymbol.getTypeParameters(); List returnTypeTypeArg = methodSymbol.getReturnType().getTypeArguments(); @@ -439,17 +444,15 @@ public void checkTypeParameterNullnessForAssignability( // if generic type in return type Map genericNullness = new HashMap<>(); for (int i = 0; i < typeParam.size(); i++) { - Type upperBound = typeParam.get(i).type.getUpperBound(); + Type curTypeParam = typeParam.get(i).type; + Type upperBound = curTypeParam.getUpperBound(); // generic has nullable upperbound if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { Type lhsInferredType = inferMethodTypeArgument( - typeParam.get(i).type, - lhsType.getTypeArguments(), - returnTypeTypeArg, - state); + curTypeParam, lhsType.getTypeArguments(), returnTypeTypeArg, state); if (lhsInferredType != null) { - genericNullness.put(typeParam.get(i).type, lhsInferredType); + genericNullness.put(curTypeParam, lhsInferredType); } } } @@ -474,7 +477,12 @@ public void checkTypeParameterNullnessForAssignability( Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); if (inferredTypes.containsKey(rhsTree)) { Map genericNullness = inferredTypes.get(rhsTree); - rhsType = replaceGenerics(rhsType, methodSymbol.getReturnType(), genericNullness); + com.sun.tools.javac.util.List from = + com.sun.tools.javac.util.List.from(genericNullness.keySet()); + com.sun.tools.javac.util.List to = + com.sun.tools.javac.util.List.from(genericNullness.values()); + rhsType = + TypeSubstitutionUtils.subst(state.getTypes(), methodSymbol.getReturnType(), from, to); } } } @@ -487,57 +495,78 @@ public void checkTypeParameterNullnessForAssignability( } } + /** + * For a type variable, it looks into a set of actual type arguments and type parameters to find + * the type that the type variable can infer to. + * + * @param typeVariable The type variable that we are looking for to infer + * @param actualTypeArg List of actual type arguments that will be used to infer types + * @param typeParameters List of type parameters + * @param state The visitor state + * @return The inferred type or null if typeParameters doesn't contain the type variable + */ private @Nullable Type inferMethodTypeArgument( - Type typeParam, List lhsTypeArg, List typeArg, VisitorState state) { - Type inferType = null; + Type typeVariable, List actualTypeArg, List typeParameters, VisitorState state) { - for (int i = 0; i < typeArg.size(); i++) { - Type type = typeArg.get(i); + // for each type variables + for (int i = 0; i < typeParameters.size(); i++) { + Type type = typeParameters.get(i); // base case: found type to infer to - if (state.getTypes().isSameType(typeParam, type)) { - return lhsTypeArg.get(i); + if (state.getTypes().isSameType(typeVariable, type)) { + return actualTypeArg.get(i); } else if (!type.getTypeArguments().isEmpty()) { // recursive case: generic class type - inferType = + Type inferType = inferMethodTypeArgument( - typeParam, lhsTypeArg.get(i).getTypeArguments(), type.getTypeArguments(), state); + typeVariable, + actualTypeArg.get(i).getTypeArguments(), + type.getTypeArguments(), + state); if (inferType != null) { return inferType; } } } - // base case: typeArg doesn't contain typeParam - return inferType; + // base case: typeParameters doesn't contain typeVariable + return null; } - private Type replaceGenerics( - Type currentType, Type typeWithGenerics, Map genericNullness) { - // base case - if (genericNullness.containsKey(typeWithGenerics)) { // type is generic - return genericNullness.get(typeWithGenerics); - } - if (typeWithGenerics.getTypeArguments().isEmpty()) { // dose not have a generic type argument - return currentType; - } - - // recursive case - List newTypeArgument = new ArrayList<>(); - for (int i = 0; i < typeWithGenerics.getTypeArguments().size(); i++) { - Type newType = - replaceGenerics( - currentType.getTypeArguments().get(i), - typeWithGenerics.getTypeArguments().get(i), - genericNullness); - newTypeArgument.add(newType); - } - - Type.ClassType curClassType = (Type.ClassType) currentType; - Type.ClassType updatedType = - new Type.ClassType( - curClassType.getEnclosingType(), - com.sun.tools.javac.util.List.from(newTypeArgument), - curClassType.tsym); - return updatedType; - } + // /** + // * Replaces the currentType to inferred types in genericNullness. + // * + // * @param currentType Current type that will be replaced with inferred types. + // * @param typeWithGenerics The type of {@code currentType} + // * @param genericNullness A map that contains type variables and their inferred types + // * @return Replaced {@code currentType} if needed + // */ + // private Type replaceGenerics(Type currentType, Type typeWithGenerics, Map genericNullness) { + // // base case + // if (genericNullness.containsKey(typeWithGenerics)) { // type is generic + // return genericNullness.get(typeWithGenerics); + // } + // if (typeWithGenerics.getTypeArguments().isEmpty()) { // does not have a generic type + // argument + // return currentType; + // } + // + // // recursive case + // List newTypeArgument = new ArrayList<>(); + // for (int i = 0; i < typeWithGenerics.getTypeArguments().size(); i++) { + // Type newType = + // replaceGenerics( + // currentType.getTypeArguments().get(i), + // typeWithGenerics.getTypeArguments().get(i), + // genericNullness); + // newTypeArgument.add(newType); + // } + // + // Type.ClassType curClassType = (Type.ClassType) currentType; + // Type.ClassType updatedType = + // new Type.ClassType( + // curClassType.getEnclosingType(), + // com.sun.tools.javac.util.List.from(newTypeArgument), + // curClassType.tsym); + // return updatedType; + // } /** * Checks that the nullability of type parameters for a returned expression matches that of the From 575006edaf823544c2fbb8fb55499a2906bc1242 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 00:35:28 -0800 Subject: [PATCH 41/55] add visitor for infering types on assignments using method calls --- .../nullaway/generics/GenericsChecks.java | 96 +------------------ .../nullaway/generics/InferTypeVisitor.java | 60 ++++++++++++ 2 files changed, 65 insertions(+), 91 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 3ba4a76b88..de794056dd 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -438,25 +438,12 @@ public void checkTypeParameterNullnessForAssignability( && lhsType != null && !lhsType.getTypeArguments().isEmpty()) { // lhs type has generic if (lhsType != null) { - List typeParam = methodSymbol.getTypeParameters(); - List returnTypeTypeArg = methodSymbol.getReturnType().getTypeArguments(); - - // if generic type in return type - Map genericNullness = new HashMap<>(); - for (int i = 0; i < typeParam.size(); i++) { - Type curTypeParam = typeParam.get(i).type; - Type upperBound = curTypeParam.getUpperBound(); - // generic has nullable upperbound - if (getTypeNullness(upperBound, analysis.getConfig()) == Nullness.NULLABLE) { - Type lhsInferredType = - inferMethodTypeArgument( - curTypeParam, lhsType.getTypeArguments(), returnTypeTypeArg, state); - if (lhsInferredType != null) { - genericNullness.put(curTypeParam, lhsInferredType); - } - } + Type returnType = methodSymbol.getReturnType(); + @Nullable Map genericNullness = + returnType.accept(new InferTypeVisitor(config), lhsType); + if (genericNullness != null) { + inferredTypes.put(rhsTree, genericNullness); } - inferredTypes.put(rhsTree, genericNullness); } } } @@ -495,79 +482,6 @@ public void checkTypeParameterNullnessForAssignability( } } - /** - * For a type variable, it looks into a set of actual type arguments and type parameters to find - * the type that the type variable can infer to. - * - * @param typeVariable The type variable that we are looking for to infer - * @param actualTypeArg List of actual type arguments that will be used to infer types - * @param typeParameters List of type parameters - * @param state The visitor state - * @return The inferred type or null if typeParameters doesn't contain the type variable - */ - private @Nullable Type inferMethodTypeArgument( - Type typeVariable, List actualTypeArg, List typeParameters, VisitorState state) { - - // for each type variables - for (int i = 0; i < typeParameters.size(); i++) { - Type type = typeParameters.get(i); - // base case: found type to infer to - if (state.getTypes().isSameType(typeVariable, type)) { - return actualTypeArg.get(i); - } else if (!type.getTypeArguments().isEmpty()) { // recursive case: generic class type - Type inferType = - inferMethodTypeArgument( - typeVariable, - actualTypeArg.get(i).getTypeArguments(), - type.getTypeArguments(), - state); - if (inferType != null) { - return inferType; - } - } - } - // base case: typeParameters doesn't contain typeVariable - return null; - } - - // /** - // * Replaces the currentType to inferred types in genericNullness. - // * - // * @param currentType Current type that will be replaced with inferred types. - // * @param typeWithGenerics The type of {@code currentType} - // * @param genericNullness A map that contains type variables and their inferred types - // * @return Replaced {@code currentType} if needed - // */ - // private Type replaceGenerics(Type currentType, Type typeWithGenerics, Map genericNullness) { - // // base case - // if (genericNullness.containsKey(typeWithGenerics)) { // type is generic - // return genericNullness.get(typeWithGenerics); - // } - // if (typeWithGenerics.getTypeArguments().isEmpty()) { // does not have a generic type - // argument - // return currentType; - // } - // - // // recursive case - // List newTypeArgument = new ArrayList<>(); - // for (int i = 0; i < typeWithGenerics.getTypeArguments().size(); i++) { - // Type newType = - // replaceGenerics( - // currentType.getTypeArguments().get(i), - // typeWithGenerics.getTypeArguments().get(i), - // genericNullness); - // newTypeArgument.add(newType); - // } - // - // Type.ClassType curClassType = (Type.ClassType) currentType; - // Type.ClassType updatedType = - // new Type.ClassType( - // curClassType.getEnclosingType(), - // com.sun.tools.javac.util.List.from(newTypeArgument), - // curClassType.tsym); - // return updatedType; - // } - /** * Checks that the nullability of type parameters for a returned expression matches that of the * type parameters of the enclosing method's return type. diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java new file mode 100644 index 0000000000..7d12131726 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -0,0 +1,60 @@ +package com.uber.nullaway.generics; + +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Types; +import com.uber.nullaway.Config; +import com.uber.nullaway.Nullness; +import java.util.HashMap; +import java.util.Map; +import org.jspecify.annotations.Nullable; + +public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { + private final Config config; + + InferTypeVisitor(Config config) { + this.config = config; + } + + @Override + public @Nullable Map visitClassType(Type.ClassType rhsType, Type lhsType) { + Map genericNullness = new HashMap<>(); + // for each type parameter, call accept with this visitor and add all results to one map + com.sun.tools.javac.util.List rhsTypeArguments = rhsType.getTypeArguments(); + com.sun.tools.javac.util.List lhsTypeArguments = lhsType.getTypeArguments(); + for (int i = 0; i < rhsTypeArguments.size(); i++) { + Type rhsTypeArg = rhsTypeArguments.get(i); + Type lhsTypeArg = lhsTypeArguments.get(i); + Map map = rhsTypeArg.accept(this, lhsTypeArg); + if (map != null) { + genericNullness.putAll(map); + } + } + return genericNullness.isEmpty() ? null : genericNullness; + } + + @Override + public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // type variable itself + Map genericNullness = new HashMap<>(); + // if lhs is not nullable, just use that + // else if rhs upperbound is nullable, use lhs + // use upperbound + Boolean isLhsNullable = + Nullness.hasNullableAnnotation(lhsType.getAnnotationMirrors().stream(), config); + Type upperBound = rhsType.getUpperBound(); + Boolean isRhsNullable = + Nullness.hasNullableAnnotation(upperBound.getAnnotationMirrors().stream(), config); + if (!isLhsNullable) { // lhsType is NonNull, we can just use this + genericNullness.put(rhsType, lhsType); + } else if (isRhsNullable) { // lhsType & rhsType is Nullable, can use lhs for inference + genericNullness.put(rhsType, lhsType); + } else { // rhs can't be nullable, use upperbound + genericNullness.put(rhsType, upperBound); + } + return genericNullness; + } + + @Override + public @Nullable Map visitType(Type t, Type type) { + return null; + } +} From 2ab7a318ab3c8e2142427322626f8e02191ac4db Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 10:20:18 -0800 Subject: [PATCH 42/55] javadoc --- .../java/com/uber/nullaway/generics/InferTypeVisitor.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 7d12131726..4e139e3d1f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -8,6 +8,9 @@ import java.util.Map; import org.jspecify.annotations.Nullable; +/** + * Visitor that uses two types to infer the type of type variables. + */ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { private final Config config; @@ -24,6 +27,7 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map map = rhsTypeArg.accept(this, lhsTypeArg); if (map != null) { genericNullness.putAll(map); @@ -35,9 +39,6 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // type variable itself Map genericNullness = new HashMap<>(); - // if lhs is not nullable, just use that - // else if rhs upperbound is nullable, use lhs - // use upperbound Boolean isLhsNullable = Nullness.hasNullableAnnotation(lhsType.getAnnotationMirrors().stream(), config); Type upperBound = rhsType.getUpperBound(); From 61d6f95599362c9d66f463a51182f514b1f27a33 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 10:37:28 -0800 Subject: [PATCH 43/55] update according to changed method definition --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 3 ++- .../java/com/uber/nullaway/generics/InferTypeVisitor.java | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index de794056dd..1a28ab99ae 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -469,7 +469,8 @@ public void checkTypeParameterNullnessForAssignability( com.sun.tools.javac.util.List to = com.sun.tools.javac.util.List.from(genericNullness.values()); rhsType = - TypeSubstitutionUtils.subst(state.getTypes(), methodSymbol.getReturnType(), from, to); + TypeSubstitutionUtils.subst( + state.getTypes(), methodSymbol.getReturnType(), from, to, config); } } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 4e139e3d1f..4e5248b4f8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -8,9 +8,7 @@ import java.util.Map; import org.jspecify.annotations.Nullable; -/** - * Visitor that uses two types to infer the type of type variables. - */ +/** Visitor that uses two types to infer the type of type variables. */ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { private final Config config; From f7c4a3f27a434fe61e92bf39e3692b5e7eba2f59 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 15:16:03 -0800 Subject: [PATCH 44/55] add test case for return types with arrays --- .../nullaway/jspecify/GenericMethodTests.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index bf68dcdd87..b502853fd4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -285,6 +285,59 @@ public void genericsUsedForGenericClasses() { .doTest(); } + @Test + public void genericInferenceOnAssignmentsWithArrays() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + " class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo[]> test1Null(U u) {", + " return new Foo<>((Foo[]) new Foo[5]);", + " }", + " static Foo[]> test1Nonnull(U u) {", + " return new Foo<>((Foo[]) new Foo[5]);", + " }", + " static Foo[] test2Null(U u) {", + " return (Foo[]) new Foo[5];", + " }", + " static Foo[] test2Nonnull(U u) {", + " return (Foo[]) new Foo[5];", + " }", + " }", + " static void testLocalAssign() {", + " Foo[]> f1 = Foo.test1Null(new Object());", + " Foo[]> f2 = Foo.test1Null(new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo[]> f3 = Foo.test1Null(null);", + " Foo[]> f4 = Foo.test1Null(null);", + " Foo[]> f5 = Foo.test1Nonnull(new Object());", + " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", + " Foo[]> f6 = Foo.test1Nonnull(new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo[]> f7 = Foo.test1Nonnull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo[]> f8 = Foo.test1Nonnull(null);", + " Foo[] f9 = Foo.test2Null(new Object());", + " Foo<@Nullable Object>[] f10 = Foo.test2Null(new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo[] f11 = Foo.test2Null(null);", + " Foo<@Nullable Object>[] f12 = Foo.test2Null(null);", + " Foo[] f13 = Foo.test2Nonnull(new Object());", + " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", + " Foo<@Nullable Object>[] f14 = Foo.test2Nonnull(new Object());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo[] f15 = Foo.test2Nonnull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo<@Nullable Object>[] f16 = Foo.test2Nonnull(null);", + " }", + " }") + .doTest(); + } + @Test public void issue1035() { makeHelper() From 461c30952e6d761d5f25fe1075b2022eaf7522b2 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 15:19:14 -0800 Subject: [PATCH 45/55] add array type inference to visitor --- .../nullaway/generics/GenericsChecks.java | 41 ++++++++----------- .../nullaway/generics/InferTypeVisitor.java | 11 ++++- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 1a28ab99ae..99789077e6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -435,15 +435,12 @@ public void checkTypeParameterNullnessForAssignability( // update inferredTypes cache for assignments if (methodSymbol.type instanceof Type.ForAll // generic method call && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments - && lhsType != null - && !lhsType.getTypeArguments().isEmpty()) { // lhs type has generic - if (lhsType != null) { - Type returnType = methodSymbol.getReturnType(); - @Nullable Map genericNullness = - returnType.accept(new InferTypeVisitor(config), lhsType); - if (genericNullness != null) { - inferredTypes.put(rhsTree, genericNullness); - } + && lhsType != null) { + Type returnType = methodSymbol.getReturnType(); + @Nullable Map genericNullness = + returnType.accept(new InferTypeVisitor(config), lhsType); + if (genericNullness != null) { + inferredTypes.put(rhsTree, genericNullness); } } } @@ -458,20 +455,18 @@ public void checkTypeParameterNullnessForAssignability( } Type rhsType = getTreeType(rhsTree, config); - if (rhsType != null && !rhsType.getTypeArguments().isEmpty()) { - if (rhsTree instanceof MethodInvocationTree) { - // recreate rhsType using inferred types - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); - if (inferredTypes.containsKey(rhsTree)) { - Map genericNullness = inferredTypes.get(rhsTree); - com.sun.tools.javac.util.List from = - com.sun.tools.javac.util.List.from(genericNullness.keySet()); - com.sun.tools.javac.util.List to = - com.sun.tools.javac.util.List.from(genericNullness.values()); - rhsType = - TypeSubstitutionUtils.subst( - state.getTypes(), methodSymbol.getReturnType(), from, to, config); - } + if (rhsType != null && rhsTree instanceof MethodInvocationTree) { + // recreate rhsType using inferred types + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); + if (inferredTypes.containsKey(rhsTree)) { + Map genericNullness = inferredTypes.get(rhsTree); + com.sun.tools.javac.util.List from = + com.sun.tools.javac.util.List.from(genericNullness.keySet()); + com.sun.tools.javac.util.List to = + com.sun.tools.javac.util.List.from(genericNullness.values()); + rhsType = + TypeSubstitutionUtils.subst( + state.getTypes(), methodSymbol.getReturnType(), from, to, config); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 4e5248b4f8..8bbbd04e4b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -21,7 +21,8 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map genericNullness = new HashMap<>(); // for each type parameter, call accept with this visitor and add all results to one map com.sun.tools.javac.util.List rhsTypeArguments = rhsType.getTypeArguments(); - com.sun.tools.javac.util.List lhsTypeArguments = lhsType.getTypeArguments(); + com.sun.tools.javac.util.List lhsTypeArguments = + ((Type.ClassType) lhsType).getTypeArguments(); for (int i = 0; i < rhsTypeArguments.size(); i++) { Type rhsTypeArg = rhsTypeArguments.get(i); Type lhsTypeArg = lhsTypeArguments.get(i); @@ -52,6 +53,14 @@ public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // typ return genericNullness; } + @Override + public @Nullable Map visitArrayType(Type.ArrayType rhsType, Type lhsType) { + Type rhsComponentType = rhsType.elemtype; + Type lhsComponentType = ((Type.ArrayType) lhsType).elemtype; + Map genericNullness = rhsComponentType.accept(this, lhsComponentType); + return genericNullness; + } + @Override public @Nullable Map visitType(Type t, Type type) { return null; From 48bb54245add67da38c33f88f54c0e7f24fee226 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 17:02:43 -0800 Subject: [PATCH 46/55] clean up --- .../java/com/uber/nullaway/generics/InferTypeVisitor.java | 8 ++++---- .../com/uber/nullaway/jspecify/GenericMethodTests.java | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 8bbbd04e4b..76fda0aef7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -19,14 +19,13 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map visitClassType(Type.ClassType rhsType, Type lhsType) { Map genericNullness = new HashMap<>(); - // for each type parameter, call accept with this visitor and add all results to one map com.sun.tools.javac.util.List rhsTypeArguments = rhsType.getTypeArguments(); com.sun.tools.javac.util.List lhsTypeArguments = ((Type.ClassType) lhsType).getTypeArguments(); + // get the inferred type for each type arguments and add them to genericNullness for (int i = 0; i < rhsTypeArguments.size(); i++) { Type rhsTypeArg = rhsTypeArguments.get(i); Type lhsTypeArg = lhsTypeArguments.get(i); - // get the inferred type for each type arguments and add them to genericNullness Map map = rhsTypeArg.accept(this, lhsTypeArg); if (map != null) { genericNullness.putAll(map); @@ -36,7 +35,7 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // type variable itself + public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { Map genericNullness = new HashMap<>(); Boolean isLhsNullable = Nullness.hasNullableAnnotation(lhsType.getAnnotationMirrors().stream(), config); @@ -45,7 +44,7 @@ public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // typ Nullness.hasNullableAnnotation(upperBound.getAnnotationMirrors().stream(), config); if (!isLhsNullable) { // lhsType is NonNull, we can just use this genericNullness.put(rhsType, lhsType); - } else if (isRhsNullable) { // lhsType & rhsType is Nullable, can use lhs for inference + } else if (isRhsNullable) { // lhsType & rhsType are Nullable, can use lhs for inference genericNullness.put(rhsType, lhsType); } else { // rhs can't be nullable, use upperbound genericNullness.put(rhsType, upperBound); @@ -55,6 +54,7 @@ public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { // typ @Override public @Nullable Map visitArrayType(Type.ArrayType rhsType, Type lhsType) { + // unwrap the type of the array and call accept on it Type rhsComponentType = rhsType.elemtype; Type lhsComponentType = ((Type.ArrayType) lhsType).elemtype; Map genericNullness = rhsComponentType.accept(this, lhsComponentType); diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index b502853fd4..1394687297 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -323,15 +323,15 @@ public void genericInferenceOnAssignmentsWithArrays() { " Foo[]> f8 = Foo.test1Nonnull(null);", " Foo[] f9 = Foo.test2Null(new Object());", " Foo<@Nullable Object>[] f10 = Foo.test2Null(new Object());", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo[] f11 = Foo.test2Null(null);", " Foo<@Nullable Object>[] f12 = Foo.test2Null(null);", " Foo[] f13 = Foo.test2Nonnull(new Object());", " // BUG: Diagnostic contains: due to mismatched nullability of type parameters", " Foo<@Nullable Object>[] f14 = Foo.test2Nonnull(new Object());", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo[] f15 = Foo.test2Nonnull(null);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", " Foo<@Nullable Object>[] f16 = Foo.test2Nonnull(null);", " }", " }") From 55738000c72c8df01d8bce340520aedca5e02d66 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 17:12:14 -0800 Subject: [PATCH 47/55] change key of inferredTypes to MethodInvokationTree --- .../com/uber/nullaway/generics/GenericsChecks.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 99789077e6..77d49f3093 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -51,7 +51,7 @@ public final class GenericsChecks { * Any generic type parameter that are not explicitly stated are inferred and cached in this * field. */ - private final Map> inferredTypes = new HashMap<>(); + private final Map> inferredTypes = new HashMap<>(); /** * Checks that for an instantiated generic type, {@code @Nullable} types are only used for type @@ -440,7 +440,7 @@ public void checkTypeParameterNullnessForAssignability( @Nullable Map genericNullness = returnType.accept(new InferTypeVisitor(config), lhsType); if (genericNullness != null) { - inferredTypes.put(rhsTree, genericNullness); + inferredTypes.put(methodInvocationTree, genericNullness); } } } @@ -457,9 +457,10 @@ public void checkTypeParameterNullnessForAssignability( if (rhsType != null && rhsTree instanceof MethodInvocationTree) { // recreate rhsType using inferred types - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) rhsTree); - if (inferredTypes.containsKey(rhsTree)) { - Map genericNullness = inferredTypes.get(rhsTree); + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); + if (inferredTypes.containsKey(methodInvocationTree)) { + Map genericNullness = inferredTypes.get(methodInvocationTree); com.sun.tools.javac.util.List from = com.sun.tools.javac.util.List.from(genericNullness.keySet()); com.sun.tools.javac.util.List to = From bbae811e636fe2e655e67f692b9f732d251589ab Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 19:15:43 -0800 Subject: [PATCH 48/55] change inferredTypes value type to Map --- .../nullaway/generics/GenericsChecks.java | 15 ++++++--- .../nullaway/generics/InferTypeVisitor.java | 33 ++++++++++--------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 77d49f3093..595b7a9841 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -38,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import javax.lang.model.type.ExecutableType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeVariable; @@ -51,7 +52,7 @@ public final class GenericsChecks { * Any generic type parameter that are not explicitly stated are inferred and cached in this * field. */ - private final Map> inferredTypes = new HashMap<>(); + private final Map> inferredTypes = new HashMap<>(); /** * Checks that for an instantiated generic type, {@code @Nullable} types are only used for type @@ -437,7 +438,7 @@ public void checkTypeParameterNullnessForAssignability( && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments && lhsType != null) { Type returnType = methodSymbol.getReturnType(); - @Nullable Map genericNullness = + @Nullable Map genericNullness = returnType.accept(new InferTypeVisitor(config), lhsType); if (genericNullness != null) { inferredTypes.put(methodInvocationTree, genericNullness); @@ -460,9 +461,13 @@ public void checkTypeParameterNullnessForAssignability( MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); if (inferredTypes.containsKey(methodInvocationTree)) { - Map genericNullness = inferredTypes.get(methodInvocationTree); + Map genericNullness = inferredTypes.get(methodInvocationTree); + List keyTypeList = genericNullness.keySet().stream() + .filter(typeVar -> typeVar instanceof Type.TypeVar) + .map(typeVar -> (Type) typeVar) + .collect(Collectors.toList()); com.sun.tools.javac.util.List from = - com.sun.tools.javac.util.List.from(genericNullness.keySet()); + com.sun.tools.javac.util.List.from(keyTypeList); com.sun.tools.javac.util.List to = com.sun.tools.javac.util.List.from(genericNullness.values()); rhsType = @@ -987,7 +992,7 @@ public Nullness getGenericParameterNullnessAtInvocation( } // check nullness of inferred types if (inferredTypes.containsKey(tree)) { - Map genericNullness = inferredTypes.get(tree); + Map genericNullness = inferredTypes.get(tree); List parameters = invokedMethodSymbol.getParameters(); if (genericNullness.containsKey(parameters.get(paramIndex).type)) { Type genericType = parameters.get(paramIndex).type; diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 76fda0aef7..482a5a357c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -6,10 +6,11 @@ import com.uber.nullaway.Nullness; import java.util.HashMap; import java.util.Map; +import javax.lang.model.type.TypeVariable; import org.jspecify.annotations.Nullable; /** Visitor that uses two types to infer the type of type variables. */ -public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { +public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { private final Config config; InferTypeVisitor(Config config) { @@ -17,8 +18,8 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map visitClassType(Type.ClassType rhsType, Type lhsType) { - Map genericNullness = new HashMap<>(); + public @Nullable Map visitClassType(Type.ClassType rhsType, Type lhsType) { + Map genericNullness = new HashMap<>(); com.sun.tools.javac.util.List rhsTypeArguments = rhsType.getTypeArguments(); com.sun.tools.javac.util.List lhsTypeArguments = ((Type.ClassType) lhsType).getTypeArguments(); @@ -26,7 +27,7 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map map = rhsTypeArg.accept(this, lhsTypeArg); + Map map = rhsTypeArg.accept(this, lhsTypeArg); if (map != null) { genericNullness.putAll(map); } @@ -35,8 +36,17 @@ public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { - Map genericNullness = new HashMap<>(); + public @Nullable Map visitArrayType(Type.ArrayType rhsType, Type lhsType) { + // unwrap the type of the array and call accept on it + Type rhsComponentType = rhsType.elemtype; + Type lhsComponentType = ((Type.ArrayType) lhsType).elemtype; + Map genericNullness = rhsComponentType.accept(this, lhsComponentType); + return genericNullness; + } + + @Override + public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { + Map genericNullness = new HashMap<>(); Boolean isLhsNullable = Nullness.hasNullableAnnotation(lhsType.getAnnotationMirrors().stream(), config); Type upperBound = rhsType.getUpperBound(); @@ -53,16 +63,7 @@ public Map visitTypeVar(Type.TypeVar rhsType, Type lhsType) { } @Override - public @Nullable Map visitArrayType(Type.ArrayType rhsType, Type lhsType) { - // unwrap the type of the array and call accept on it - Type rhsComponentType = rhsType.elemtype; - Type lhsComponentType = ((Type.ArrayType) lhsType).elemtype; - Map genericNullness = rhsComponentType.accept(this, lhsComponentType); - return genericNullness; - } - - @Override - public @Nullable Map visitType(Type t, Type type) { + public @Nullable Map visitType(Type t, Type type) { return null; } } From 3c77d708d580063e66d114dc8a5ec185f2e7cca5 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 21 Feb 2025 19:37:10 -0800 Subject: [PATCH 49/55] format --- .../java/com/uber/nullaway/generics/GenericsChecks.java | 6 +++--- .../java/com/uber/nullaway/generics/InferTypeVisitor.java | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 595b7a9841..cccefab267 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -462,12 +462,12 @@ public void checkTypeParameterNullnessForAssignability( Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); if (inferredTypes.containsKey(methodInvocationTree)) { Map genericNullness = inferredTypes.get(methodInvocationTree); - List keyTypeList = genericNullness.keySet().stream() + List keyTypeList = + genericNullness.keySet().stream() .filter(typeVar -> typeVar instanceof Type.TypeVar) .map(typeVar -> (Type) typeVar) .collect(Collectors.toList()); - com.sun.tools.javac.util.List from = - com.sun.tools.javac.util.List.from(keyTypeList); + com.sun.tools.javac.util.List from = com.sun.tools.javac.util.List.from(keyTypeList); com.sun.tools.javac.util.List to = com.sun.tools.javac.util.List.from(genericNullness.values()); rhsType = diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java index 482a5a357c..479ab943d8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/InferTypeVisitor.java @@ -10,7 +10,8 @@ import org.jspecify.annotations.Nullable; /** Visitor that uses two types to infer the type of type variables. */ -public class InferTypeVisitor extends Types.DefaultTypeVisitor<@Nullable Map, Type> { +public class InferTypeVisitor + extends Types.DefaultTypeVisitor<@Nullable Map, Type> { private final Config config; InferTypeVisitor(Config config) { From ea785baabcae2e5503cce7ce09e63435c2dff703 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 22 Feb 2025 09:10:21 -0800 Subject: [PATCH 50/55] remove filter call --- .../src/main/java/com/uber/nullaway/generics/GenericsChecks.java | 1 - 1 file changed, 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index cccefab267..db45d3c74a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -464,7 +464,6 @@ public void checkTypeParameterNullnessForAssignability( Map genericNullness = inferredTypes.get(methodInvocationTree); List keyTypeList = genericNullness.keySet().stream() - .filter(typeVar -> typeVar instanceof Type.TypeVar) .map(typeVar -> (Type) typeVar) .collect(Collectors.toList()); com.sun.tools.javac.util.List from = com.sun.tools.javac.util.List.from(keyTypeList); From 2aec29bfcd15fc483702ce787382db803b29de01 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 22 Feb 2025 09:16:48 -0800 Subject: [PATCH 51/55] simplify --- .../com/uber/nullaway/generics/GenericsChecks.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index db45d3c74a..4ed96c472b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -425,6 +425,9 @@ public void checkTypeParameterNullnessForAssignability( return; } Type lhsType = getTreeType(tree, config); + if (lhsType == null) { + return; + } Tree rhsTree; if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; @@ -434,11 +437,11 @@ public void checkTypeParameterNullnessForAssignability( MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); // update inferredTypes cache for assignments - if (methodSymbol.type instanceof Type.ForAll // generic method call - && methodInvocationTree.getTypeArguments().isEmpty() // no explicit generic arguments - && lhsType != null) { + // generic method call with no explicit generic arguments + if (methodSymbol.type instanceof Type.ForAll + && methodInvocationTree.getTypeArguments().isEmpty()) { Type returnType = methodSymbol.getReturnType(); - @Nullable Map genericNullness = + Map genericNullness = returnType.accept(new InferTypeVisitor(config), lhsType); if (genericNullness != null) { inferredTypes.put(methodInvocationTree, genericNullness); @@ -475,7 +478,7 @@ public void checkTypeParameterNullnessForAssignability( } } - if (lhsType != null && rhsType != null) { + if (rhsType != null) { boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state, config); if (!isAssignmentValid) { reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); From 014fc931ee9b7eabb57d7b84a78f6f91aface985 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 22 Feb 2025 09:23:04 -0800 Subject: [PATCH 52/55] add failing test --- .../nullaway/jspecify/GenericMethodTests.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 1394687297..fb497ae30e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -198,6 +198,33 @@ public void genericInferenceOnAssignments() { .doTest(); } + @Test + public void genericInferenceOnAssignmentAfterDeclaration() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + " class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo makeNull(U u) {", + " return new Foo<>(u);", + " }", + " static Foo makeNonNull(U u) {", + " return new Foo<>(u);", + " }", + " }", + " static void testAssignAfterDeclaration() {", + " // legal", + " Foo<@Nullable Object> f1; f1 = Foo.makeNull(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " Foo f2; f2 = Foo.makeNull(null);", + " }", + " }") + .doTest(); + } + @Test public void genericInferenceOnAssignmentsMultipleParams() { makeHelper() From b45bbad8d06bb6338a6effcf330ad62ee31af740 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sat, 22 Feb 2025 13:32:48 -0800 Subject: [PATCH 53/55] make it work for assignments that are not declarations --- .../nullaway/generics/GenericsChecks.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 4ed96c472b..0b2c2cc4f0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -432,26 +432,26 @@ public void checkTypeParameterNullnessForAssignability( if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; rhsTree = varTree.getInitializer(); - - if (rhsTree instanceof MethodInvocationTree) { - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); - // update inferredTypes cache for assignments - // generic method call with no explicit generic arguments - if (methodSymbol.type instanceof Type.ForAll - && methodInvocationTree.getTypeArguments().isEmpty()) { - Type returnType = methodSymbol.getReturnType(); - Map genericNullness = - returnType.accept(new InferTypeVisitor(config), lhsType); - if (genericNullness != null) { - inferredTypes.put(methodInvocationTree, genericNullness); - } - } - } } else { AssignmentTree assignmentTree = (AssignmentTree) tree; rhsTree = assignmentTree.getExpression(); } + + if (rhsTree instanceof MethodInvocationTree) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); + // update inferredTypes cache for assignments + // generic method call with no explicit generic arguments + if (methodSymbol.type instanceof Type.ForAll + && methodInvocationTree.getTypeArguments().isEmpty()) { + Type returnType = methodSymbol.getReturnType(); + Map genericNullness = + returnType.accept(new InferTypeVisitor(config), lhsType); + if (genericNullness != null) { + inferredTypes.put(methodInvocationTree, genericNullness); + } + } + } // rhsTree can be null for a VariableTree. Also, we don't need to do a check // if rhsTree is the null literal if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { From 50f73de4c13d3f20e7510423145e0e5f027c50ce Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 22 Feb 2025 14:01:48 -0800 Subject: [PATCH 54/55] simplify --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 0b2c2cc4f0..31149da0b4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -459,7 +459,7 @@ public void checkTypeParameterNullnessForAssignability( } Type rhsType = getTreeType(rhsTree, config); - if (rhsType != null && rhsTree instanceof MethodInvocationTree) { + if (rhsTree instanceof MethodInvocationTree) { // recreate rhsType using inferred types MethodInvocationTree methodInvocationTree = (MethodInvocationTree) rhsTree; Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocationTree); From cd2ec93c14f5ee0dc6f3e271e38df07a639b3b60 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 22 Feb 2025 14:24:38 -0800 Subject: [PATCH 55/55] another test --- .../nullaway/jspecify/GenericMethodTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index fb497ae30e..aba1a4a025 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -225,6 +225,30 @@ public void genericInferenceOnAssignmentAfterDeclaration() { .doTest(); } + @Test + public void multipleParametersOneNested() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class Foo {", + " Foo(T t) {}", + " static Foo create(U u, Foo other) {", + " return new Foo<>(u);", + " }", + " static void test(Foo<@Nullable Object> f1, Foo f2) {", + " // no error expected", + " Foo<@Nullable Object> result = Foo.create(null, f1);", + " // BUG: Diagnostic contains: XXX", + " Foo<@Nullable Object> result2 = Foo.create(null, f2);", + " }", + " }", + "}") + .doTest(); + } + @Test public void genericInferenceOnAssignmentsMultipleParams() { makeHelper()