From b3eba82814944363811dfd2fd82d16690b09ab1a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 5 Nov 2024 13:08:39 -0800 Subject: [PATCH 01/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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 2a8512ec96254d572c7e106b4120b17095807480 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 13 Nov 2024 15:43:35 -0800 Subject: [PATCH 09/10] minor cleanup and comments --- .../nullaway/generics/GenericsChecks.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 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 d95ebec3ee..85ca6861bb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -613,6 +613,7 @@ public static void compareGenericTypeParameterNullabilityForCall( return; } Type invokedMethodType = methodSymbol.type; + // substitute class-level type arguments for instance methods if (!methodSymbol.isStatic() && tree instanceof MethodInvocationTree) { ExpressionTree methodSelect = ((MethodInvocationTree) tree).getMethodSelect(); Type enclosingType; @@ -626,10 +627,10 @@ public static void compareGenericTypeParameterNullabilityForCall( invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol); } } - // Handle generic methods + // substitute type arguments for generic methods if (tree instanceof MethodInvocationTree && methodSymbol.type instanceof Type.ForAll) { invokedMethodType = - substituteGenericTypeArgsToExplicit((MethodInvocationTree) tree, methodSymbol, state); + substituteTypeArgsInGenericMethodType((MethodInvocationTree) tree, methodSymbol, state); } List formalParamTypes = invokedMethodType.getParameterTypes(); int n = formalParamTypes.size(); @@ -820,7 +821,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { // Substitute type arguments inside the return type Type substitutedReturnType = - substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getReturnType(); + substituteTypeArgsInGenericMethodType(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 @@ -854,11 +855,18 @@ private static com.sun.tools.javac.util.List convertTreesToTypes( return com.sun.tools.javac.util.List.from(types); } - private static Type substituteGenericTypeArgsToExplicit( - MethodInvocationTree methodInvocationTreetree, + /** + * Substitutes the type arguments from a generic method invocation into the method's type. + * + * @param methodInvocationTree the method invocation tree + * @param methodSymbol symbol for the invoked generic method + * @param state the visitor state + * @return the substituted method type for the generic method + */ + private static Type substituteTypeArgsInGenericMethodType( + MethodInvocationTree methodInvocationTree, Symbol.MethodSymbol methodSymbol, VisitorState state) { - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) methodInvocationTreetree; List typeArgumentTrees = methodInvocationTree.getTypeArguments(); com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); @@ -913,7 +921,8 @@ public static Nullness getGenericParameterNullnessAtInvocation( // Substitute the argument types within the MethodType // NOTE: if explicitTypeArgs is empty, this is a noop List substitutedParamTypes = - substituteGenericTypeArgsToExplicit(tree, invokedMethodSymbol, state).getParameterTypes(); + substituteTypeArgsInGenericMethodType(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 3c8455b3ca5c62cf67b6441846e72671f3555e0b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 13 Nov 2024 15:46:01 -0800 Subject: [PATCH 10/10] tweak comment --- .../java/com/uber/nullaway/jspecify/GenericMethodTests.java | 2 +- 1 file changed, 1 insertion(+), 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 40a356925b..2fbd4e68f0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -133,7 +133,7 @@ public void genericMethodAndVoidType() { } @Test - @Ignore("requires generic method support") + @Ignore("requires inference of generic method type arguments") public void genericMethodAndVoidTypeWithInference() { makeHelper() .addSourceLines(