From 362ee2f0842e9b2ce7cbd8f2503955b1566a217a Mon Sep 17 00:00:00 2001 From: Nikita Dinkar Aware Date: Fri, 31 Mar 2023 15:12:53 -0700 Subject: [PATCH 01/48] all changes for handling method overrides --- .../com/uber/nullaway/GenericsChecks.java | 202 +++++++++++++- .../main/java/com/uber/nullaway/NullAway.java | 24 +- .../AccessPathNullnessPropagation.java | 19 +- .../NullAwayJSpecifyGenericsTests.java | 256 +++++++++++++++++- 4 files changed, 492 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 13ffd147f2..6c20784f60 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -12,6 +12,7 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; @@ -21,6 +22,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.tree.JCTree; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -188,6 +190,38 @@ private void reportInvalidParametersNullabilityError( errorMessage, analysis.buildDescription(paramExpression), state, null)); } + private void reportInvalidOverridingMethodReturnTypeError( + Tree methodTree, Type typeParameterType, Type methodReturnType) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC, + "Cannot return type " + + methodReturnType + + ", as corresponding type parameter has type " + + typeParameterType + + ", which has mismatched type parameter nullability"); + state.reportMatch( + errorBuilder.createErrorDescription( + errorMessage, analysis.buildDescription(methodTree), state, null)); + } + + private void reportInvalidOverridingMethodParamTypeError( + Tree methodTree, Type typeParameterType, Type methodParamType) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC, + "Cannot have method parameter type " + + methodParamType + + ", as corresponding type parameter has type " + + typeParameterType + + ", which has mismatched type parameter nullability"); + state.reportMatch( + errorBuilder.createErrorDescription( + errorMessage, analysis.buildDescription(methodTree), state, null)); + } + /** * This method returns the type of the given tree, including any type use annotations. * @@ -359,7 +393,6 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) Type nullableType = NULLABLE_TYPE_SUPPLIER.get(state); List typeArguments = tree.getTypeArguments(); List newTypeArgs = new ArrayList<>(); - boolean hasNullableAnnotation = false; for (int i = 0; i < typeArguments.size(); i++) { AnnotatedTypeTree annotatedType = null; Tree curTypeArg = typeArguments.get(i); @@ -373,6 +406,7 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) } List annotations = annotatedType != null ? annotatedType.getAnnotations() : Collections.emptyList(); + boolean hasNullableAnnotation = false; for (AnnotationTree annotation : annotations) { if (ASTHelpers.isSameType( nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) { @@ -405,6 +439,32 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) return finalType; } + /** + * The Nullness for the overridden method return type that is derived from the type parameter for + * the owner is by default considered as NonNull. This method computes and returns the Nullness of + * the method return type based on the Nullness of the corresponding instantiated type parameter + * for the owner + * + * @param overriddenMethod A symbol of the overridden method + * @param overridingMethodOwnerType An owner of the overriding method + * @param state Visitor state + * @param config The analysis config + * @return Returns the Nullness of the return type of the overridden method + */ + public static Nullness getOverriddenMethodReturnTypeNullness( + Symbol.MethodSymbol overriddenMethod, + Type overridingMethodOwnerType, + VisitorState state, + Config config) { + + Type overriddenMethodType = + state.getTypes().memberType(overridingMethodOwnerType, overriddenMethod); + if (!(overriddenMethodType instanceof Type.MethodType)) { + return Nullness.NONNULL; + } + return getTypeNullness(overriddenMethodType.getReturnType(), config); + } + /** * For a conditional expression c, check whether the type parameter nullability for each * sub-expression of c matches the type parameter nullability of c itself. @@ -504,4 +564,144 @@ public void compareGenericTypeParameterNullabilityForCall( } } } + + /** + * This method gets a method type with return type and method parameters having annotations of the + * corresponding type parameters of the owner and then use it to compare the nullability + * annotations of the return type and the method params with the corresponding type params of the + * owner. + * + * @param tree A method tree to check + * @param overridingMethod A symbol of the overriding method + * @param overriddenMethod A symbol of the overridden method + */ + public void checkTypeParameterNullnessForMethodOverriding( + MethodTree tree, Symbol.MethodSymbol overridingMethod, Symbol.MethodSymbol overriddenMethod) { + if (!config.isJSpecifyMode()) { + return; + } + // A method type with the return type and method parameters having the annotations of the type + // parameters of the + // owner if they are derived from the type parameters + Type methodWithTypeParams = + state.getTypes().memberType(overridingMethod.owner.type, overriddenMethod); + + checkTypeParameterNullnessForOverridingMethodReturnType(tree, methodWithTypeParams); + checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); + } + + /** + * The Nullness for the overriding method arguments that are derived from the type parameters for + * the owner are by default considered as NonNull. This method computes and returns the Nullness + * of the method arguments based on the Nullness of the corresponding instantiated type parameters + * for the owner + * + * @param paramIndex An index of the method parameter to get the Nullness + * @param methodSymbol A symbol of the overridden method + * @param tree A method tree to check + * @return Returns Nullness of the parameterIndex th parameter of the overriding method + */ + public Nullness getOverridingMethodParamNullness( + int paramIndex, Symbol.MethodSymbol methodSymbol, Tree tree) { + JCTree.JCMethodInvocation methodInvocationTree = (JCTree.JCMethodInvocation) tree; + // if methodInvocationTree.meth is of type JCTree.JCIdent return Nullness.NONNULL + // if not in JSpecify mode then also the Nullness is set to Nullness.NONNULL, so it won't affect + // the logic outside our scope + if (!(methodInvocationTree.meth instanceof JCTree.JCFieldAccess)) { + return Nullness.NONNULL; + } + Type methodReceiverType = ((JCTree.JCFieldAccess) methodInvocationTree.meth).selected.type; + Type methodType = state.getTypes().memberType(methodReceiverType, methodSymbol); + Type formalParamType = methodType.getParameterTypes().get(paramIndex); + return getTypeNullness(formalParamType, config); + } + + /** + * The Nullness for the overridden method arguments that are derived from the type parameters for + * the owner are by default considered as NonNull. This method computes and returns the Nullness + * of the method arguments based on the Nullness of the corresponding instantiated type parameters + * for the owner + * + * @param parameterIndex An index of the method parameter to get the Nullness + * @param overriddenMethod A symbol of the overridden method + * @param overridingMethodParam A paramIndex th parameter of the overriding method + * @return Returns Nullness of the parameterIndex th parameter of the overridden method + */ + public Nullness getOverriddenMethodArgNullness( + int parameterIndex, + Symbol.MethodSymbol overriddenMethod, + Symbol.VarSymbol overridingMethodParam) { + Type methodType = + state.getTypes().memberType(overridingMethodParam.owner.owner.type, overriddenMethod); + Type paramType = methodType.getParameterTypes().get(parameterIndex); + return getTypeNullness(paramType, config); + } + + /** + * This method compares the type parameter annotations for the overriding method parameters with + * corresponding type parameters for the owner and reports an error if they don't match + * + * @param tree A method tree to check + * @param methodWithTypeParams A method type with the annotations of corresponding type parameters + * of the owner of the overriding method + */ + private void checkTypeParameterNullnessForOverridingMethodParameterType( + MethodTree tree, Type methodWithTypeParams) { + List methodParameters = tree.getParameters(); + List typeParameterTypes = methodWithTypeParams.getParameterTypes(); + for (int i = 0; i < methodParameters.size(); i++) { + Type methodParameterType = ASTHelpers.getType(methodParameters.get(i)); + Type typeParameterType = typeParameterTypes.get(i); + if (typeParameterType instanceof Type.ClassType + && methodParameterType instanceof Type.ClassType) { + // for generic types check if the nullability annotations of the type params match + boolean doTypeParamNullabilityAnnotationsMatch = + compareNullabilityAnnotations( + (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType); + + if (!doTypeParamNullabilityAnnotationsMatch) { + reportInvalidOverridingMethodParamTypeError( + methodParameters.get(i), typeParameterType, methodParameterType); + } + } + } + } + + /** + * This method compares the type parameter annotations for the overriding method return type with + * corresponding type parameters for the owner and reports an error if they don't match + * + * @param tree A method tree to check + * @param methodWithTypeParams A method type with the annotations of corresponding type parameters + * of the owner of the overriding method + */ + private void checkTypeParameterNullnessForOverridingMethodReturnType( + MethodTree tree, Type methodWithTypeParams) { + Type typeParamType = methodWithTypeParams.getReturnType(); + Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); + if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { + return; + } + boolean doNullabilityAnnotationsMatch = + compareNullabilityAnnotations( + (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType); + + if (!doNullabilityAnnotationsMatch) { + reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); + } + } + + /** + * @param type A type for which we need the Nullness. + * @param config The analysis config + * @return Returns the Nullness of the type based on the Nullability annotation. + */ + private static Nullness getTypeNullness(Type type, Config config) { + boolean hasNullableAnnotation = + Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config); + if (hasNullableAnnotation) { + return Nullness.NULLABLE; + } + return Nullness.NONNULL; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 51f63fcd29..3cda74eaa8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -627,6 +627,11 @@ public Description matchMethod(MethodTree tree, VisitorState state) { Symbol.MethodSymbol closestOverriddenMethod = NullabilityUtil.getClosestOverriddenMethod(methodSymbol, state.getTypes()); if (closestOverriddenMethod != null) { + if (config.isJSpecifyMode()) { + new GenericsChecks(state, config, this) + .checkTypeParameterNullnessForMethodOverriding( + tree, methodSymbol, closestOverriddenMethod); + } return checkOverriding(closestOverriddenMethod, methodSymbol, null, state); } } @@ -722,7 +727,11 @@ private Description checkParamOverriding( overriddenMethodArgNullnessMap[i] = Nullness.paramHasNullableAnnotation(overriddenMethod, i, config) ? Nullness.NULLABLE - : Nullness.NONNULL; + : (config.isJSpecifyMode() + ? new GenericsChecks(state, config, this) + .getOverriddenMethodArgNullness( + i, overriddenMethod, overridingParamSymbols.get(i)) + : Nullness.NONNULL); } } @@ -913,7 +922,13 @@ private Description checkOverriding( Nullness overriddenMethodReturnNullness = Nullness.NULLABLE; // Permissive default for unannotated code. if (isOverriddenMethodAnnotated && !Nullness.hasNullableAnnotation(overriddenMethod, config)) { - overriddenMethodReturnNullness = Nullness.NONNULL; + if (config.isJSpecifyMode()) { + overriddenMethodReturnNullness = + GenericsChecks.getOverriddenMethodReturnTypeNullness( + overriddenMethod, overridingMethod.owner.type, state, config); + } else { + overriddenMethodReturnNullness = Nullness.NONNULL; + } } overriddenMethodReturnNullness = handler.onOverrideMethodInvocationReturnNullability( @@ -1621,7 +1636,10 @@ private Description handleInvocation( argumentPositionNullness[i] = Nullness.paramHasNullableAnnotation(methodSymbol, i, config) ? Nullness.NULLABLE - : Nullness.NONNULL; + : (config.isJSpecifyMode() + ? new GenericsChecks(state, config, this) + .getOverridingMethodParamNullness(i, methodSymbol, tree) + : Nullness.NONNULL); } } new GenericsChecks(state, config, this) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index 85fa9ce61c..cecc00f8ee 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -33,6 +33,7 @@ import com.sun.tools.javac.code.TypeTag; import com.uber.nullaway.CodeAnnotationInfo; import com.uber.nullaway.Config; +import com.uber.nullaway.GenericsChecks; import com.uber.nullaway.NullabilityUtil; import com.uber.nullaway.Nullness; import com.uber.nullaway.handlers.Handler; @@ -1008,7 +1009,8 @@ Nullness returnValueNullness( nullness = input.getRegularStore().valueOfMethodCall(node, state, NULLABLE, apContext); } else if (node == null || methodReturnsNonNull.test(node) - || !Nullness.hasNullableAnnotation((Symbol) node.getTarget().getMethod(), config)) { + || (!Nullness.hasNullableAnnotation((Symbol) node.getTarget().getMethod(), config) + && !genericReturnIsNullable(node))) { // definite non-null return nullness = NONNULL; } else { @@ -1018,6 +1020,21 @@ Nullness returnValueNullness( return nullness; } + private boolean genericReturnIsNullable(MethodInvocationNode node) { + if (node != null && config.isJSpecifyMode()) { + if (node.getTarget() != null && node.getTarget().getMethod() != null) { + Nullness nullness = + GenericsChecks.getOverriddenMethodReturnTypeNullness( + (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree()), + (Type) node.getTarget().getReceiver().getType(), + state, + config); + return nullness.equals(NULLABLE); + } + } + return false; + } + @Override public TransferResult visitObjectCreation( ObjectCreationNode objectCreationNode, TransferInput input) { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index b16dd52cc7..395a89f24c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -44,10 +44,6 @@ public void constructorTypeParamInstantiation() { " NonNullTypeParam t2 = new NonNullTypeParam<@Nullable String>();", " // BUG: Diagnostic contains: Generic type parameter", " testBadNonNull(new NonNullTypeParam<@Nullable String>());", - " testBadNonNull(", - " new NonNullTypeParam<", - " // BUG: Diagnostic contains: Generic type parameter", - " @Nullable String>());", " }", " static void testOkNullable(NullableTypeParam t1, NullableTypeParam<@Nullable String> t2) {", " NullableTypeParam t3 = new NullableTypeParam();", @@ -718,6 +714,258 @@ public void varargsParameter() { .doTest(); } + @Test + public void overrideReturnTypes() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static class TestFunc1 implements Fn {", + " @Override", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static class TestFunc2 implements Fn {", + " @Override", + " public String apply(String s) {", + " return s;", + " }", + " }", + " static class TestFunc4 implements Fn<@Nullable String, String> {", + " @Override", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static class TestFunc3 implements Fn {", + " @Override", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static void useTestFunc(String s) {", + " Fn f1 = new TestFunc1();", + " String t1 = f1.apply(s);", + " // BUG: Diagnostic contains: dereferenced expression", + " t1.hashCode();", + " TestFunc2 f2 = new TestFunc2();", + " String t2 = f2.apply(s);", + " // There should not be an error here", + " t2.hashCode();", + " Fn f3 = new TestFunc2();", + " String t3 = f3.apply(s);", + " // BUG: Diagnostic contains: dereferenced expression", + " t3.hashCode();", + " }", + "}") + .doTest(); + } + + @Test + public void overrideWithNullCheck() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static class TestFunc1 implements Fn {", + " @Override", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static void useTestFuncWithCast() {", + " Fn f1 = new TestFunc1();", + " if (f1.apply(\"hello\") != null) {", + " String t1 = f1.apply(\"hello\");", + " // no error here due to null check", + " t1.hashCode();", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void overrideParameterType() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static class TestFunc1 implements Fn<@Nullable String, String> {", + " @Override", + " // BUG: Diagnostic contains: parameter s is", + " public String apply(String s) {", + " return s;", + " }", + " }", + " static class TestFunc2 implements Fn<@Nullable String, String> {", + " @Override", + " public String apply(@Nullable String s) {", + " return \"hi\";", + " }", + " }", + " static class TestFunc3 implements Fn {", + " @Override", + " public String apply(String s) {", + " return \"hi\";", + " }", + " }", + " static class TestFunc4 implements Fn {", + " // this override is legal, we should get no error", + " @Override", + " public String apply(@Nullable String s) {", + " return \"hi\";", + " }", + " }", + " static void useTestFunc(String s) {", + " Fn<@Nullable String, String> f1 = new TestFunc2();", + " // should get no error here", + " f1.apply(null);", + " Fn f2 = new TestFunc3();", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " f2.apply(null);", + " }", + "}") + .doTest(); + } + + @Test + public void nestedMethodMatch() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class P{}", + " interface Fn< T extends P, R extends @Nullable Object> {", + " R apply(String s);", + " }", + " static class TestFunc1 implements Fn, @Nullable String> {", + " @Override", + " public String apply(String s) {", + " return s;", + " }", + " }", + " static class TestFunc2 implements Fn, @Nullable String> {", + " @Override", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static void useTestFunc(String s) {", + " Fn, @Nullable String> f1 = new TestFunc1();", + " String t1 = f1.apply(s);", + " // BUG: Diagnostic contains: dereferenced expression", + " t1.hashCode();", + " Fn, @Nullable String> f2 = new TestFunc2();", + " String t2 = f2.apply(s);", + " // BUG: Diagnostic contains: dereferenced expression", + " t2.hashCode();", + " }", + "}") + .doTest(); + } + + @Test + public void methodMatchNullableAnnotatedMethod() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " @Nullable R apply(P p);", + " }", + " static class TestFunc implements Fn {", + " @Override", + " //This override is fine and is handled by the current code", + " public @Nullable String apply(String s) {", + " return s;", + " }", + " }", + " static void useTestFunc(String s) {", + " Fn f = new TestFunc();", + " String t = f.apply(s);", + " // BUG: Diagnostic contains: dereferenced expression", + " t.hashCode();", + " }", + "}") + .doTest(); + } + + @Test + public void nestedMethodReturnTypeMatch() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class P{}", + " interface Fn, R extends @Nullable Object> {", + " T apply();", + " }", + " class TestFunc1 implements Fn, @Nullable String> {", + " @Override", + " // BUG: Diagnostic contains: Cannot return", + " public P<@Nullable String, @Nullable String> apply() {", + " return new P<@Nullable String, @Nullable String>();", + " }", + " }", + " class TestFunc2 implements Fn, @Nullable String> {", + " @Override", + " // BUG: Diagnostic contains: Cannot return", + " public P<@Nullable String, String> apply() {", + " return new P<@Nullable String, String>();", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void nestedMethodParamTypeMatch() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class P{}", + " interface Fn, R extends @Nullable Object> {", + " String apply(T t, String s);", + " }", + " class TestFunc implements Fn, String> {", + " @Override", + " // BUG: Diagnostic contains: Cannot have method parameter", + " public String apply(P<@Nullable String, String> p, String s) {", + " return s;", + " }", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From ac1e7f8217d32beec235b13a598565e327674d6c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 26 Apr 2023 13:11:44 -0400 Subject: [PATCH 02/48] code for pretty printing generic types --- .../com/uber/nullaway/GenericsChecks.java | 78 ++++++++++++++++--- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 6c20784f60..d87b22483f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -1,6 +1,7 @@ package com.uber.nullaway; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; import com.google.errorprone.VisitorState; @@ -18,6 +19,7 @@ import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; @@ -191,15 +193,18 @@ private void reportInvalidParametersNullabilityError( } private void reportInvalidOverridingMethodReturnTypeError( - Tree methodTree, Type typeParameterType, Type methodReturnType) { + Tree methodTree, + Type typeParameterType, + Type methodReturnType, + @SuppressWarnings("UnusedVariable") Symbol.MethodSymbol overriddenMethod) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( - ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC, - "Cannot return type " - + methodReturnType - + ", as corresponding type parameter has type " - + typeParameterType + ErrorMessage.MessageTypes.WRONG_OVERRIDE_RETURN_GENERIC, + "Method returns " + + prettyTypeForError(methodReturnType) + + ", but overridden method returns " + + prettyTypeForError(typeParameterType) + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( @@ -586,7 +591,8 @@ public void checkTypeParameterNullnessForMethodOverriding( Type methodWithTypeParams = state.getTypes().memberType(overridingMethod.owner.type, overriddenMethod); - checkTypeParameterNullnessForOverridingMethodReturnType(tree, methodWithTypeParams); + checkTypeParameterNullnessForOverridingMethodReturnType( + tree, methodWithTypeParams, overriddenMethod); checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); } @@ -674,9 +680,10 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( * @param tree A method tree to check * @param methodWithTypeParams A method type with the annotations of corresponding type parameters * of the owner of the overriding method + * @param overriddenMethod the overridden method */ private void checkTypeParameterNullnessForOverridingMethodReturnType( - MethodTree tree, Type methodWithTypeParams) { + MethodTree tree, Type methodWithTypeParams, Symbol.MethodSymbol overriddenMethod) { Type typeParamType = methodWithTypeParams.getReturnType(); Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { @@ -687,7 +694,8 @@ private void checkTypeParameterNullnessForOverridingMethodReturnType( (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType); if (!doNullabilityAnnotationsMatch) { - reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); + reportInvalidOverridingMethodReturnTypeError( + tree, typeParamType, methodReturnType, overriddenMethod); } } @@ -704,4 +712,56 @@ private static Nullness getTypeNullness(Type type, Config config) { } return Nullness.NONNULL; } + + public static String prettyTypeForError(Type type) { + return type.accept(PRETTY_TYPE_VISITOR, null); + } + + private static final Type.Visitor PRETTY_TYPE_VISITOR = + new Types.DefaultTypeVisitor<>() { + @Override + public String visitWildcardType(Type.WildcardType t, Void unused) { + StringBuilder sb = new StringBuilder(); + sb.append(t.kind); + if (t.kind != BoundKind.UNBOUND) { + sb.append(t.type.accept(this, null)); + } + return sb.toString(); + } + + @Override + public String visitClassType(Type.ClassType t, Void s) { + StringBuilder sb = new StringBuilder(); + for (Attribute.TypeCompound compound : t.getAnnotationMirrors()) { + sb.append('@'); + sb.append(compound.type.accept(this, null)); + sb.append(' '); + } + sb.append(t.tsym.getSimpleName()); + if (t.getTypeArguments().nonEmpty()) { + sb.append('<'); + sb.append( + t.getTypeArguments().stream() + .map(a -> a.accept(this, null)) + .collect(joining(", "))); + sb.append(">"); + } + return sb.toString(); + } + + @Override + public String visitCapturedType(Type.CapturedType t, Void s) { + return t.wildcard.accept(this, null); + } + + @Override + public String visitArrayType(Type.ArrayType t, Void unused) { + return t.elemtype.accept(this, null) + "[]"; + } + + @Override + public String visitType(Type t, Void s) { + return t.toString(); + } + }; } From 241793d6da1cc0b43c0e33fea5bbef94e0df228c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 26 Apr 2023 22:44:42 -0400 Subject: [PATCH 03/48] better printing of types in errors related to generic types --- .../com/uber/nullaway/GenericsChecks.java | 75 +++++++++++++++++-- .../NullAwayJSpecifyGenericsTests.java | 12 +-- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 13ffd147f2..a056046ca1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -1,6 +1,7 @@ package com.uber.nullaway; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; import com.google.errorprone.VisitorState; @@ -17,6 +18,7 @@ import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; @@ -125,9 +127,9 @@ private static void reportInvalidAssignmentInstantiationError( ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE, String.format( "Cannot assign from type " - + rhsType + + prettyTypeForError(rhsType) + " to type " - + lhsType + + prettyTypeForError(lhsType) + " due to mismatched nullability of type parameters")); state.reportMatch( errorBuilder.createErrorDescription( @@ -142,9 +144,9 @@ private static void reportInvalidReturnTypeError( ErrorMessage.MessageTypes.RETURN_NULLABLE_GENERIC, String.format( "Cannot return expression of type " - + returnType + + prettyTypeForError(returnType) + " from method with return type " - + methodType + + prettyTypeForError(methodType) + " due to mismatched nullability of type parameters")); state.reportMatch( errorBuilder.createErrorDescription( @@ -159,9 +161,9 @@ private static void reportMismatchedTypeForTernaryOperator( ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE, String.format( "Conditional expression must have type " - + expressionType + + prettyTypeForError(expressionType) + " but the sub-expression has type " - + subPartType + + prettyTypeForError(subPartType) + ", which has mismatched nullability of type parameters")); state.reportMatch( errorBuilder.createErrorDescription( @@ -179,9 +181,9 @@ private void reportInvalidParametersNullabilityError( new ErrorMessage( ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC, "Cannot pass parameter of type " - + actualParameterType + + prettyTypeForError(actualParameterType) + ", as formal parameter has type " - + formalParameterType + + prettyTypeForError(formalParameterType) + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( @@ -504,4 +506,61 @@ public void compareGenericTypeParameterNullabilityForCall( } } } + + /** + * 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. + */ + public static String prettyTypeForError(Type type) { + return type.accept(PRETTY_TYPE_VISITOR, null); + } + + /** This code is a modified version of code in {@link com.google.errorprone.util.Signatures} */ + private static final Type.Visitor PRETTY_TYPE_VISITOR = + new Types.DefaultTypeVisitor<>() { + @Override + public String visitWildcardType(Type.WildcardType t, Void unused) { + StringBuilder sb = new StringBuilder(); + sb.append(t.kind); + if (t.kind != BoundKind.UNBOUND) { + sb.append(t.type.accept(this, null)); + } + return sb.toString(); + } + + @Override + public String visitClassType(Type.ClassType t, Void s) { + StringBuilder sb = new StringBuilder(); + for (Attribute.TypeCompound compound : t.getAnnotationMirrors()) { + sb.append('@'); + sb.append(compound.type.accept(this, null)); + sb.append(' '); + } + sb.append(t.tsym.getSimpleName()); + if (t.getTypeArguments().nonEmpty()) { + sb.append('<'); + sb.append( + t.getTypeArguments().stream() + .map(a -> a.accept(this, null)) + .collect(joining(", "))); + sb.append(">"); + } + return sb.toString(); + } + + @Override + public String visitCapturedType(Type.CapturedType t, Void s) { + return t.wildcard.accept(this, null); + } + + @Override + public String visitArrayType(Type.ArrayType t, Void unused) { + return t.elemtype.accept(this, null) + "[]"; + } + + @Override + public String visitType(Type t, Void s) { + return t.toString(); + } + }; } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index b16dd52cc7..2cac6729cc 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -234,7 +234,7 @@ public void genericsChecksForAssignments() { "class Test {", " static class NullableTypeParam {}", " static void testPositive(NullableTypeParam<@Nullable String> t1) {", - " // BUG: Diagnostic contains: Cannot assign from type", + " // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>", " NullableTypeParam t2 = t1;", " }", " static void testNegative(NullableTypeParam<@Nullable String> t1) {", @@ -255,7 +255,7 @@ public void nestedChecksForAssignmentsMultipleArguments() { " static class SampleClass {}", " static class SampleClassMultipleArguments {}", " static void testPositive() {", - " // BUG: Diagnostic contains: Cannot assign from type", + " // BUG: Diagnostic contains: Cannot assign from type SampleClassMultipleArguments>", " SampleClassMultipleArguments>, String> t1 =", " new SampleClassMultipleArguments>, String>();", " }", @@ -278,7 +278,7 @@ public void superTypeAssignmentChecksSingleInterface() { " interface Fn

{}", " class FnImpl implements Fn<@Nullable String, @Nullable String> {}", " void testPositive() {", - " // BUG: Diagnostic contains: Cannot assign from type", + " // BUG: Diagnostic contains: Cannot assign from type FnImpl", " Fn<@Nullable String, String> f = new FnImpl();", " }", " void testNegative() {", @@ -499,7 +499,7 @@ public void genericFunctionReturnTypeNewClassTree() { "class Test {", " static class A { }", " static A testPositive1() {", - " // BUG: Diagnostic contains: mismatched nullability of type parameters", + " // BUG: Diagnostic contains: Cannot return expression of type A<@Nullable String>", " return new A<@Nullable String>();", " }", " static A<@Nullable String> testPositive2() {", @@ -567,7 +567,7 @@ public void genericsChecksForTernaryOperator() { "class Test {", "static class A { }", " static A testPositive(A a, boolean t) {", - " // BUG: Diagnostic contains: Conditional expression must have type", + " // BUG: Diagnostic contains: Conditional expression must have type A<@Nullable String>", " A<@Nullable String> t1 = t ? new A() : new A<@Nullable String>();", " // BUG: Diagnostic contains: Conditional expression must have type", " return t ? new A<@Nullable String>() : new A<@Nullable String>();", @@ -678,7 +678,7 @@ public void parameterPassing() { " return a2;", " }", " static void testPositive1(A> a1, A a2) {", - " // BUG: Diagnostic contains: Cannot pass parameter of type", + " // BUG: Diagnostic contains: Cannot pass parameter of type A>", " A a = sampleMethod1(a1, a2);", " }", " static void testPositive2(A> a1, A a2) {", From 7c86f301d9c8ce7e48e8f803ed596c9366009dce Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 26 Apr 2023 22:54:04 -0400 Subject: [PATCH 04/48] fix compile error on JDK 8 --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index a056046ca1..e66430e114 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -517,7 +517,7 @@ public static String prettyTypeForError(Type type) { /** This code is a modified version of code in {@link com.google.errorprone.util.Signatures} */ private static final Type.Visitor PRETTY_TYPE_VISITOR = - new Types.DefaultTypeVisitor<>() { + new Types.DefaultTypeVisitor() { @Override public String visitWildcardType(Type.WildcardType t, Void unused) { StringBuilder sb = new StringBuilder(); From 90b0c698d5c33ec9f3060e051aebd240333f6249 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:04:36 -0400 Subject: [PATCH 05/48] fix --- nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java | 1 + 1 file changed, 1 insertion(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index 22ebf13b4b..65ada0760b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -56,6 +56,7 @@ public enum MessageTypes { ASSIGN_GENERIC_NULLABLE, RETURN_NULLABLE_GENERIC, PASS_NULLABLE_GENERIC, + WRONG_OVERRIDE_RETURN_GENERIC, } public String getMessage() { From 74837a1856d7e53b88b0e9360ddc7bb0cf72a81e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:07:13 -0400 Subject: [PATCH 06/48] cleanup --- .../main/java/com/uber/nullaway/GenericsChecks.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index d0af81ffa0..9f367fefdc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -193,10 +193,7 @@ private void reportInvalidParametersNullabilityError( } private void reportInvalidOverridingMethodReturnTypeError( - Tree methodTree, - Type typeParameterType, - Type methodReturnType, - @SuppressWarnings("UnusedVariable") Symbol.MethodSymbol overriddenMethod) { + Tree methodTree, Type typeParameterType, Type methodReturnType) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( @@ -591,8 +588,7 @@ public void checkTypeParameterNullnessForMethodOverriding( Type methodWithTypeParams = state.getTypes().memberType(overridingMethod.owner.type, overriddenMethod); - checkTypeParameterNullnessForOverridingMethodReturnType( - tree, methodWithTypeParams, overriddenMethod); + checkTypeParameterNullnessForOverridingMethodReturnType(tree, methodWithTypeParams); checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); } @@ -683,7 +679,7 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( * @param overriddenMethod the overridden method */ private void checkTypeParameterNullnessForOverridingMethodReturnType( - MethodTree tree, Type methodWithTypeParams, Symbol.MethodSymbol overriddenMethod) { + MethodTree tree, Type methodWithTypeParams) { Type typeParamType = methodWithTypeParams.getReturnType(); Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { @@ -694,8 +690,7 @@ private void checkTypeParameterNullnessForOverridingMethodReturnType( (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType); if (!doNullabilityAnnotationsMatch) { - reportInvalidOverridingMethodReturnTypeError( - tree, typeParamType, methodReturnType, overriddenMethod); + reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); } } From 6e3f2d0b2dea4c8ef4f8d19ef86f73a8ed192d24 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:13:27 -0400 Subject: [PATCH 07/48] better error message --- .../main/java/com/uber/nullaway/ErrorMessage.java | 1 + .../java/com/uber/nullaway/GenericsChecks.java | 15 +++++++-------- .../nullaway/NullAwayJSpecifyGenericsTests.java | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index 65ada0760b..186b386cde 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -57,6 +57,7 @@ public enum MessageTypes { RETURN_NULLABLE_GENERIC, PASS_NULLABLE_GENERIC, WRONG_OVERRIDE_RETURN_GENERIC, + WRONG_OVERRIDE_PARAM_GENERIC, } public String getMessage() { diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 9f367fefdc..1375e73730 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -209,19 +209,19 @@ private void reportInvalidOverridingMethodReturnTypeError( } private void reportInvalidOverridingMethodParamTypeError( - Tree methodTree, Type typeParameterType, Type methodParamType) { + Tree formalParameterTree, Type typeParameterType, Type methodParamType) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( - ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC, - "Cannot have method parameter type " - + methodParamType - + ", as corresponding type parameter has type " - + typeParameterType + ErrorMessage.MessageTypes.WRONG_OVERRIDE_PARAM_GENERIC, + "Parameter has type " + + prettyTypeForError(methodParamType) + + ", but overridden method has parameter type " + + prettyTypeForError(typeParameterType) + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(methodTree), state, null)); + errorMessage, analysis.buildDescription(formalParameterTree), state, null)); } /** @@ -676,7 +676,6 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( * @param tree A method tree to check * @param methodWithTypeParams A method type with the annotations of corresponding type parameters * of the owner of the overriding method - * @param overriddenMethod the overridden method */ private void checkTypeParameterNullnessForOverridingMethodReturnType( MethodTree tree, Type methodWithTypeParams) { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 24036e74f9..31cebd790b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -957,7 +957,7 @@ public void nestedMethodParamTypeMatch() { " }", " class TestFunc implements Fn, String> {", " @Override", - " // BUG: Diagnostic contains: Cannot have method parameter", + " // BUG: Diagnostic contains: Parameter has type P<@Nullable String, String>, but overridden method has parameter type P", " public String apply(P<@Nullable String, String> p, String s) {", " return s;", " }", From 56cddbe6b64ec9ef1a9501ba6df672f989493f5f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:18:25 -0400 Subject: [PATCH 08/48] WIP --- .github/workflows/continuous-integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 9bb0d03707..deb327857f 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -53,7 +53,7 @@ jobs: ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }} uses: gradle/gradle-build-action@v2 with: - arguments: verGJF build + arguments: -q javaToolchains if: matrix.java == '11' - name: Build and test using Java 17 and Error Prone ${{ matrix.epVersion }} env: From 56fb4885176b391c06d92773f45ccee7528e8583 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:23:17 -0400 Subject: [PATCH 09/48] test --- .github/workflows/continuous-integration.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index deb327857f..2b9ac83243 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -41,6 +41,11 @@ jobs: with: java-version: ${{ matrix.java }} distribution: 'temurin' + - name: 'Set up JDK 17' + uses: actions/setup-java@v3 + with: + java-version: 17 + distribution: 'temurin' - name: Build and test using Java 8 and Error Prone ${{ matrix.epVersion }} env: ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }} @@ -53,7 +58,7 @@ jobs: ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }} uses: gradle/gradle-build-action@v2 with: - arguments: -q javaToolchains + arguments: verGJF build if: matrix.java == '11' - name: Build and test using Java 17 and Error Prone ${{ matrix.epVersion }} env: From 304e5677fbd470a2280ef6987d9706b33a40854a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:26:00 -0400 Subject: [PATCH 10/48] reorder --- .github/workflows/continuous-integration.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 2b9ac83243..eaebf8bbe7 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -36,15 +36,15 @@ jobs: steps: - name: Check out NullAway sources uses: actions/checkout@v3 - - name: 'Set up JDK ${{ matrix.java }}' + - name: 'Set up JDK 17' uses: actions/setup-java@v3 with: - java-version: ${{ matrix.java }} + java-version: '17' distribution: 'temurin' - - name: 'Set up JDK 17' + - name: 'Set up JDK ${{ matrix.java }}' uses: actions/setup-java@v3 with: - java-version: 17 + java-version: ${{ matrix.java }} distribution: 'temurin' - name: Build and test using Java 8 and Error Prone ${{ matrix.epVersion }} env: From 7381c6e2c66461df885ca711aba926c5f934a4ba Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 27 Apr 2023 09:31:30 -0400 Subject: [PATCH 11/48] just windows --- .github/workflows/continuous-integration.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index eaebf8bbe7..5c682a2fd1 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -36,11 +36,12 @@ jobs: steps: - name: Check out NullAway sources uses: actions/checkout@v3 - - name: 'Set up JDK 17' + - name: 'Set up JDK 17 on Windows' uses: actions/setup-java@v3 with: java-version: '17' distribution: 'temurin' + if: matrix.os == 'windows-latest' - name: 'Set up JDK ${{ matrix.java }}' uses: actions/setup-java@v3 with: From 608f3db1fe8fd165648692092e6c5ddb5d484114 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 28 Apr 2023 08:55:12 -0400 Subject: [PATCH 12/48] tweaks --- .../java/com/uber/nullaway/GenericsChecks.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 1375e73730..2a3a925a9b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -442,25 +442,25 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) } /** - * The Nullness for the overridden method return type that is derived from the type parameter for - * the owner is by default considered as NonNull. This method computes and returns the Nullness of - * the method return type based on the Nullness of the corresponding instantiated type parameter - * for the owner + * Computes the nullability of the return type of some generic overridden method in the context of + * an overriding method, based on the nullability of the type parameters used for the overriding + * method's class. * - * @param overriddenMethod A symbol of the overridden method - * @param overridingMethodOwnerType An owner of the overriding method + * @param overriddenMethod the overridden method + * @param overridingMethodEnclosingType the enclosing class of the overriding method * @param state Visitor state * @param config The analysis config - * @return Returns the Nullness of the return type of the overridden method + * @return nullability of the return type of the overridden method in the context of + * overridingMethodEnclosingType */ public static Nullness getOverriddenMethodReturnTypeNullness( Symbol.MethodSymbol overriddenMethod, - Type overridingMethodOwnerType, + Type overridingMethodEnclosingType, VisitorState state, Config config) { Type overriddenMethodType = - state.getTypes().memberType(overridingMethodOwnerType, overriddenMethod); + state.getTypes().memberType(overridingMethodEnclosingType, overriddenMethod); if (!(overriddenMethodType instanceof Type.MethodType)) { return Nullness.NONNULL; } From 39c6e7d6f45d057fd6d9ebdb063937fea1e37293 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 28 Apr 2023 09:23:35 -0400 Subject: [PATCH 13/48] cleanup --- .../com/uber/nullaway/GenericsChecks.java | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 2a3a925a9b..d2d87f7294 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -442,15 +442,14 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) } /** - * Computes the nullability of the return type of some generic overridden method in the context of - * an overriding method, based on the nullability of the type parameters used for the overriding - * method's class. + * Computes the nullability of the return type of some (generic) overridden method in the context + * of the class C of an overriding method, based on the nullability of the type parameters for C. * * @param overriddenMethod the overridden method * @param overridingMethodEnclosingType the enclosing class of the overriding method * @param state Visitor state * @param config The analysis config - * @return nullability of the return type of the overridden method in the context of + * @return nullability of the return type of overriddenMethod in the context of * overridingMethodEnclosingType */ public static Nullness getOverriddenMethodReturnTypeNullness( @@ -462,7 +461,7 @@ public static Nullness getOverriddenMethodReturnTypeNullness( Type overriddenMethodType = state.getTypes().memberType(overridingMethodEnclosingType, overriddenMethod); if (!(overriddenMethodType instanceof Type.MethodType)) { - return Nullness.NONNULL; + throw new RuntimeException("expected method type but instead got " + overriddenMethodType); } return getTypeNullness(overriddenMethodType.getReturnType(), config); } @@ -568,10 +567,8 @@ public void compareGenericTypeParameterNullabilityForCall( } /** - * This method gets a method type with return type and method parameters having annotations of the - * corresponding type parameters of the owner and then use it to compare the nullability - * annotations of the return type and the method params with the corresponding type params of the - * owner. + * Checks that type parameter nullability is consistent between an overriding method and the + * corresponding overridden method. * * @param tree A method tree to check * @param overridingMethod A symbol of the overriding method @@ -582,9 +579,8 @@ public void checkTypeParameterNullnessForMethodOverriding( if (!config.isJSpecifyMode()) { return; } - // A method type with the return type and method parameters having the annotations of the type - // parameters of the - // owner if they are derived from the type parameters + // Obtain type parameters for the overridden method within the context of the overriding + // method's class Type methodWithTypeParams = state.getTypes().memberType(overridingMethod.owner.type, overriddenMethod); From ac24db56a6558a6eb974c35fa5e35db93d1f7485 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 28 Apr 2023 09:31:18 -0400 Subject: [PATCH 14/48] more cleanup --- .../main/java/com/uber/nullaway/GenericsChecks.java | 13 +++++++------ .../src/main/java/com/uber/nullaway/NullAway.java | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index d2d87f7294..e82ecfe039 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -13,6 +13,7 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; @@ -589,18 +590,18 @@ public void checkTypeParameterNullnessForMethodOverriding( } /** - * The Nullness for the overriding method arguments that are derived from the type parameters for - * the owner are by default considered as NonNull. This method computes and returns the Nullness - * of the method arguments based on the Nullness of the corresponding instantiated type parameters - * for the owner + * Computes the nullness of a formal parameter of a generic method at an invocation, in the + * context of the declared type of its receiver. If the formal parameter's type is a type + * variable, its nullness depends on the nullability of the corresponding type parameter in the + * receiver's declared type. * * @param paramIndex An index of the method parameter to get the Nullness * @param methodSymbol A symbol of the overridden method * @param tree A method tree to check * @return Returns Nullness of the parameterIndex th parameter of the overriding method */ - public Nullness getOverridingMethodParamNullness( - int paramIndex, Symbol.MethodSymbol methodSymbol, Tree tree) { + public Nullness getGenericMethodParameterNullness( + int paramIndex, Symbol.MethodSymbol methodSymbol, MethodInvocationTree tree) { JCTree.JCMethodInvocation methodInvocationTree = (JCTree.JCMethodInvocation) tree; // if methodInvocationTree.meth is of type JCTree.JCIdent return Nullness.NONNULL // if not in JSpecify mode then also the Nullness is set to Nullness.NONNULL, so it won't affect diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 8d890ecc10..b49227dce1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1636,9 +1636,10 @@ private Description handleInvocation( argumentPositionNullness[i] = Nullness.paramHasNullableAnnotation(methodSymbol, i, config) ? Nullness.NULLABLE - : (config.isJSpecifyMode() + : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? new GenericsChecks(state, config, this) - .getOverridingMethodParamNullness(i, methodSymbol, tree) + .getGenericMethodParameterNullness( + i, methodSymbol, (MethodInvocationTree) tree) : Nullness.NONNULL); } } From eb1ecaef38d777769226e2f599f2a0e8d5ea550d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 28 Apr 2023 11:53:31 -0400 Subject: [PATCH 15/48] cleanup --- .../com/uber/nullaway/GenericsChecks.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index e82ecfe039..82475584eb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -13,6 +13,7 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; @@ -25,7 +26,6 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; import com.sun.tools.javac.code.Types; -import com.sun.tools.javac.tree.JCTree; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -595,21 +595,21 @@ public void checkTypeParameterNullnessForMethodOverriding( * variable, its nullness depends on the nullability of the corresponding type parameter in the * receiver's declared type. * - * @param paramIndex An index of the method parameter to get the Nullness - * @param methodSymbol A symbol of the overridden method - * @param tree A method tree to check - * @return Returns Nullness of the parameterIndex th parameter of the overriding method + * @param paramIndex parameter index + * @param methodSymbol symbol for the invoked method + * @param tree the tree for the invocation + * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the invoked method + * is not a generic instance method */ public Nullness getGenericMethodParameterNullness( int paramIndex, Symbol.MethodSymbol methodSymbol, MethodInvocationTree tree) { - JCTree.JCMethodInvocation methodInvocationTree = (JCTree.JCMethodInvocation) tree; - // if methodInvocationTree.meth is of type JCTree.JCIdent return Nullness.NONNULL - // if not in JSpecify mode then also the Nullness is set to Nullness.NONNULL, so it won't affect - // the logic outside our scope - if (!(methodInvocationTree.meth instanceof JCTree.JCFieldAccess)) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { + // by default return Nullness.NONNULL; } - Type methodReceiverType = ((JCTree.JCFieldAccess) methodInvocationTree.meth).selected.type; + Type methodReceiverType = + castToNonNull( + ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); Type methodType = state.getTypes().memberType(methodReceiverType, methodSymbol); Type formalParamType = methodType.getParameterTypes().get(paramIndex); return getTypeNullness(formalParamType, config); From 3bbd1d6d5b4c1020f020118e734eb8f59eb35c53 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 30 Apr 2023 10:58:14 -0700 Subject: [PATCH 16/48] docs --- .../com/uber/nullaway/GenericsChecks.java | 79 +++++++++++-------- .../main/java/com/uber/nullaway/NullAway.java | 2 +- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 82475584eb..338e7d43ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -442,31 +442,6 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) return finalType; } - /** - * Computes the nullability of the return type of some (generic) overridden method in the context - * of the class C of an overriding method, based on the nullability of the type parameters for C. - * - * @param overriddenMethod the overridden method - * @param overridingMethodEnclosingType the enclosing class of the overriding method - * @param state Visitor state - * @param config The analysis config - * @return nullability of the return type of overriddenMethod in the context of - * overridingMethodEnclosingType - */ - public static Nullness getOverriddenMethodReturnTypeNullness( - Symbol.MethodSymbol overriddenMethod, - Type overridingMethodEnclosingType, - VisitorState state, - Config config) { - - Type overriddenMethodType = - state.getTypes().memberType(overridingMethodEnclosingType, overriddenMethod); - if (!(overriddenMethodType instanceof Type.MethodType)) { - throw new RuntimeException("expected method type but instead got " + overriddenMethodType); - } - return getTypeNullness(overriddenMethodType.getReturnType(), config); - } - /** * For a conditional expression c, check whether the type parameter nullability for each * sub-expression of c matches the type parameter nullability of c itself. @@ -589,22 +564,64 @@ public void checkTypeParameterNullnessForMethodOverriding( checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); } + /** + * Computes the nullability of the return type of some (generic) overridden method in the context + * of the class C of an overriding method, based on the nullability of the type parameters for C. + * + *

Consider the following example: + * + * 

+   *     interface Fn
 {
+   *         R apply(P p);
+   *     }
+   *     class C implements Fn {
+   *         public @Nullable String apply(String p) {
+   *             return null;
+   *         }
+   *     }
+   *
+ * + * Within the context of class {@code C}, the inherited method {@code Fn.apply} has a return type + * of {@code @Nullable String}, since {@code @Nullable String} is passed as the type parameter for + * {@code R}. Hence, it is valid for overriding method {@code C.apply} to return {@code @Nullable + * String}. + * + * @param overriddenMethod the overridden method + * @param overridingMethodEnclosingType the enclosing class of the overriding method + * @param state Visitor state + * @param config The analysis config + * @return nullability of the return type of overriddenMethod in the context of + * overridingMethodEnclosingType + */ + public static Nullness getOverriddenMethodReturnTypeNullness( + Symbol.MethodSymbol overriddenMethod, + Type overridingMethodEnclosingType, + VisitorState state, + Config config) { + + Type overriddenMethodType = + state.getTypes().memberType(overridingMethodEnclosingType, overriddenMethod); + if (!(overriddenMethodType instanceof Type.MethodType)) { + throw new RuntimeException("expected method type but instead got " + overriddenMethodType); + } + return getTypeNullness(overriddenMethodType.getReturnType(), config); + } + /** * Computes the nullness of a formal parameter of a generic method at an invocation, in the - * context of the declared type of its receiver. If the formal parameter's type is a type + * context of the declared type of its receiver argument. If the formal parameter's type is a type * variable, its nullness depends on the nullability of the corresponding type parameter in the - * receiver's declared type. + * receiver's type. * * @param paramIndex parameter index * @param methodSymbol symbol for the invoked method * @param tree the tree for the invocation - * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the invoked method - * is not a generic instance method + * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not + * invoke an instance method */ - public Nullness getGenericMethodParameterNullness( + public Nullness getGenericParameterNullnessAtInvocation( int paramIndex, Symbol.MethodSymbol methodSymbol, MethodInvocationTree tree) { if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { - // by default return Nullness.NONNULL; } Type methodReceiverType = diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index b49227dce1..e39a318c74 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1638,7 +1638,7 @@ private Description handleInvocation( ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) ? new GenericsChecks(state, config, this) - .getGenericMethodParameterNullness( + .getGenericParameterNullnessAtInvocation( i, methodSymbol, (MethodInvocationTree) tree) : Nullness.NONNULL); } From ee73b04d561d2e165bd89ee6efb9008aa6d0b1d8 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 5 May 2023 18:08:26 -0700 Subject: [PATCH 17/48] more --- .../com/uber/nullaway/GenericsChecks.java | 40 +++++++++++++------ .../main/java/com/uber/nullaway/NullAway.java | 4 +- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 338e7d43ca..39c4943945 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -613,23 +613,42 @@ public static Nullness getOverriddenMethodReturnTypeNullness( * variable, its nullness depends on the nullability of the corresponding type parameter in the * receiver's type. * + *

Consider the following example: + * + * 

+   *     interface Fn
 {
+   *         R apply(P p);
+   *     }
+   *     class C implements Fn<@Nullable String, String> {
+   *         public String apply(@Nullable String p) {
+   *             return "";
+   *         }
+   *     }
+   *     static void m() {
+   *         Fn<@Nullable String, String> f = new C();
+   *         f.apply(null);
+   *     }
+   *
+ * + * The declared type of {@code f} passes {@code Nullable String} as the type parameter for type + * variable {@code P}. So, it is legal to pass {@code null} as a parameter to {@code f.apply}. + * * @param paramIndex parameter index - * @param methodSymbol symbol for the invoked method + * @param invokedMethodSymbol symbol for the invoked method * @param tree the tree for the invocation * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not * invoke an instance method */ public Nullness getGenericParameterNullnessAtInvocation( - int paramIndex, Symbol.MethodSymbol methodSymbol, MethodInvocationTree tree) { + int paramIndex, Symbol.MethodSymbol invokedMethodSymbol, MethodInvocationTree tree) { if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { return Nullness.NONNULL; } Type methodReceiverType = castToNonNull( ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); - Type methodType = state.getTypes().memberType(methodReceiverType, methodSymbol); - Type formalParamType = methodType.getParameterTypes().get(paramIndex); - return getTypeNullness(formalParamType, config); + return getOverriddenMethodParameterNullness( + paramIndex, invokedMethodSymbol, methodReceiverType); } /** @@ -640,15 +659,12 @@ public Nullness getGenericParameterNullnessAtInvocation( * * @param parameterIndex An index of the method parameter to get the Nullness * @param overriddenMethod A symbol of the overridden method - * @param overridingMethodParam A paramIndex th parameter of the overriding method + * @param enclosingType A paramIndex th parameter of the overriding method * @return Returns Nullness of the parameterIndex th parameter of the overridden method */ - public Nullness getOverriddenMethodArgNullness( - int parameterIndex, - Symbol.MethodSymbol overriddenMethod, - Symbol.VarSymbol overridingMethodParam) { - Type methodType = - state.getTypes().memberType(overridingMethodParam.owner.owner.type, overriddenMethod); + public Nullness getOverriddenMethodParameterNullness( + int parameterIndex, Symbol.MethodSymbol overriddenMethod, Type enclosingType) { + Type methodType = state.getTypes().memberType(enclosingType, overriddenMethod); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e39a318c74..23defbbeb8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -729,8 +729,8 @@ private Description checkParamOverriding( ? Nullness.NULLABLE : (config.isJSpecifyMode() ? new GenericsChecks(state, config, this) - .getOverriddenMethodArgNullness( - i, overriddenMethod, overridingParamSymbols.get(i)) + .getOverriddenMethodParameterNullness( + i, overriddenMethod, overridingParamSymbols.get(i).owner.owner.type) : Nullness.NONNULL); } } From e56cb1258e2da73a15495094abf2006745c6bd56 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 5 May 2023 18:24:23 -0700 Subject: [PATCH 18/48] more --- .../com/uber/nullaway/GenericsChecks.java | 38 +++++++++---------- .../main/java/com/uber/nullaway/NullAway.java | 4 +- .../AccessPathNullnessPropagation.java | 10 ++++- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 39c4943945..647d893531 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -565,8 +565,8 @@ public void checkTypeParameterNullnessForMethodOverriding( } /** - * Computes the nullability of the return type of some (generic) overridden method in the context - * of the class C of an overriding method, based on the nullability of the type parameters for C. + * Computes the nullability of the return type of some generic method when seen as a member of + * some class {@code C}, based on type parameter nullability within {@code C}. * *

Consider the following example: * @@ -581,26 +581,23 @@ public void checkTypeParameterNullnessForMethodOverriding( * } * * - * Within the context of class {@code C}, the inherited method {@code Fn.apply} has a return type - * of {@code @Nullable String}, since {@code @Nullable String} is passed as the type parameter for + * Within the context of class {@code C}, the method {@code Fn.apply} has a return type of + * {@code @Nullable String}, since {@code @Nullable String} is passed as the type parameter for * {@code R}. Hence, it is valid for overriding method {@code C.apply} to return {@code @Nullable * String}. * - * @param overriddenMethod the overridden method - * @param overridingMethodEnclosingType the enclosing class of the overriding method + * @param method the generic method + * @param enclosingType the enclosing type in which we want to know {@code method}'s return type + * nullability * @param state Visitor state * @param config The analysis config - * @return nullability of the return type of overriddenMethod in the context of - * overridingMethodEnclosingType + * @return nullability of the return type of {@code method} in the context of {@code + * enclosingType} */ - public static Nullness getOverriddenMethodReturnTypeNullness( - Symbol.MethodSymbol overriddenMethod, - Type overridingMethodEnclosingType, - VisitorState state, - Config config) { + public static Nullness getGenericMethodReturnTypeNullness( + Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { - Type overriddenMethodType = - state.getTypes().memberType(overridingMethodEnclosingType, overriddenMethod); + Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { throw new RuntimeException("expected method type but instead got " + overriddenMethodType); } @@ -647,8 +644,7 @@ public Nullness getGenericParameterNullnessAtInvocation( Type methodReceiverType = castToNonNull( ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); - return getOverriddenMethodParameterNullness( - paramIndex, invokedMethodSymbol, methodReceiverType); + return getGenericMethodParameterNullness(paramIndex, invokedMethodSymbol, methodReceiverType); } /** @@ -658,13 +654,13 @@ public Nullness getGenericParameterNullnessAtInvocation( * for the owner * * @param parameterIndex An index of the method parameter to get the Nullness - * @param overriddenMethod A symbol of the overridden method + * @param method A symbol of the overridden method * @param enclosingType A paramIndex th parameter of the overriding method * @return Returns Nullness of the parameterIndex th parameter of the overridden method */ - public Nullness getOverriddenMethodParameterNullness( - int parameterIndex, Symbol.MethodSymbol overriddenMethod, Type enclosingType) { - Type methodType = state.getTypes().memberType(enclosingType, overriddenMethod); + public Nullness getGenericMethodParameterNullness( + int parameterIndex, Symbol.MethodSymbol method, Type enclosingType) { + Type methodType = state.getTypes().memberType(enclosingType, method); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 23defbbeb8..340fad556c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -729,7 +729,7 @@ private Description checkParamOverriding( ? Nullness.NULLABLE : (config.isJSpecifyMode() ? new GenericsChecks(state, config, this) - .getOverriddenMethodParameterNullness( + .getGenericMethodParameterNullness( i, overriddenMethod, overridingParamSymbols.get(i).owner.owner.type) : Nullness.NONNULL); } @@ -924,7 +924,7 @@ private Description checkOverriding( if (isOverriddenMethodAnnotated && !Nullness.hasNullableAnnotation(overriddenMethod, config)) { if (config.isJSpecifyMode()) { overriddenMethodReturnNullness = - GenericsChecks.getOverriddenMethodReturnTypeNullness( + GenericsChecks.getGenericMethodReturnTypeNullness( overriddenMethod, overridingMethod.owner.type, state, config); } else { overriddenMethodReturnNullness = Nullness.NONNULL; diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index cecc00f8ee..c3c077dccd 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -1020,11 +1020,19 @@ Nullness returnValueNullness( return nullness; } + /** + * Computes the nullability of a generic return type in the context of some receiver type at an + * invocation. + * + * @param node the invocation node + * @return nullability of the return type in the context of the type of the receiver argument at + * {@code node} + */ private boolean genericReturnIsNullable(MethodInvocationNode node) { if (node != null && config.isJSpecifyMode()) { if (node.getTarget() != null && node.getTarget().getMethod() != null) { Nullness nullness = - GenericsChecks.getOverriddenMethodReturnTypeNullness( + GenericsChecks.getGenericMethodReturnTypeNullness( (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree()), (Type) node.getTarget().getReceiver().getType(), state, From 05782ebccc99255f948e71db5997aefc7c27c144 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 5 May 2023 18:33:58 -0700 Subject: [PATCH 19/48] more --- .../com/uber/nullaway/GenericsChecks.java | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 647d893531..1793cb8291 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -596,7 +596,6 @@ public void checkTypeParameterNullnessForMethodOverriding( */ public static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { - Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { throw new RuntimeException("expected method type but instead got " + overriddenMethodType); @@ -648,15 +647,33 @@ public Nullness getGenericParameterNullnessAtInvocation( } /** - * The Nullness for the overridden method arguments that are derived from the type parameters for - * the owner are by default considered as NonNull. This method computes and returns the Nullness - * of the method arguments based on the Nullness of the corresponding instantiated type parameters - * for the owner + * Computes the nullability of a parameter type of some generic method when seen as a member of + * some class {@code C}, based on type parameter nullability within {@code C}. * - * @param parameterIndex An index of the method parameter to get the Nullness - * @param method A symbol of the overridden method - * @param enclosingType A paramIndex th parameter of the overriding method - * @return Returns Nullness of the parameterIndex th parameter of the overridden method + *

Consider the following example: + * + * 

+   *     interface Fn
 {
+   *         R apply(P p);
+   *     }
+   *     class C implements Fn<@Nullable String, String> {
+   *         public String apply(@Nullable String p) {
+   *             return "";
+   *         }
+   *     }
+   *
+ * + * Within the context of class {@code C}, the method {@code Fn.apply} has a parameter type of + * {@code @Nullable String}, since {@code @Nullable String} is passed as the type parameter for + * {@code P}. Hence, overriding method {@code C.apply} must take a {@code @Nullable String} as a + * parameter. + * + * @param parameterIndex index of the parameter + * @param method the generic method + * @param enclosingType the enclosing type in which we want to know {@code method}'s parameter + * type nullability + * @return nullability of the relevant parameter type of {@code method} in the context of {@code + * enclosingType} */ public Nullness getGenericMethodParameterNullness( int parameterIndex, Symbol.MethodSymbol method, Type enclosingType) { From 90a4530713dee43b72230c99182a314baead3a1f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 6 May 2023 08:43:51 -0700 Subject: [PATCH 20/48] more --- .../com/uber/nullaway/GenericsChecks.java | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 1793cb8291..3e74bbd7ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -683,28 +683,24 @@ public Nullness getGenericMethodParameterNullness( } /** - * This method compares the type parameter annotations for the overriding method parameters with - * corresponding type parameters for the owner and reports an error if they don't match + * This method compares the type parameter annotations for overriding method parameters with + * corresponding type parameters for the overridden method and reports an error if they don't + * match * - * @param tree A method tree to check - * @param methodWithTypeParams A method type with the annotations of corresponding type parameters - * of the owner of the overriding method + * @param tree tree for overriding method + * @param overriddenMethodType type of the overridden method */ private void checkTypeParameterNullnessForOverridingMethodParameterType( - MethodTree tree, Type methodWithTypeParams) { + MethodTree tree, Type overriddenMethodType) { List methodParameters = tree.getParameters(); - List typeParameterTypes = methodWithTypeParams.getParameterTypes(); + List typeParameterTypes = overriddenMethodType.getParameterTypes(); for (int i = 0; i < methodParameters.size(); i++) { Type methodParameterType = ASTHelpers.getType(methodParameters.get(i)); Type typeParameterType = typeParameterTypes.get(i); if (typeParameterType instanceof Type.ClassType && methodParameterType instanceof Type.ClassType) { - // for generic types check if the nullability annotations of the type params match - boolean doTypeParamNullabilityAnnotationsMatch = - compareNullabilityAnnotations( - (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType); - - if (!doTypeParamNullabilityAnnotationsMatch) { + if (!compareNullabilityAnnotations( + (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType)) { reportInvalidOverridingMethodParamTypeError( methodParameters.get(i), typeParameterType, methodParameterType); } @@ -713,25 +709,22 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( } /** - * This method compares the type parameter annotations for the overriding method return type with - * corresponding type parameters for the owner and reports an error if they don't match + * This method compares the type parameter annotations for an overriding method's return type with + * corresponding type parameters for the overridden method and reports an error if they don't + * match * - * @param tree A method tree to check - * @param methodWithTypeParams A method type with the annotations of corresponding type parameters - * of the owner of the overriding method + * @param tree tree for overriding method + * @param overriddenMethodType type of the overridden method */ private void checkTypeParameterNullnessForOverridingMethodReturnType( - MethodTree tree, Type methodWithTypeParams) { - Type typeParamType = methodWithTypeParams.getReturnType(); + MethodTree tree, Type overriddenMethodType) { + Type typeParamType = overriddenMethodType.getReturnType(); Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { return; } - boolean doNullabilityAnnotationsMatch = - compareNullabilityAnnotations( - (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType); - - if (!doNullabilityAnnotationsMatch) { + if (!compareNullabilityAnnotations( + (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType)) { reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); } } From a769e30cbf65fc8498774a59bb4c1ff3d92fde13 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 6 May 2023 08:48:52 -0700 Subject: [PATCH 21/48] cleanup tests --- .../NullAwayJSpecifyGenericsTests.java | 44 ++----------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 31cebd790b..a408484053 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -848,45 +848,7 @@ public void overrideParameterType() { } @Test - public void nestedMethodMatch() { - makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "class Test {", - " class P{}", - " interface Fn< T extends P, R extends @Nullable Object> {", - " R apply(String s);", - " }", - " static class TestFunc1 implements Fn, @Nullable String> {", - " @Override", - " public String apply(String s) {", - " return s;", - " }", - " }", - " static class TestFunc2 implements Fn, @Nullable String> {", - " @Override", - " public @Nullable String apply(String s) {", - " return s;", - " }", - " }", - " static void useTestFunc(String s) {", - " Fn, @Nullable String> f1 = new TestFunc1();", - " String t1 = f1.apply(s);", - " // BUG: Diagnostic contains: dereferenced expression", - " t1.hashCode();", - " Fn, @Nullable String> f2 = new TestFunc2();", - " String t2 = f2.apply(s);", - " // BUG: Diagnostic contains: dereferenced expression", - " t2.hashCode();", - " }", - "}") - .doTest(); - } - - @Test - public void methodMatchNullableAnnotatedMethod() { + public void nullableGenericTypeVariable() { makeHelper() .addSourceLines( "Test.java", @@ -914,7 +876,7 @@ public void methodMatchNullableAnnotatedMethod() { } @Test - public void nestedMethodReturnTypeMatch() { + public void overrideWithNestedTypeParametersInReturnType() { makeHelper() .addSourceLines( "Test.java", @@ -944,7 +906,7 @@ public void nestedMethodReturnTypeMatch() { } @Test - public void nestedMethodParamTypeMatch() { + public void overrideWithNestedTypeParametersInParameterType() { makeHelper() .addSourceLines( "Test.java", From 6630bbfb24a852e307a7cf66f3cf768d1aded025 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 6 May 2023 08:49:27 -0700 Subject: [PATCH 22/48] another rename --- .../java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index a408484053..e449d40dc3 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -848,7 +848,7 @@ public void overrideParameterType() { } @Test - public void nullableGenericTypeVariable() { + public void nullableGenericTypeVariableReturnType() { makeHelper() .addSourceLines( "Test.java", From b33ff485bb79af33e5b5bfd09dd3a69b345bf5dc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 6 May 2023 14:03:57 -0700 Subject: [PATCH 23/48] WIP --- .../com/uber/nullaway/GenericsChecks.java | 22 ++++++++++++++----- .../main/java/com/uber/nullaway/NullAway.java | 2 +- .../NullAwayJSpecifyGenericsTests.java | 20 +++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 3e74bbd7ca..8246226043 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -20,6 +20,8 @@ import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.Trees; import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; @@ -640,10 +642,10 @@ public Nullness getGenericParameterNullnessAtInvocation( if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { return Nullness.NONNULL; } - Type methodReceiverType = + Symbol methodReceiverSymbol = castToNonNull( - ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); - return getGenericMethodParameterNullness(paramIndex, invokedMethodSymbol, methodReceiverType); + ASTHelpers.getSymbol(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + return getGenericMethodParameterNullness(paramIndex, invokedMethodSymbol, methodReceiverSymbol); } /** @@ -670,18 +672,28 @@ public Nullness getGenericParameterNullnessAtInvocation( * * @param parameterIndex index of the parameter * @param method the generic method - * @param enclosingType the enclosing type in which we want to know {@code method}'s parameter + * @param enclosingSymbol the enclosing type in which we want to know {@code method}'s parameter * type nullability * @return nullability of the relevant parameter type of {@code method} in the context of {@code * enclosingType} */ public Nullness getGenericMethodParameterNullness( - int parameterIndex, Symbol.MethodSymbol method, Type enclosingType) { + int parameterIndex, Symbol.MethodSymbol method, Symbol enclosingSymbol) { + Type enclosingType = + enclosingSymbol.isAnonymous() + ? getTypeForAnonymousSymbol(enclosingSymbol) + : enclosingSymbol.type; Type methodType = state.getTypes().memberType(enclosingType, method); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); } + private Type getTypeForAnonymousSymbol(Symbol enclosingSymbol) { + Trees treesInstance = NullAway.getTreesInstance(state); + TreePath path = treesInstance.getPath(enclosingSymbol); + return getTreeType(path.getParentPath().getLeaf()); + } + /** * This method compares the type parameter annotations for overriding method parameters with * corresponding type parameters for the overridden method and reports an error if they don't diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 340fad556c..bbc8034538 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -730,7 +730,7 @@ private Description checkParamOverriding( : (config.isJSpecifyMode() ? new GenericsChecks(state, config, this) .getGenericMethodParameterNullness( - i, overriddenMethod, overridingParamSymbols.get(i).owner.owner.type) + i, overriddenMethod, overridingParamSymbols.get(i).owner.owner) : Nullness.NONNULL); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index e449d40dc3..fce0dac91f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -847,6 +847,26 @@ public void overrideParameterType() { .doTest(); } + @Test + public void overrideExplicitlyTypedAnonymousClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static void anonymousClasses() {", + " Fn<@Nullable String, String> fn1 = new Fn<@Nullable String, String>() {", + " public String apply(String s) { return s; }", + " };", + " }", + "}") + .doTest(); + } + @Test public void nullableGenericTypeVariableReturnType() { makeHelper() From 850095760fcf47ecba4b37db40c041afaee4107d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 7 May 2023 09:10:49 -0700 Subject: [PATCH 24/48] more tests --- .../nullaway/NullAwayJSpecifyGenericsTests.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index fce0dac91f..866c83f536 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -858,10 +858,24 @@ public void overrideExplicitlyTypedAnonymousClass() { " interface Fn

{", " R apply(P p);", " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", " static void anonymousClasses() {", " Fn<@Nullable String, String> fn1 = new Fn<@Nullable String, String>() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " FnClass<@Nullable String, String> fn2 = new FnClass<@Nullable String, String>() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", " public String apply(String s) { return s; }", " };", + " Fn fn3 = new Fn() {", + " public @Nullable String apply(String s) { return null; }", + " };", + " FnClass fn4 = new FnClass() {", + " public @Nullable String apply(String s) { return null; }", + " };", " }", "}") .doTest(); From 250fb68e02939c1cf80cabd5458c48b65d4c4339 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 7 May 2023 09:11:13 -0700 Subject: [PATCH 25/48] more --- .../com/uber/nullaway/GenericsChecks.java | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 8246226043..9a0d0c29c4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -11,6 +11,7 @@ import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ClassTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MemberSelectTree; @@ -20,7 +21,6 @@ import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; -import com.sun.source.util.TreePath; import com.sun.source.util.Trees; import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.BoundKind; @@ -597,7 +597,11 @@ public void checkTypeParameterNullnessForMethodOverriding( * enclosingType} */ public static Nullness getGenericMethodReturnTypeNullness( - Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { + Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) { + Type enclosingType = + enclosingSymbol.isAnonymous() + ? getTypeForAnonymousSymbol(enclosingSymbol) + : enclosingSymbol.type; Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { throw new RuntimeException("expected method type but instead got " + overriddenMethodType); @@ -688,10 +692,38 @@ public Nullness getGenericMethodParameterNullness( return getTypeNullness(paramType, config); } - private Type getTypeForAnonymousSymbol(Symbol enclosingSymbol) { - Trees treesInstance = NullAway.getTreesInstance(state); - TreePath path = treesInstance.getPath(enclosingSymbol); - return getTreeType(path.getParentPath().getLeaf()); + private static Type getTypeForAnonymousSymbol(Symbol enclosingSymbol, VisitorState state) { + Trees trees = NullAway.getTreesInstance(state); + // tree for the symbol will be a class tree + ClassTree anonClassTree = (ClassTree) trees.getTree(enclosingSymbol); + // This class must either extend one other class or implement one interface. Figure out which, + // and use the corresponding ParameterizedTypeTree to get the real type + Tree extendsClause = anonClassTree.getExtendsClause(); + ParameterizedTypeTree superTypeTree = null; + if (extendsClause != null) { + if (extendsClause instanceof ParameterizedTypeTree) { + superTypeTree = (ParameterizedTypeTree) extendsClause; + } + } else { + List implementsClause = anonClassTree.getImplementsClause(); + if (implementsClause.size() != 1) { + throw new RuntimeException( + "unexpected implements clause list " + + implementsClause + + " for anonymous type " + + anonClassTree); + } + Tree implementsTree = implementsClause.get(0); + if (implementsTree instanceof ParameterizedTypeTree) { + superTypeTree = (ParameterizedTypeTree) implementsTree; + } + } + if (superTypeTree != null && !superTypeTree.getTypeArguments().isEmpty()) { + return typeWithPreservedAnnotations(superTypeTree); + } else { + // not a generic type we need to modify + return enclosingSymbol.type; + } } /** From 05933410e970361130b17f91717695ecca64cc22 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 7 May 2023 11:03:46 -0700 Subject: [PATCH 26/48] Make all methods in GenericsChecks static --- .../com/uber/nullaway/GenericsChecks.java | 152 +++++++++++------- .../main/java/com/uber/nullaway/NullAway.java | 38 ++--- 2 files changed, 110 insertions(+), 80 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 3e74bbd7ca..defd9af13f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -40,15 +40,9 @@ public final class GenericsChecks { private static final Supplier NULLABLE_TYPE_SUPPLIER = Suppliers.typeFromString(NULLABLE_NAME); - private VisitorState state; - private Config config; - private NullAway analysis; - - public GenericsChecks(VisitorState state, Config config, NullAway analysis) { - this.state = state; - this.config = config; - this.analysis = analysis; - } + + /** Do not instantiate; all methods should be static */ + private GenericsChecks() {} /** * Checks that for an instantiated generic type, {@code @Nullable} types are only used for type @@ -173,7 +167,7 @@ private static void reportMismatchedTypeForTernaryOperator( errorMessage, analysis.buildDescription(tree), state, null)); } - private void reportInvalidParametersNullabilityError( + private static void reportInvalidParametersNullabilityError( Type formalParameterType, Type actualParameterType, ExpressionTree paramExpression, @@ -193,8 +187,12 @@ private void reportInvalidParametersNullabilityError( errorMessage, analysis.buildDescription(paramExpression), state, null)); } - private void reportInvalidOverridingMethodReturnTypeError( - Tree methodTree, Type typeParameterType, Type methodReturnType) { + private static void reportInvalidOverridingMethodReturnTypeError( + Tree methodTree, + Type typeParameterType, + Type methodReturnType, + NullAway analysis, + VisitorState state) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( @@ -209,8 +207,12 @@ private void reportInvalidOverridingMethodReturnTypeError( errorMessage, analysis.buildDescription(methodTree), state, null)); } - private void reportInvalidOverridingMethodParamTypeError( - Tree formalParameterTree, Type typeParameterType, Type methodParamType) { + private static void reportInvalidOverridingMethodParamTypeError( + Tree formalParameterTree, + Type typeParameterType, + Type methodParamType, + NullAway analysis, + VisitorState state) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( @@ -237,7 +239,7 @@ private void reportInvalidOverridingMethodParamTypeError( * @return Type of the tree with preserved annotations. */ @Nullable - private Type getTreeType(Tree tree) { + private static Type getTreeType(Tree tree, VisitorState state) { if (tree instanceof NewClassTree && ((NewClassTree) tree).getIdentifier() instanceof ParameterizedTypeTree) { ParameterizedTypeTree paramTypedTree = @@ -247,7 +249,7 @@ private Type getTreeType(Tree tree) { // TODO: support diamond operators return null; } - return typeWithPreservedAnnotations(paramTypedTree); + return typeWithPreservedAnnotations(paramTypedTree, state); } else { return ASTHelpers.getType(tree); } @@ -262,8 +264,9 @@ private Type getTreeType(Tree tree) { * @param tree the tree to check, which must be either an {@link AssignmentTree} or a {@link * VariableTree} */ - public void checkTypeParameterNullnessForAssignability(Tree tree) { - if (!config.isJSpecifyMode()) { + public static void checkTypeParameterNullnessForAssignability( + Tree tree, NullAway analysis, VisitorState state) { + if (!analysis.getConfig().isJSpecifyMode()) { return; } Tree lhsTree; @@ -282,21 +285,24 @@ public void checkTypeParameterNullnessForAssignability(Tree tree) { if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { return; } - Type lhsType = getTreeType(lhsTree); - Type rhsType = getTreeType(rhsTree); + Type lhsType = getTreeType(lhsTree, state); + Type rhsType = getTreeType(rhsTree, state); if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) { boolean isAssignmentValid = - compareNullabilityAnnotations((Type.ClassType) lhsType, (Type.ClassType) rhsType); + compareNullabilityAnnotations((Type.ClassType) lhsType, (Type.ClassType) rhsType, state); if (!isAssignmentValid) { reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); } } } - public void checkTypeParameterNullnessForFunctionReturnType( - ExpressionTree retExpr, Symbol.MethodSymbol methodSymbol) { - if (!config.isJSpecifyMode()) { + public static void checkTypeParameterNullnessForFunctionReturnType( + ExpressionTree retExpr, + Symbol.MethodSymbol methodSymbol, + NullAway analysis, + VisitorState state) { + if (!analysis.getConfig().isJSpecifyMode()) { return; } @@ -305,12 +311,12 @@ public void checkTypeParameterNullnessForFunctionReturnType( if (formalReturnType.getTypeArguments().isEmpty()) { return; } - Type returnExpressionType = getTreeType(retExpr); + Type returnExpressionType = getTreeType(retExpr, state); if (formalReturnType instanceof Type.ClassType && returnExpressionType instanceof Type.ClassType) { boolean isReturnTypeValid = compareNullabilityAnnotations( - (Type.ClassType) formalReturnType, (Type.ClassType) returnExpressionType); + (Type.ClassType) formalReturnType, (Type.ClassType) returnExpressionType, state); if (!isReturnTypeValid) { reportInvalidReturnTypeError( retExpr, formalReturnType, returnExpressionType, state, analysis); @@ -329,7 +335,8 @@ public void checkTypeParameterNullnessForFunctionReturnType( * @param lhsType type for the lhs of the assignment * @param rhsType type for the rhs of the assignment */ - private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.ClassType rhsType) { + private static boolean compareNullabilityAnnotations( + Type.ClassType lhsType, Type.ClassType rhsType, VisitorState state) { Types types = state.getTypes(); // The base type of rhsType may be a subtype of lhsType's base type. In such cases, we must // compare lhsType against the supertype of rhsType with a matching base type. @@ -374,7 +381,7 @@ private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.Class // nested generics if (lhsTypeArgument.getTypeArguments().length() > 0) { if (!compareNullabilityAnnotations( - (Type.ClassType) lhsTypeArgument, (Type.ClassType) rhsTypeArgument)) { + (Type.ClassType) lhsTypeArgument, (Type.ClassType) rhsTypeArgument, state)) { return false; } } @@ -390,7 +397,8 @@ private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.Class * @param tree A parameterized typed tree for which we need class type with preserved annotations. * @return A Type with preserved annotations. */ - private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) { + private static Type.ClassType typeWithPreservedAnnotations( + ParameterizedTypeTree tree, VisitorState state) { Type.ClassType type = (Type.ClassType) ASTHelpers.getType(tree); Preconditions.checkNotNull(type); Type nullableType = NULLABLE_TYPE_SUPPLIER.get(state); @@ -430,7 +438,8 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) Type currentTypeArgType = castToNonNull(ASTHelpers.getType(curTypeArg)); if (currentTypeArgType.getTypeArguments().size() > 0) { // nested generic type; recursively preserve its nullability type argument annotations - currentTypeArgType = typeWithPreservedAnnotations((ParameterizedTypeTree) curTypeArg); + currentTypeArgType = + typeWithPreservedAnnotations((ParameterizedTypeTree) curTypeArg, state); } Type.ClassType newTypeArgType = (Type.ClassType) currentTypeArgType.cloneWithMetadata(typeMetadata); @@ -455,31 +464,32 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) * * @param tree A conditional expression tree to check */ - public void checkTypeParameterNullnessForConditionalExpression(ConditionalExpressionTree tree) { - if (!config.isJSpecifyMode()) { + public static void checkTypeParameterNullnessForConditionalExpression( + ConditionalExpressionTree tree, NullAway analysis, VisitorState state) { + if (!analysis.getConfig().isJSpecifyMode()) { return; } Tree truePartTree = tree.getTrueExpression(); Tree falsePartTree = tree.getFalseExpression(); - Type condExprType = getTreeType(tree); - Type truePartType = getTreeType(truePartTree); - Type falsePartType = getTreeType(falsePartTree); + Type condExprType = getTreeType(tree, state); + Type truePartType = getTreeType(truePartTree, state); + Type falsePartType = getTreeType(falsePartTree, state); // The condExpr type should be the least-upper bound of the true and false part types. To check // the nullability annotations, we check that the true and false parts are assignable to the // type of the whole expression if (condExprType instanceof Type.ClassType) { if (truePartType instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) condExprType, (Type.ClassType) truePartType)) { + (Type.ClassType) condExprType, (Type.ClassType) truePartType, state)) { reportMismatchedTypeForTernaryOperator( truePartTree, condExprType, truePartType, state, analysis); } } if (falsePartType instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) condExprType, (Type.ClassType) falsePartType)) { + (Type.ClassType) condExprType, (Type.ClassType) falsePartType, state)) { reportMismatchedTypeForTernaryOperator( falsePartTree, condExprType, falsePartType, state, analysis); } @@ -495,11 +505,13 @@ public void checkTypeParameterNullnessForConditionalExpression(ConditionalExpres * @param actualParams the actual parameters * @param isVarArgs true if the call is to a varargs method */ - public void compareGenericTypeParameterNullabilityForCall( + public static void compareGenericTypeParameterNullabilityForCall( List formalParams, List actualParams, - boolean isVarArgs) { - if (!config.isJSpecifyMode()) { + boolean isVarArgs, + NullAway analysis, + VisitorState state) { + if (!analysis.getConfig().isJSpecifyMode()) { return; } int n = formalParams.size(); @@ -511,11 +523,11 @@ public void compareGenericTypeParameterNullabilityForCall( for (int i = 0; i < n - 1; i++) { Type formalParameter = formalParams.get(i).type; if (!formalParameter.getTypeArguments().isEmpty()) { - Type actualParameter = getTreeType(actualParams.get(i)); + Type actualParameter = getTreeType(actualParams.get(i), state); if (formalParameter instanceof Type.ClassType && actualParameter instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) formalParameter, (Type.ClassType) actualParameter)) { + (Type.ClassType) formalParameter, (Type.ClassType) actualParameter, state)) { reportInvalidParametersNullabilityError( formalParameter, actualParameter, actualParams.get(i), state, analysis); } @@ -528,11 +540,11 @@ public void compareGenericTypeParameterNullabilityForCall( Type varargsElementType = varargsArrayType.elemtype; if (varargsElementType.getTypeArguments().size() > 0) { for (int i = formalParams.size() - 1; i < actualParams.size(); i++) { - Type actualParameter = getTreeType(actualParams.get(i)); + Type actualParameter = getTreeType(actualParams.get(i), state); if (varargsElementType instanceof Type.ClassType && actualParameter instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) varargsElementType, (Type.ClassType) actualParameter)) { + (Type.ClassType) varargsElementType, (Type.ClassType) actualParameter, state)) { reportInvalidParametersNullabilityError( varargsElementType, actualParameter, actualParams.get(i), state, analysis); } @@ -550,9 +562,13 @@ public void compareGenericTypeParameterNullabilityForCall( * @param overridingMethod A symbol of the overriding method * @param overriddenMethod A symbol of the overridden method */ - public void checkTypeParameterNullnessForMethodOverriding( - MethodTree tree, Symbol.MethodSymbol overridingMethod, Symbol.MethodSymbol overriddenMethod) { - if (!config.isJSpecifyMode()) { + public static void checkTypeParameterNullnessForMethodOverriding( + MethodTree tree, + Symbol.MethodSymbol overridingMethod, + Symbol.MethodSymbol overriddenMethod, + NullAway analysis, + VisitorState state) { + if (!analysis.getConfig().isJSpecifyMode()) { return; } // Obtain type parameters for the overridden method within the context of the overriding @@ -560,8 +576,10 @@ public void checkTypeParameterNullnessForMethodOverriding( Type methodWithTypeParams = state.getTypes().memberType(overridingMethod.owner.type, overriddenMethod); - checkTypeParameterNullnessForOverridingMethodReturnType(tree, methodWithTypeParams); - checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); + checkTypeParameterNullnessForOverridingMethodReturnType( + tree, methodWithTypeParams, analysis, state); + checkTypeParameterNullnessForOverridingMethodParameterType( + tree, methodWithTypeParams, analysis, state); } /** @@ -635,15 +653,20 @@ public static Nullness getGenericMethodReturnTypeNullness( * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not * invoke an instance method */ - public Nullness getGenericParameterNullnessAtInvocation( - int paramIndex, Symbol.MethodSymbol invokedMethodSymbol, MethodInvocationTree tree) { + public static Nullness getGenericParameterNullnessAtInvocation( + int paramIndex, + Symbol.MethodSymbol invokedMethodSymbol, + MethodInvocationTree tree, + VisitorState state, + Config config) { if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { return Nullness.NONNULL; } Type methodReceiverType = castToNonNull( ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); - return getGenericMethodParameterNullness(paramIndex, invokedMethodSymbol, methodReceiverType); + return getGenericMethodParameterNullness( + paramIndex, invokedMethodSymbol, methodReceiverType, state, config); } /** @@ -675,8 +698,12 @@ public Nullness getGenericParameterNullnessAtInvocation( * @return nullability of the relevant parameter type of {@code method} in the context of {@code * enclosingType} */ - public Nullness getGenericMethodParameterNullness( - int parameterIndex, Symbol.MethodSymbol method, Type enclosingType) { + public static Nullness getGenericMethodParameterNullness( + int parameterIndex, + Symbol.MethodSymbol method, + Type enclosingType, + VisitorState state, + Config config) { Type methodType = state.getTypes().memberType(enclosingType, method); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); @@ -690,8 +717,8 @@ public Nullness getGenericMethodParameterNullness( * @param tree tree for overriding method * @param overriddenMethodType type of the overridden method */ - private void checkTypeParameterNullnessForOverridingMethodParameterType( - MethodTree tree, Type overriddenMethodType) { + private static void checkTypeParameterNullnessForOverridingMethodParameterType( + MethodTree tree, Type overriddenMethodType, NullAway analysis, VisitorState state) { List methodParameters = tree.getParameters(); List typeParameterTypes = overriddenMethodType.getParameterTypes(); for (int i = 0; i < methodParameters.size(); i++) { @@ -700,9 +727,9 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( if (typeParameterType instanceof Type.ClassType && methodParameterType instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType)) { + (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType, state)) { reportInvalidOverridingMethodParamTypeError( - methodParameters.get(i), typeParameterType, methodParameterType); + methodParameters.get(i), typeParameterType, methodParameterType, analysis, state); } } } @@ -716,16 +743,17 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( * @param tree tree for overriding method * @param overriddenMethodType type of the overridden method */ - private void checkTypeParameterNullnessForOverridingMethodReturnType( - MethodTree tree, Type overriddenMethodType) { + private static void checkTypeParameterNullnessForOverridingMethodReturnType( + MethodTree tree, Type overriddenMethodType, NullAway analysis, VisitorState state) { Type typeParamType = overriddenMethodType.getReturnType(); Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { return; } if (!compareNullabilityAnnotations( - (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType)) { - reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); + (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType, state)) { + reportInvalidOverridingMethodReturnTypeError( + tree, typeParamType, methodReturnType, analysis, state); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 340fad556c..9c228854c2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -470,7 +470,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { } // generics check if (lhsType != null && lhsType.getTypeArguments().length() > 0) { - new GenericsChecks(state, config, this).checkTypeParameterNullnessForAssignability(tree); + GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } Symbol assigned = ASTHelpers.getSymbol(tree.getVariable()); @@ -628,9 +628,8 @@ public Description matchMethod(MethodTree tree, VisitorState state) { NullabilityUtil.getClosestOverriddenMethod(methodSymbol, state.getTypes()); if (closestOverriddenMethod != null) { if (config.isJSpecifyMode()) { - new GenericsChecks(state, config, this) - .checkTypeParameterNullnessForMethodOverriding( - tree, methodSymbol, closestOverriddenMethod); + GenericsChecks.checkTypeParameterNullnessForMethodOverriding( + tree, methodSymbol, closestOverriddenMethod, this, state); } return checkOverriding(closestOverriddenMethod, methodSymbol, null, state); } @@ -728,9 +727,12 @@ private Description checkParamOverriding( Nullness.paramHasNullableAnnotation(overriddenMethod, i, config) ? Nullness.NULLABLE : (config.isJSpecifyMode() - ? new GenericsChecks(state, config, this) - .getGenericMethodParameterNullness( - i, overriddenMethod, overridingParamSymbols.get(i).owner.owner.type) + ? GenericsChecks.getGenericMethodParameterNullness( + i, + overriddenMethod, + overridingParamSymbols.get(i).owner.owner.type, + state, + config) : Nullness.NONNULL); } } @@ -845,8 +847,8 @@ private Description checkReturnExpression( state, methodSymbol); } - new GenericsChecks(state, config, this) - .checkTypeParameterNullnessForFunctionReturnType(retExpr, methodSymbol); + GenericsChecks.checkTypeParameterNullnessForFunctionReturnType( + retExpr, methodSymbol, this, state); return Description.NO_MATCH; } @@ -1358,7 +1360,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } VarSymbol symbol = ASTHelpers.getSymbol(tree); if (tree.getInitializer() != null) { - new GenericsChecks(state, config, this).checkTypeParameterNullnessForAssignability(tree); + GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } if (symbol.type.isPrimitive() && tree.getInitializer() != null) { @@ -1510,8 +1512,7 @@ public Description matchUnary(UnaryTree tree, VisitorState state) { public Description matchConditionalExpression( ConditionalExpressionTree tree, VisitorState state) { if (withinAnnotatedCode(state)) { - new GenericsChecks(state, config, this) - .checkTypeParameterNullnessForConditionalExpression(tree); + GenericsChecks.checkTypeParameterNullnessForConditionalExpression(tree, this, state); doUnboxingCheck(state, tree.getCondition()); } return Description.NO_MATCH; @@ -1637,15 +1638,13 @@ private Description handleInvocation( Nullness.paramHasNullableAnnotation(methodSymbol, i, config) ? Nullness.NULLABLE : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) - ? new GenericsChecks(state, config, this) - .getGenericParameterNullnessAtInvocation( - i, methodSymbol, (MethodInvocationTree) tree) + ? GenericsChecks.getGenericParameterNullnessAtInvocation( + i, methodSymbol, (MethodInvocationTree) tree, state, config) : Nullness.NONNULL); } } - new GenericsChecks(state, config, this) - .compareGenericTypeParameterNullabilityForCall( - formalParams, actualParams, methodSymbol.isVarArgs()); + GenericsChecks.compareGenericTypeParameterNullabilityForCall( + formalParams, actualParams, methodSymbol.isVarArgs(), this, state); } // Allow handlers to override the list of non-null argument positions @@ -2401,6 +2400,9 @@ public ErrorBuilder getErrorBuilder() { return errorBuilder; } + public Config getConfig() { + return config; + } /** * strip out enclosing parentheses, type casts and Nullchk operators. * From 1298081e0966b71ca6a9307fa71e751258be8e11 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 7 May 2023 16:11:33 -0700 Subject: [PATCH 27/48] more, still need tests for conditional expressions as receivers --- .../com/uber/nullaway/GenericsChecks.java | 34 ++++++++++++++----- .../AccessPathNullnessPropagation.java | 2 +- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index edecd82393..df9daa3af8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -616,10 +616,31 @@ public static void checkTypeParameterNullnessForMethodOverriding( */ public static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) { - Type enclosingType = - enclosingSymbol.isAnonymous() - ? getTypeForAnonymousSymbol(enclosingSymbol, state) - : enclosingSymbol.type; + Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + return getGenericMethodReturnTypeNullness(method, enclosingType, state, config); + } + + private static Type getTypeForSymbol(Symbol enclosingSymbol, VisitorState state) { + return enclosingSymbol.isAnonymous() + ? getTypeForAnonymousSymbol(enclosingSymbol, state) + : enclosingSymbol.type; + } + + public static Nullness getGenericMethodReturnTypeNullness( + Symbol.MethodSymbol method, Tree tree, VisitorState state, Config config) { + if (method.isStatic()) { + return Nullness.NONNULL; + } + Symbol symbol = ASTHelpers.getSymbol(tree); + if (symbol != null) { + return getGenericMethodReturnTypeNullness(method, symbol, state, config); + } + return getGenericMethodReturnTypeNullness( + method, castToNonNull(ASTHelpers.getType(tree)), state, config); + } + + private static Nullness getGenericMethodReturnTypeNullness( + Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { throw new RuntimeException("expected method type but instead got " + overriddenMethodType); @@ -710,10 +731,7 @@ public static Nullness getGenericMethodParameterNullness( Symbol enclosingSymbol, VisitorState state, Config config) { - Type enclosingType = - enclosingSymbol.isAnonymous() - ? getTypeForAnonymousSymbol(enclosingSymbol, state) - : enclosingSymbol.type; + Type enclosingType = getTypeForSymbol(enclosingSymbol, state); Type methodType = state.getTypes().memberType(enclosingType, method); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index c3c077dccd..fca8dd89c7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -1034,7 +1034,7 @@ private boolean genericReturnIsNullable(MethodInvocationNode node) { Nullness nullness = GenericsChecks.getGenericMethodReturnTypeNullness( (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree()), - (Type) node.getTarget().getReceiver().getType(), + node.getTarget().getReceiver().getTree(), state, config); return nullness.equals(NULLABLE); From 662606a36804a72f0a5febc59561f07828694f7e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 14 May 2023 16:13:38 -0700 Subject: [PATCH 28/48] fix nullaway error --- .../uber/nullaway/dataflow/AccessPathNullnessPropagation.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index c3c077dccd..b7180a68ea 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -1033,7 +1033,8 @@ private boolean genericReturnIsNullable(MethodInvocationNode node) { if (node.getTarget() != null && node.getTarget().getMethod() != null) { Nullness nullness = GenericsChecks.getGenericMethodReturnTypeNullness( - (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree()), + castToNonNull( + (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree())), (Type) node.getTarget().getReceiver().getType(), state, config); From 08596f0b090afb40603ea7b961086a5e407dcc2f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 16:23:34 -0700 Subject: [PATCH 29/48] fix class ordering in test --- .../java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index e449d40dc3..d1a939c1d1 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -737,14 +737,14 @@ public void overrideReturnTypes() { " return s;", " }", " }", - " static class TestFunc4 implements Fn<@Nullable String, String> {", + " static class TestFunc3 implements Fn {", " @Override", " // BUG: Diagnostic contains: method returns @Nullable, but superclass", " public @Nullable String apply(String s) {", " return s;", " }", " }", - " static class TestFunc3 implements Fn {", + " static class TestFunc4 implements Fn<@Nullable String, String> {", " @Override", " // BUG: Diagnostic contains: method returns @Nullable, but superclass", " public @Nullable String apply(String s) {", From 97ec62a80be35e1a013cf275e83e4e83a7de24fa Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:07:00 -0700 Subject: [PATCH 30/48] bug fix: did not handle direct dereference of method call expression with generic @Nullable return type --- .../com/uber/nullaway/GenericsChecks.java | 46 +++++++++++++++++++ .../main/java/com/uber/nullaway/NullAway.java | 13 ++++-- .../NullAwayJSpecifyGenericsTests.java | 2 + 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 3e74bbd7ca..462ab3b94f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -603,6 +603,52 @@ public static Nullness getGenericMethodReturnTypeNullness( return getTypeNullness(overriddenMethodType.getReturnType(), config); } + /** + * Computes the nullness of the return of a generic method at an invocation, in the context of the + * declared type of its receiver argument. If the return type is a type variable, its nullness + * depends on the nullability of the corresponding type parameter in the receiver's type. + * + *

Consider the following example: + * + * 

+   *     interface Fn
 {
+   *         R apply(P p);
+   *     }
+   *     class C implements Fn {
+   *         public @Nullable String apply(String p) {
+   *             return null;
+   *         }
+   *     }
+   *     static void m() {
+   *         Fn f = new C();
+   *         f.apply("hello").hashCode(); // NPE
+   *     }
+   *
+ * + * The declared type of {@code f} passes {@code Nullable String} as the type parameter for type + * variable {@code R}. So, the call {@code f.apply("hello")} returns {@code @Nullable} and an + * error should be reported. + * + * @param invokedMethodSymbol symbol for the invoked method + * @param tree the tree for the invocation + * @return Nullness of invocation's return type, or {@code NONNULL} if the call does not invoke an + * instance method + */ + public static Nullness getGenericReturnNullnessAtInvocation( + Symbol.MethodSymbol invokedMethodSymbol, + MethodInvocationTree tree, + VisitorState state, + Config config) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { + return Nullness.NONNULL; + } + Type methodReceiverType = + castToNonNull( + ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + return getGenericMethodReturnTypeNullness( + invokedMethodSymbol, methodReceiverType, state, config); + } + /** * 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 diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index d032408714..539eb22074 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -2309,7 +2309,9 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) { + " for method invocation " + state.getSourceForNode(expr)); } - exprMayBeNull = mayBeNullMethodCall((Symbol.MethodSymbol) exprSymbol); + exprMayBeNull = + mayBeNullMethodCall( + (Symbol.MethodSymbol) exprSymbol, (MethodInvocationTree) expr, state); break; case CONDITIONAL_EXPRESSION: case ASSIGNMENT: @@ -2328,11 +2330,16 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) { return exprMayBeNull && nullnessFromDataflow(state, expr); } - private boolean mayBeNullMethodCall(Symbol.MethodSymbol exprSymbol) { + private boolean mayBeNullMethodCall( + Symbol.MethodSymbol exprSymbol, MethodInvocationTree invocationTree, VisitorState state) { if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config)) { return false; } - return Nullness.hasNullableAnnotation(exprSymbol, config); + return Nullness.hasNullableAnnotation(exprSymbol, config) + || (config.isJSpecifyMode() + && GenericsChecks.getGenericReturnNullnessAtInvocation( + exprSymbol, invocationTree, state, config) + .equals(Nullness.NULLABLE)); } public boolean nullnessFromDataflow(VisitorState state, ExpressionTree expr) { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index d1a939c1d1..25dde321a4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -764,6 +764,8 @@ public void overrideReturnTypes() { " String t3 = f3.apply(s);", " // BUG: Diagnostic contains: dereferenced expression", " t3.hashCode();", + " // BUG: Diagnostic contains: dereferenced expression", + " f3.apply(s).hashCode();", " }", "}") .doTest(); From 8bca16e16692067b11eb27907ba791cb36e2a9d0 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:12:13 -0700 Subject: [PATCH 31/48] test interactions with @Contract --- .../NullAwayJSpecifyGenericsTests.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 25dde321a4..7bdc787e85 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -930,6 +930,51 @@ public void overrideWithNestedTypeParametersInParameterType() { .doTest(); } + @Test + public void interactionWithContracts() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import org.jetbrains.annotations.Contract;", + "class Test {", + " interface Fn1

{", + " R apply(P p);", + " }", + " static class TestFunc1 implements Fn1<@Nullable String, @Nullable String> {", + " @Override", + " @Contract(\"!null -> !null\")", + " public @Nullable String apply(@Nullable String s) {", + " return s;", + " }", + " }", + " interface Fn2

{", + " @Contract(\"!null -> !null\")", + " R apply(P p);", + " }", + " static class TestFunc2 implements Fn2<@Nullable String, @Nullable String> {", + " @Override", + " public @Nullable String apply(@Nullable String s) {", + " return s;", + " }", + " }", + " static void testMethod() {", + " // No error due to @Contract", + " (new TestFunc1()).apply(\"hello\").hashCode();", + " Fn1<@Nullable String, @Nullable String> fn1 = new TestFunc1();", + " // BUG: Diagnostic contains: dereferenced expression fn1.apply(\"hello\")", + " fn1.apply(\"hello\").hashCode();", + " // BUG: Diagnostic contains: dereferenced expression (new TestFunc2())", + " (new TestFunc2()).apply(\"hello\").hashCode();", + " Fn2<@Nullable String, @Nullable String> fn2 = new TestFunc2();", + " // No error due to @Contract", + " fn2.apply(\"hello\").hashCode();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 242b2e057a6c8386ab90ed3834ee3eb7f1e87546 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:22:32 -0700 Subject: [PATCH 32/48] simplify using new API --- .../dataflow/AccessPathNullnessPropagation.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index b7180a68ea..b954c72514 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -28,6 +28,7 @@ import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.suppliers.Suppliers; import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.MethodInvocationTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeTag; @@ -1030,14 +1031,11 @@ Nullness returnValueNullness( */ private boolean genericReturnIsNullable(MethodInvocationNode node) { if (node != null && config.isJSpecifyMode()) { - if (node.getTarget() != null && node.getTarget().getMethod() != null) { + MethodInvocationTree tree = node.getTree(); + if (tree != null) { Nullness nullness = - GenericsChecks.getGenericMethodReturnTypeNullness( - castToNonNull( - (Symbol.MethodSymbol) ASTHelpers.getSymbol(node.getTarget().getTree())), - (Type) node.getTarget().getReceiver().getType(), - state, - config); + GenericsChecks.getGenericReturnNullnessAtInvocation( + ASTHelpers.getSymbol(tree), tree, state, config); return nullness.equals(NULLABLE); } } From 631c417240c22e69351b01e7604d5724a9fb8add Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:37:41 -0700 Subject: [PATCH 33/48] better variable names --- .../java/com/uber/nullaway/GenericsChecks.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 462ab3b94f..3e24c8f02b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -194,15 +194,15 @@ private void reportInvalidParametersNullabilityError( } private void reportInvalidOverridingMethodReturnTypeError( - Tree methodTree, Type typeParameterType, Type methodReturnType) { + Tree methodTree, Type overriddenMethodReturnType, Type overridingMethodReturnType) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); ErrorMessage errorMessage = new ErrorMessage( ErrorMessage.MessageTypes.WRONG_OVERRIDE_RETURN_GENERIC, "Method returns " - + prettyTypeForError(methodReturnType) + + prettyTypeForError(overridingMethodReturnType) + ", but overridden method returns " - + prettyTypeForError(typeParameterType) + + prettyTypeForError(overriddenMethodReturnType) + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( @@ -764,14 +764,16 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( */ private void checkTypeParameterNullnessForOverridingMethodReturnType( MethodTree tree, Type overriddenMethodType) { - Type typeParamType = overriddenMethodType.getReturnType(); - Type methodReturnType = ASTHelpers.getType(tree.getReturnType()); - if (!(typeParamType instanceof Type.ClassType && methodReturnType instanceof Type.ClassType)) { + Type overriddenMethodReturnType = overriddenMethodType.getReturnType(); + Type overridingMethodReturnType = ASTHelpers.getType(tree.getReturnType()); + if (!(overriddenMethodReturnType instanceof Type.ClassType + && overridingMethodReturnType instanceof Type.ClassType)) { return; } if (!compareNullabilityAnnotations( - (Type.ClassType) typeParamType, (Type.ClassType) methodReturnType)) { - reportInvalidOverridingMethodReturnTypeError(tree, typeParamType, methodReturnType); + (Type.ClassType) overriddenMethodReturnType, (Type.ClassType) overridingMethodReturnType)) { + reportInvalidOverridingMethodReturnTypeError( + tree, overriddenMethodReturnType, overridingMethodReturnType); } } From 9024a67b4a6dd51e511a933d75d0260b36f6d3bc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:42:04 -0700 Subject: [PATCH 34/48] rename variables and add a TODO --- .../java/com/uber/nullaway/GenericsChecks.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 3e24c8f02b..975d7d2e71 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -739,16 +739,20 @@ public Nullness getGenericMethodParameterNullness( private void checkTypeParameterNullnessForOverridingMethodParameterType( MethodTree tree, Type overriddenMethodType) { List methodParameters = tree.getParameters(); - List typeParameterTypes = overriddenMethodType.getParameterTypes(); + List overriddenMethodParameterTypes = overriddenMethodType.getParameterTypes(); + // TODO handle varargs; they are not handled for now for (int i = 0; i < methodParameters.size(); i++) { - Type methodParameterType = ASTHelpers.getType(methodParameters.get(i)); - Type typeParameterType = typeParameterTypes.get(i); - if (typeParameterType instanceof Type.ClassType - && methodParameterType instanceof Type.ClassType) { + Type overridingMethodParameterType = ASTHelpers.getType(methodParameters.get(i)); + Type overriddenMethodParameterType = overriddenMethodParameterTypes.get(i); + if (overriddenMethodParameterType instanceof Type.ClassType + && overridingMethodParameterType instanceof Type.ClassType) { if (!compareNullabilityAnnotations( - (Type.ClassType) typeParameterType, (Type.ClassType) methodParameterType)) { + (Type.ClassType) overriddenMethodParameterType, + (Type.ClassType) overridingMethodParameterType)) { reportInvalidOverridingMethodParamTypeError( - methodParameters.get(i), typeParameterType, methodParameterType); + methodParameters.get(i), + overriddenMethodParameterType, + overridingMethodParameterType); } } } From be06ff1a784775fa2c93f92827cd59cb30c03c5a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 14 Jun 2023 17:45:31 -0700 Subject: [PATCH 35/48] add a comment --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 539eb22074..741e3408c2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -628,6 +628,8 @@ public Description matchMethod(MethodTree tree, VisitorState state) { NullabilityUtil.getClosestOverriddenMethod(methodSymbol, state.getTypes()); if (closestOverriddenMethod != null) { if (config.isJSpecifyMode()) { + // Check that any generic type parameters in the return type and parameter types are + // identical (invariant) across the overriding and overridden methods new GenericsChecks(state, config, this) .checkTypeParameterNullnessForMethodOverriding( tree, methodSymbol, closestOverriddenMethod); From 1ad0952b830d925f7b00809c659300089916b1dc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 15:55:28 -0700 Subject: [PATCH 36/48] bug fix --- .../src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 1855062004..0fe6b74a30 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -639,7 +639,7 @@ public static Nullness getGenericMethodReturnTypeNullness( method, castToNonNull(ASTHelpers.getType(tree)), state, config); } - public static Nullness getGenericMethodReturnTypeNullness( + private static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 2b6d519b2e..25c2b5c26b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -938,7 +938,7 @@ private Description checkOverriding( // if the super method returns nonnull, overriding method better not return nullable // Note that, for the overriding method, the permissive default is non-null, // but it's nullable for the overridden one. - if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner.type, state) + if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner, state) && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) .equals(Nullness.NULLABLE) && (memberReferenceTree == null @@ -977,7 +977,7 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) } private boolean overriddenMethodReturnsNonNull( - Symbol.MethodSymbol overriddenMethod, Type overridingMethodType, VisitorState state) { + Symbol.MethodSymbol overriddenMethod, Symbol enclosingSymbol, VisitorState state) { Nullness methodReturnNullness = getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE); if (!methodReturnNullness.equals(Nullness.NONNULL)) { @@ -987,7 +987,7 @@ private boolean overriddenMethodReturnsNonNull( // using the type parameters from the type enclosing the overriding method return !config.isJSpecifyMode() || GenericsChecks.getGenericMethodReturnTypeNullness( - overriddenMethod, overridingMethodType, state, config) + overriddenMethod, enclosingSymbol, state, config) .equals(Nullness.NONNULL); } From 73335292d916b12abbca37029eb631c3e9c09d3c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 16:00:04 -0700 Subject: [PATCH 37/48] delete unused method --- .../main/java/com/uber/nullaway/GenericsChecks.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 0fe6b74a30..6c53ee8943 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -626,19 +626,6 @@ private static Type getTypeForSymbol(Symbol enclosingSymbol, VisitorState state) : enclosingSymbol.type; } - public static Nullness getGenericMethodReturnTypeNullness( - Symbol.MethodSymbol method, Tree tree, VisitorState state, Config config) { - if (method.isStatic()) { - return Nullness.NONNULL; - } - Symbol symbol = ASTHelpers.getSymbol(tree); - if (symbol != null) { - return getGenericMethodReturnTypeNullness(method, symbol, state, config); - } - return getGenericMethodReturnTypeNullness( - method, castToNonNull(ASTHelpers.getType(tree)), state, config); - } - private static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); From 09a26795945be7fe1450c800ef24d4cb85131af0 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 16:15:08 -0700 Subject: [PATCH 38/48] failing test --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- .../java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 6c53ee8943..dbfa663ef3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -676,7 +676,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( } Type methodReceiverType = castToNonNull( - ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); return getGenericMethodReturnTypeNullness( invokedMethodSymbol, methodReceiverType, state, config); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 93780a800a..a0412d206d 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -878,6 +878,10 @@ public void overrideExplicitlyTypedAnonymousClass() { " FnClass fn4 = new FnClass() {", " public @Nullable String apply(String s) { return null; }", " };", + " String s1 = (new Fn() {", + " public @Nullable String apply(String s) { return null; }", + " }).apply(\"hi\");", + " s1.hashCode();", " }", "}") .doTest(); From d0aa308e434736fc184fbdb4204b26cc88712eb7 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 17:08:00 -0700 Subject: [PATCH 39/48] fixes --- .../com/uber/nullaway/GenericsChecks.java | 15 +++++-- .../NullAwayJSpecifyGenericsTests.java | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index dbfa663ef3..360d2b0410 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -722,11 +722,11 @@ public static Nullness getGenericParameterNullnessAtInvocation( if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { return Nullness.NONNULL; } - Symbol methodReceiverSymbol = + Type enclosingType = castToNonNull( - ASTHelpers.getSymbol(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); return getGenericMethodParameterNullness( - paramIndex, invokedMethodSymbol, methodReceiverSymbol, state, config); + paramIndex, invokedMethodSymbol, enclosingType, state, config); } /** @@ -765,6 +765,15 @@ public static Nullness getGenericMethodParameterNullness( VisitorState state, Config config) { Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config); + } + + public static Nullness getGenericMethodParameterNullness( + int parameterIndex, + Symbol.MethodSymbol method, + Type enclosingType, + VisitorState state, + Config config) { Type methodType = state.getTypes().memberType(enclosingType, method); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index a0412d206d..0733f0eafe 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -878,10 +878,50 @@ public void overrideExplicitlyTypedAnonymousClass() { " FnClass fn4 = new FnClass() {", " public @Nullable String apply(String s) { return null; }", " };", + " }", + "}") + .doTest(); + } + + @Test + public void explicitlyTypedAnonymousClassAsReceiver() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", + " static void anonymousClasses() {", " String s1 = (new Fn() {", " public @Nullable String apply(String s) { return null; }", " }).apply(\"hi\");", + " // BUG: Diagnostic contains: dereferenced expression s1", " s1.hashCode();", + " String s2 = (new FnClass() {", + " public @Nullable String apply(String s) { return null; }", + " }).apply(\"hi\");", + " // BUG: Diagnostic contains: dereferenced expression s2", + " s2.hashCode();", + " (new Fn() {", + " public String apply(String s) { return \"hi\"; }", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " }).apply(null);", + " (new FnClass() {", + " public String apply(String s) { return \"hi\"; }", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " }).apply(null);", + " (new Fn<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hi\"; }", + " }).apply(null);", + " (new FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hi\"; }", + " }).apply(null);", " }", "}") .doTest(); From 12e78ea03001b173bfe1bfee34fae1526145171e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 17:22:01 -0700 Subject: [PATCH 40/48] big simplification --- .../com/uber/nullaway/GenericsChecks.java | 51 +++++-------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 360d2b0410..c96b13e6c0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -11,7 +11,6 @@ import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; -import com.sun.source.tree.ClassTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MemberSelectTree; @@ -21,7 +20,7 @@ import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; -import com.sun.source.util.Trees; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; @@ -621,9 +620,17 @@ public static Nullness getGenericMethodReturnTypeNullness( } private static Type getTypeForSymbol(Symbol enclosingSymbol, VisitorState state) { - return enclosingSymbol.isAnonymous() - ? getTypeForAnonymousSymbol(enclosingSymbol, state) - : enclosingSymbol.type; + if (enclosingSymbol.isAnonymous()) { + TreePath path = state.getPath(); + NewClassTree newClassTree = ASTHelpers.findEnclosingNode(path, NewClassTree.class); + if (newClassTree == null) { + throw new RuntimeException( + "method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf())); + } + return getTreeType(newClassTree, state); + } else { + return enclosingSymbol.type; + } } private static Nullness getGenericMethodReturnTypeNullness( @@ -779,40 +786,6 @@ public static Nullness getGenericMethodParameterNullness( return getTypeNullness(paramType, config); } - private static Type getTypeForAnonymousSymbol(Symbol enclosingSymbol, VisitorState state) { - Trees trees = NullAway.getTreesInstance(state); - // tree for the symbol will be a class tree - ClassTree anonClassTree = (ClassTree) trees.getTree(enclosingSymbol); - // This class must either extend one other class or implement one interface. Figure out which, - // and use the corresponding ParameterizedTypeTree to get the real type - Tree extendsClause = anonClassTree.getExtendsClause(); - ParameterizedTypeTree superTypeTree = null; - if (extendsClause != null) { - if (extendsClause instanceof ParameterizedTypeTree) { - superTypeTree = (ParameterizedTypeTree) extendsClause; - } - } else { - List implementsClause = anonClassTree.getImplementsClause(); - if (implementsClause.size() != 1) { - throw new RuntimeException( - "unexpected implements clause list " - + implementsClause - + " for anonymous type " - + anonClassTree); - } - Tree implementsTree = implementsClause.get(0); - if (implementsTree instanceof ParameterizedTypeTree) { - superTypeTree = (ParameterizedTypeTree) implementsTree; - } - } - if (superTypeTree != null && !superTypeTree.getTypeArguments().isEmpty()) { - return typeWithPreservedAnnotations(superTypeTree, state); - } else { - // not a generic type we need to modify - return enclosingSymbol.type; - } - } - /** * This method compares the type parameter annotations for overriding method parameters with * corresponding type parameters for the overridden method and reports an error if they don't From 2e36bdea4fd7a4fdd1fb44c3619f4a47b3eae92c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 21 Jun 2023 18:03:02 -0700 Subject: [PATCH 41/48] comments --- .../com/uber/nullaway/GenericsChecks.java | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index c96b13e6c0..2a992caa73 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -619,8 +619,17 @@ public static Nullness getGenericMethodReturnTypeNullness( return getGenericMethodReturnTypeNullness(method, enclosingType, state, config); } - private static Type getTypeForSymbol(Symbol enclosingSymbol, VisitorState state) { - if (enclosingSymbol.isAnonymous()) { + /** + * Get the type for the symbol, accounting for anonymous classes + * + * @param symbol the symbol + * @param state the visitor state + * @return the type for {@code symbol} + */ + private static Type getTypeForSymbol(Symbol symbol, VisitorState state) { + if (symbol.isAnonymous()) { + // For anonymous classes, symbol.type does not contain annotations on generic type parameters. + // So, we get a correct type from the enclosing NewClassTree. TreePath path = state.getPath(); NewClassTree newClassTree = ASTHelpers.findEnclosingNode(path, NewClassTree.class); if (newClassTree == null) { @@ -629,7 +638,7 @@ private static Type getTypeForSymbol(Symbol enclosingSymbol, VisitorState state) } return getTreeType(newClassTree, state); } else { - return enclosingSymbol.type; + return symbol.type; } } @@ -760,10 +769,12 @@ public static Nullness getGenericParameterNullnessAtInvocation( * * @param parameterIndex index of the parameter * @param method the generic method - * @param enclosingSymbol the enclosing type in which we want to know {@code method}'s parameter + * @param enclosingSymbol the enclosing symbol in which we want to know {@code method}'s parameter * type nullability + * @param state the visitor state + * @param config the config * @return nullability of the relevant parameter type of {@code method} in the context of {@code - * enclosingType} + * enclosingSymbol} */ public static Nullness getGenericMethodParameterNullness( int parameterIndex, @@ -775,6 +786,20 @@ public static Nullness getGenericMethodParameterNullness( return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config); } + /** + * Just like {@link #getGenericMethodParameterNullness(int, Symbol.MethodSymbol, Symbol, + * VisitorState, Config)}, but takes the enclosing {@code Type} rather than the enclosing {@code + * Symbol}. + * + * @param parameterIndex index of the parameter + * @param method the generic method + * @param enclosingType the enclosing type in which we want to know {@code method}'s parameter + * type nullability + * @param state the visitor state + * @param config the config + * @return nullability of the relevant parameter type of {@code method} in the context of {@code + * enclosingSymbol} + */ public static Nullness getGenericMethodParameterNullness( int parameterIndex, Symbol.MethodSymbol method, From 033b081e1784e3ce07cd4c0da1d85cb69bf6fe41 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 13 Aug 2023 11:22:24 -0700 Subject: [PATCH 42/48] add test case --- .../NullAwayJSpecifyGenericsTests.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 2a22d2eca3..64a3fcab50 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -973,6 +973,45 @@ public void explicitlyTypedAnonymousClassAsReceiver() { .doTest(); } + /** Diamond anonymous classes are not supported yet; tests are for future reference */ + @Test + public void overrideDiamondAnonymousClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", + " static void anonymousClasses() {", + " Fn<@Nullable String, String> fn1 = new Fn<>() {", + " // TODO: should report a bug here", + " public String apply(String s) { return s; }", + " };", + " FnClass<@Nullable String, String> fn2 = new FnClass<>() {", + " // TODO: should report a bug here", + " public String apply(String s) { return s; }", + " };", + " Fn fn3 = new Fn<>() {", + " // TODO: this is a false positive", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " FnClass fn4 = new FnClass<>() {", + " // TODO: this is a false positive", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " }", + "}") + .doTest(); + } + @Test public void nullableGenericTypeVariableReturnType() { makeHelper() From f2660068a1815b299943b2e2abfaa215725392bf Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 18 Aug 2023 10:28:07 -0400 Subject: [PATCH 43/48] fix comment --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 37c724ec1e..2ec7b826e1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -836,7 +836,7 @@ public static Nullness getGenericMethodParameterNullness( * @param state the visitor state * @param config the analysis config * @return nullability of the relevant parameter type of {@code method} in the context of {@code - * enclosingSymbol} + * enclosingType} */ public static Nullness getGenericMethodParameterNullness( int parameterIndex, From 9a981dc49d07ef72eb4c4f901f7e6dff3057ad3c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 18 Aug 2023 10:48:09 -0400 Subject: [PATCH 44/48] exclude test --- nullaway/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nullaway/build.gradle b/nullaway/build.gradle index b9811c1267..8bc60a4012 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -142,6 +142,10 @@ def jdk8Test = tasks.register("testJdk8", Test) { classpath = testTask.classpath testClassesDirs = testTask.testClassesDirs jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}" + filter { + // JDK 8 does not support diamonds on anonymous classes + excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests.overrideDiamondAnonymousClass" + } } tasks.named('check').configure { From 778447e8ffeab85afe4f0b2fac9f4f28cd11a6fb Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 16:40:38 -0700 Subject: [PATCH 45/48] test more cases --- .../uber/nullaway/NullAwayJSpecifyGenericsTests.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index f52f795d99..67ba2b7813 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -993,15 +993,15 @@ public void overrideExplicitlyTypedAnonymousClass() { " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", " public String apply(String s) { return s; }", " };", - " FnClass<@Nullable String, String> fn2 = new FnClass<@Nullable String, String>() {", - " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", - " public String apply(String s) { return s; }", + " FnClass fn2 = new FnClass() {", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", " };", " Fn fn3 = new Fn() {", " public @Nullable String apply(String s) { return null; }", " };", - " FnClass fn4 = new FnClass() {", - " public @Nullable String apply(String s) { return null; }", + " FnClass<@Nullable String, String> fn4 = new FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hello\"; }", " };", " }", "}") From af6ca1fb5da1a0e9b6e4db4e3d805e6dd7d0bb70 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 16:49:50 -0700 Subject: [PATCH 46/48] check that type is in fact a supertype --- .../src/main/java/com/uber/nullaway/GenericsChecks.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 93d6d7c628..fa99a91775 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -4,6 +4,7 @@ import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.errorprone.VisitorState; import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.suppliers.Suppliers; @@ -723,7 +724,11 @@ private static Type getTypeForSymbol(Symbol symbol, VisitorState state) { throw new RuntimeException( "method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf())); } - return getTreeType(newClassTree, state); + Type typeFromTree = getTreeType(newClassTree, state); + if (typeFromTree != null) { + Verify.verify(state.getTypes().isAssignable(symbol.type, typeFromTree)); + } + return typeFromTree; } else { return symbol.type; } From 16eb9c2f6f531939617dbf3f765a1e8d6557375e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 17:30:44 -0700 Subject: [PATCH 47/48] add tests --- .../NullAwayJSpecifyGenericsTests.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 67ba2b7813..328bdd11eb 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -2,6 +2,7 @@ import com.google.errorprone.CompilationTestHelper; import java.util.Arrays; +import org.junit.Ignore; import org.junit.Test; public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase { @@ -1004,6 +1005,46 @@ public void overrideExplicitlyTypedAnonymousClass() { " public String apply(@Nullable String s) { return \"hello\"; }", " };", " }", + " static void anonymousClassesFullName() {", + " Test.Fn<@Nullable String, String> fn1 = new Test.Fn<@Nullable String, String>() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " Test.FnClass fn2 = new Test.FnClass() {", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " Test.Fn fn3 = new Test.Fn() {", + " public @Nullable String apply(String s) { return null; }", + " };", + " Test.FnClass<@Nullable String, String> fn4 = new Test.FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hello\"; }", + " };", + " }", + "}") + .doTest(); + } + + @Ignore("Need to add support for this case") + @Test + public void overrideAnonymousNestedClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class Wrapper

{", + " abstract class Fn {", + " abstract R apply(P p);", + " }", + " }", + " void anonymousNestedClasses() {", + " Wrapper<@Nullable String>.Fn fn1 = (this.new Wrapper<@Nullable String>()).new Fn() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " }", "}") .doTest(); } From 26085c490b1e8078b12f0d8bf2c870a1f0da1a3e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 21:45:58 -0700 Subject: [PATCH 48/48] link issue and add static import --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 4 ++-- .../java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index fa99a91775..0d814a7bc1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -1,10 +1,10 @@ package com.uber.nullaway; +import static com.google.common.base.Verify.verify; import static com.uber.nullaway.NullabilityUtil.castToNonNull; import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; -import com.google.common.base.Verify; import com.google.errorprone.VisitorState; import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.suppliers.Suppliers; @@ -726,7 +726,7 @@ private static Type getTypeForSymbol(Symbol symbol, VisitorState state) { } Type typeFromTree = getTreeType(newClassTree, state); if (typeFromTree != null) { - Verify.verify(state.getTypes().isAssignable(symbol.type, typeFromTree)); + verify(state.getTypes().isAssignable(symbol.type, typeFromTree)); } return typeFromTree; } else { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 328bdd11eb..7f236cacfd 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1025,7 +1025,7 @@ public void overrideExplicitlyTypedAnonymousClass() { .doTest(); } - @Ignore("Need to add support for this case") + @Ignore("https://github.com/uber/NullAway/issues/836") @Test public void overrideAnonymousNestedClass() { makeHelper()