From 01ee4d7667870d604cd226755aa24f82fe8593ec Mon Sep 17 00:00:00 2001 From: Nikita Dinkar Aware Date: Sat, 12 Aug 2023 17:15:52 -0500 Subject: [PATCH] Generics checks for method overriding (#755) This PR implements JSpecify-related generics checks for method overriding, including: * checks that overrides of generic methods respect subtyping of nullability qualifiers * proper handling of calls to generic methods, taking into account (qualified) types passed as type parameters See the new tests and docs for further examples. **Note:** This PR does not handle overriding in explicitly-typed anonymous inner classes. Those will be handled in a follow-up PR, to keep the PR size manageable. --------- Co-authored-by: Manu Sridharan --- .../java/com/uber/nullaway/ErrorMessage.java | 2 + .../com/uber/nullaway/GenericsChecks.java | 290 +++++++++++++++++- .../main/java/com/uber/nullaway/NullAway.java | 57 +++- .../AccessPathNullnessPropagation.java | 26 +- .../NullAwayJSpecifyGenericsTests.java | 281 +++++++++++++++++ 5 files changed, 647 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 22ebf13b4b..186b386cde 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -56,6 +56,8 @@ public enum MessageTypes { ASSIGN_GENERIC_NULLABLE, 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 d12a112f03..c4ba2f1504 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -13,6 +13,9 @@ 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; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; @@ -190,6 +193,38 @@ private void reportInvalidParametersNullabilityError( errorMessage, analysis.buildDescription(paramExpression), state, null)); } + private void reportInvalidOverridingMethodReturnTypeError( + Tree methodTree, Type overriddenMethodReturnType, Type overridingMethodReturnType) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.WRONG_OVERRIDE_RETURN_GENERIC, + "Method returns " + + prettyTypeForError(overridingMethodReturnType) + + ", but overridden method returns " + + prettyTypeForError(overriddenMethodReturnType) + + ", which has mismatched type parameter nullability"); + state.reportMatch( + errorBuilder.createErrorDescription( + errorMessage, analysis.buildDescription(methodTree), state, null)); + } + + private void reportInvalidOverridingMethodParamTypeError( + Tree formalParameterTree, Type typeParameterType, Type methodParamType) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + 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(formalParameterTree), state, null)); + } + /** * This method returns the type of the given tree, including any type use annotations. * @@ -366,7 +401,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); @@ -380,6 +414,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)) { @@ -512,6 +547,259 @@ public void compareGenericTypeParameterNullabilityForCall( } } + /** + * 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 + * @param overriddenMethod A symbol of the overridden method + */ + public void checkTypeParameterNullnessForMethodOverriding( + MethodTree tree, Symbol.MethodSymbol overridingMethod, Symbol.MethodSymbol overriddenMethod) { + if (!config.isJSpecifyMode()) { + return; + } + // 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); + + checkTypeParameterNullnessForOverridingMethodReturnType(tree, methodWithTypeParams); + checkTypeParameterNullnessForOverridingMethodParameterType(tree, methodWithTypeParams); + } + + /** + * 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: + * + * 

+   *     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 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 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 {@code method} in the context of {@code + * enclosingType} + */ + 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); + } + 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 + * 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 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 invokedMethodSymbol, MethodInvocationTree tree) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { + return Nullness.NONNULL; + } + Type methodReceiverType = + castToNonNull( + ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + return getGenericMethodParameterNullness(paramIndex, invokedMethodSymbol, methodReceiverType); + } + + /** + * 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}. + * + *

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) { + Type methodType = state.getTypes().memberType(enclosingType, method); + Type paramType = methodType.getParameterTypes().get(parameterIndex); + return getTypeNullness(paramType, config); + } + + /** + * 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 tree for overriding method + * @param overriddenMethodType type of the overridden method + */ + private void checkTypeParameterNullnessForOverridingMethodParameterType( + MethodTree tree, Type overriddenMethodType) { + List methodParameters = tree.getParameters(); + List overriddenMethodParameterTypes = overriddenMethodType.getParameterTypes(); + // TODO handle varargs; they are not handled for now + for (int i = 0; i < methodParameters.size(); i++) { + 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) overriddenMethodParameterType, + (Type.ClassType) overridingMethodParameterType)) { + reportInvalidOverridingMethodParamTypeError( + methodParameters.get(i), + overriddenMethodParameterType, + overridingMethodParameterType); + } + } + } + } + + /** + * 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 tree for overriding method + * @param overriddenMethodType type of the overridden method + */ + private void checkTypeParameterNullnessForOverridingMethodReturnType( + MethodTree tree, Type overriddenMethodType) { + Type overriddenMethodReturnType = overriddenMethodType.getReturnType(); + Type overridingMethodReturnType = ASTHelpers.getType(tree.getReturnType()); + if (!(overriddenMethodReturnType instanceof Type.ClassType)) { + return; + } + Preconditions.checkArgument(overridingMethodReturnType instanceof Type.ClassType); + if (!compareNullabilityAnnotations( + (Type.ClassType) overriddenMethodReturnType, (Type.ClassType) overridingMethodReturnType)) { + reportInvalidOverridingMethodReturnTypeError( + tree, overriddenMethodReturnType, overridingMethodReturnType); + } + } + + /** + * @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; + } + /** * Returns a pretty-printed representation of type suitable for error messages. The representation * uses simple names rather than fully-qualified names, and retains all type-use annotations. diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index d7c0afb42c..5353b57161 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -628,6 +628,13 @@ public Description matchMethod(MethodTree tree, VisitorState state) { Symbol.MethodSymbol closestOverriddenMethod = 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); + } return checkOverriding(closestOverriddenMethod, methodSymbol, null, state); } } @@ -723,7 +730,11 @@ private Description checkParamOverriding( overriddenMethodArgNullnessMap[i] = Nullness.paramHasNullableAnnotation(overriddenMethod, i, config) ? Nullness.NULLABLE - : Nullness.NONNULL; + : (config.isJSpecifyMode() + ? new GenericsChecks(state, config, this) + .getGenericMethodParameterNullness( + i, overriddenMethod, overridingParamSymbols.get(i).owner.owner.type) + : Nullness.NONNULL); } } @@ -926,7 +937,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 (getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE).equals(Nullness.NONNULL) + if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner.type, state) && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) .equals(Nullness.NULLABLE) && (memberReferenceTree == null @@ -939,7 +950,6 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) + "." + overriddenMethod.toString() + " returns @NonNull"; - } else { message = "method returns @Nullable, but superclass method " @@ -965,6 +975,23 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) overridingMethod.getParameters(), overriddenMethod, null, memberReferenceTree, state); } + private boolean overriddenMethodReturnsNonNull( + Symbol.MethodSymbol overriddenMethod, Type overridingMethodType, VisitorState state) { + Nullness methodReturnNullness = + getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE); + if (!methodReturnNullness.equals(Nullness.NONNULL)) { + return false; + } + // In JSpecify mode, for generic methods, we additionally need to check the return nullness + // using the type parameters from the type enclosing the overriding method + if (config.isJSpecifyMode()) { + return GenericsChecks.getGenericMethodReturnTypeNullness( + overriddenMethod, overridingMethodType, state, config) + .equals(Nullness.NONNULL); + } + return true; + } + @Override public Description matchIdentifier(IdentifierTree tree, VisitorState state) { if (!withinAnnotatedCode(state)) { @@ -1610,7 +1637,11 @@ private Description handleInvocation( argumentPositionNullness[i] = Nullness.paramHasNullableAnnotation(methodSymbol, i, config) ? Nullness.NULLABLE - : Nullness.NONNULL; + : ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree) + ? new GenericsChecks(state, config, this) + .getGenericParameterNullnessAtInvocation( + i, methodSymbol, (MethodInvocationTree) tree) + : Nullness.NONNULL); } } new GenericsChecks(state, config, this) @@ -2279,7 +2310,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: @@ -2298,11 +2331,21 @@ 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); + if (Nullness.hasNullableAnnotation(exprSymbol, config)) { + return true; + } + if (config.isJSpecifyMode() + && GenericsChecks.getGenericReturnNullnessAtInvocation( + exprSymbol, invocationTree, state, config) + .equals(Nullness.NULLABLE)) { + return true; + } + return false; } public boolean nullnessFromDataflow(VisitorState state, ExpressionTree expr) { 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 ece8d4de27..60b80399fc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -29,11 +29,13 @@ 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; 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 +1010,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 +1021,27 @@ 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()) { + MethodInvocationTree tree = node.getTree(); + if (tree != null) { + Nullness nullness = + GenericsChecks.getGenericReturnNullnessAtInvocation( + ASTHelpers.getSymbol(tree), tree, 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 cafb4541c3..260b3cc7ff 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -760,6 +760,287 @@ public void rawTypes() { .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 TestFunc3 implements Fn {", + " @Override", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass", + " public @Nullable 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 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();", + " // BUG: Diagnostic contains: dereferenced expression", + " f3.apply(s).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 nullableGenericTypeVariableReturnType() { + 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 overrideWithNestedTypeParametersInReturnType() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class P{}", + " interface Fn, T4 extends @Nullable Object> {", + " T3 apply();", + " }", + " class TestFunc1 implements Fn, @Nullable String> {", + " @Override", + " // BUG: Diagnostic contains: Method returns P<@Nullable String, @Nullable String>, but overridden method", + " public P<@Nullable String, @Nullable String> apply() {", + " return new P<@Nullable String, @Nullable String>();", + " }", + " }", + " class TestFunc2 implements Fn, @Nullable String> {", + " @Override", + " // BUG: Diagnostic contains: Method returns P<@Nullable String, String>, but overridden method returns", + " public P<@Nullable String, String> apply() {", + " return new P<@Nullable String, String>();", + " }", + " }", + " class TestFunc3 implements Fn, @Nullable String> {", + " @Override", + " public P<@Nullable String, @Nullable String> apply() {", + " return new P<@Nullable String, @Nullable String>();", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void overrideWithNestedTypeParametersInParameterType() { + 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: 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;", + " }", + " }", + " class TestFunc2 implements Fn, String> {", + " @Override", + " public String apply(P p, String s) {", + " return s;", + " }", + " }", + "}") + .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 class TestFunc2_Bad implements Fn2<@Nullable String, @Nullable String> {", + " @Override", + " public @Nullable String apply(@Nullable String s) {", + " // False negative: with contract checking enabled, this should be rejected", + " // See https://github.com/uber/NullAway/issues/803", + " return null;", + " }", + " }", + " 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(