diff --git a/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java b/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java index 37fcc0386f..1409248907 100644 --- a/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java @@ -32,8 +32,10 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.util.Context; +import com.uber.nullaway.handlers.Handler; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nullable; import javax.lang.model.element.ElementKind; /** @@ -120,7 +122,7 @@ public boolean isGenerated(Symbol symbol, Config config) { symbol)); return false; } - Symbol.ClassSymbol outermostClassSymbol = get(classSymbol, config).outermostClassSymbol; + Symbol.ClassSymbol outermostClassSymbol = get(classSymbol, config, null).outermostClassSymbol; return hasDirectAnnotationWithSimpleName(outermostClassSymbol, "Generated"); } @@ -145,10 +147,11 @@ private static boolean isClassFieldOfPrimitiveType(Symbol symbol) { * * @param symbol symbol for entity * @param config NullAway config + * @param handler optional NullAway Handler * @return true if symbol represents an entity contained in a class that is unannotated; false * otherwise */ - public boolean isSymbolUnannotated(Symbol symbol, Config config) { + public boolean isSymbolUnannotated(Symbol symbol, Config config, @Nullable Handler handler) { Symbol.ClassSymbol classSymbol; if (symbol instanceof Symbol.ClassSymbol) { classSymbol = (Symbol.ClassSymbol) symbol; @@ -162,7 +165,7 @@ public boolean isSymbolUnannotated(Symbol symbol, Config config) { } else { classSymbol = castToNonNull(ASTHelpers.enclosingClass(symbol)); } - final ClassCacheRecord classCacheRecord = get(classSymbol, config); + final ClassCacheRecord classCacheRecord = get(classSymbol, config, handler); boolean inAnnotatedClass = classCacheRecord.isNullnessAnnotated; if (symbol.getKind().equals(ElementKind.METHOD) || symbol.getKind().equals(ElementKind.CONSTRUCTOR)) { @@ -176,12 +179,15 @@ public boolean isSymbolUnannotated(Symbol symbol, Config config) { * Check whether a class should be treated as nullness-annotated. * * @param classSymbol The symbol for the class to be checked + * @param config NullAway config + * @param handler optional NullAway handler * @return Whether this class should be treated as null-annotated, taking into account annotations * on enclosing classes, the containing package, and other NullAway configuration like * annotated packages */ - public boolean isClassNullAnnotated(Symbol.ClassSymbol classSymbol, Config config) { - return get(classSymbol, config).isNullnessAnnotated; + public boolean isClassNullAnnotated( + Symbol.ClassSymbol classSymbol, Config config, Handler handler) { + return get(classSymbol, config, handler).isNullnessAnnotated; } /** @@ -190,12 +196,15 @@ public boolean isClassNullAnnotated(Symbol.ClassSymbol classSymbol, Config confi *

This method is recursive, using the cache on the way up and populating it on the way down. * * @param classSymbol The class to query, possibly an inner class + * @param config NullAway config + * @param handler optional NullAway handler * @return A record including the outermost class in which the given class is nested, as well as * boolean flag noting whether it should be treated as nullness-annotated, taking into account * annotations on enclosing classes, the containing package, and other NullAway configuration * like annotated packages */ - private ClassCacheRecord get(Symbol.ClassSymbol classSymbol, Config config) { + private ClassCacheRecord get( + Symbol.ClassSymbol classSymbol, Config config, @Nullable Handler handler) { ClassCacheRecord record = classCache.getIfPresent(classSymbol); if (record != null) { return record; @@ -211,7 +220,7 @@ private ClassCacheRecord get(Symbol.ClassSymbol classSymbol, Config config) { Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(classSymbol); // enclosingClass can be null in weird cases like for array methods if (enclosingClass != null) { - ClassCacheRecord recordForEnclosing = get(enclosingClass, config); + ClassCacheRecord recordForEnclosing = get(enclosingClass, config, handler); // Check if this class is annotated, recall that enclosed scopes override enclosing scopes boolean isAnnotated = recordForEnclosing.isNullnessAnnotated; if (enclosingMethod != null) { @@ -232,7 +241,8 @@ record = new ClassCacheRecord(recordForEnclosing.outermostClassSymbol, isAnnotat } if (record == null) { // We are already at the outermost class (we can find), so let's create a record for it - record = new ClassCacheRecord(classSymbol, isAnnotatedTopLevelClass(classSymbol, config)); + record = + new ClassCacheRecord(classSymbol, isAnnotatedTopLevelClass(classSymbol, config, handler)); } classCache.put(classSymbol, record); return record; @@ -258,7 +268,17 @@ private boolean shouldTreatAsUnannotated(Symbol.ClassSymbol classSymbol, Config return false; } - private boolean isAnnotatedTopLevelClass(Symbol.ClassSymbol classSymbol, Config config) { + /** + * Check if a non-nested class should be treated as nullness-annotated. + * + * @param classSymbol symbol for the class + * @param config NullAway config + * @param handler optional NullAway handler. If present, used to check if a class is NullMarked + * based on {@link Handler#onOverrideNullMarkedClasses(String)} + * @return {@code true} iff the class should be treated as nullness-annotated + */ + private boolean isAnnotatedTopLevelClass( + Symbol.ClassSymbol classSymbol, Config config, @Nullable Handler handler) { // First, check for an explicitly @NullUnmarked top level class if (hasDirectAnnotationWithSimpleName(classSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) { return false; @@ -269,7 +289,12 @@ private boolean isAnnotatedTopLevelClass(Symbol.ClassSymbol classSymbol, Config // make sure it's not explicitly configured as unannotated return !shouldTreatAsUnannotated(classSymbol, config); } - return false; + // Check if it is NullMarked inside a Library Model when in JSpecify Mode + if (config.isJSpecifyMode() && handler != null) { + return handler.onOverrideNullMarkedClasses(classSymbol.toString()); + } else { + return false; + } } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index 1eeb219f82..1f8e0be782 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -112,6 +112,26 @@ public interface LibraryModels { */ ImmutableSet nonNullReturns(); + /** + * Get the (className, type argument index) pairs for library classes where the generic type + * argument has a {@code @Nullable} upper bound. Only used in JSpecify mode. + * + * @return map from the className to the positions of the generic type arguments that have a + * {@code Nullable} upper bound. + */ + default ImmutableSetMultimap typeVariablesWithNullableUpperBounds() { + return ImmutableSetMultimap.of(); + } + + /** + * Get the set of library classes that are NullMarked. Only used in JSpecify mode. + * + * @return set of library classes that are NullMarked. + */ + default ImmutableSet nullMarkedClasses() { + return ImmutableSet.of(); + } + /** * Get (method, parameter) pairs that act as castToNonNull(...) methods. * @@ -137,7 +157,9 @@ public interface LibraryModels { * * @return set of library fields that may be {@code null}. */ - ImmutableSet nullableFields(); + default ImmutableSet nullableFields() { + return ImmutableSet.of(); + } /** * Get a list of custom stream library specifications. diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 5902b20275..811ea592ed 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -307,7 +307,7 @@ public NullAway(ErrorProneFlags flags) { private boolean isMethodUnannotated(MethodInvocationNode invocationNode) { return invocationNode == null || codeAnnotationInfo.isSymbolUnannotated( - ASTHelpers.getSymbol(invocationNode.getTree()), config); + ASTHelpers.getSymbol(invocationNode.getTree()), config, handler); } private boolean withinAnnotatedCode(VisitorState state) { @@ -346,7 +346,7 @@ private boolean checkMarkingForPath(VisitorState state) { if (enclosingMarkableSymbol == null) { return false; } - return !codeAnnotationInfo.isSymbolUnannotated(enclosingMarkableSymbol, config); + return !codeAnnotationInfo.isSymbolUnannotated(enclosingMarkableSymbol, config, handler); } @Override @@ -612,7 +612,7 @@ private void checkForMethodNullMarkedness(MethodTree tree, VisitorState state) { methodSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) { // We still care here if this is a transition between @NullUnmarked and @NullMarked code, // within partially marked code, see checks below for markedMethodInUnmarkedContext. - if (!codeAnnotationInfo.isClassNullAnnotated(methodSymbol.enclClass(), config)) { + if (!codeAnnotationInfo.isClassNullAnnotated(methodSymbol.enclClass(), config, handler)) { markedMethodInUnmarkedContext = true; } } @@ -713,7 +713,8 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta return Description.NO_MATCH; } if (config.isJSpecifyMode()) { - GenericsChecks.checkInstantiationForParameterizedTypedTree(tree, state, this, config); + GenericsChecks.checkInstantiationForParameterizedTypedTree( + tree, state, this, config, handler); } return Description.NO_MATCH; } @@ -741,7 +742,7 @@ private Description checkParamOverriding( (memberReferenceTree != null) && ((JCTree.JCMemberReference) memberReferenceTree).kind.isUnbound(); final boolean isOverriddenMethodAnnotated = - !codeAnnotationInfo.isSymbolUnannotated(overriddenMethod, config); + !codeAnnotationInfo.isSymbolUnannotated(overriddenMethod, config, handler); // Get argument nullability for the overridden method. If overriddenMethodArgNullnessMap[i] is // null, parameter i is treated as unannotated. @@ -861,7 +862,8 @@ static Trees getTreesInstance(VisitorState state) { private Nullness getMethodReturnNullness( Symbol.MethodSymbol methodSymbol, VisitorState state, Nullness defaultForUnannotated) { - final boolean isMethodAnnotated = !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config); + final boolean isMethodAnnotated = + !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config, handler); Nullness methodReturnNullness = defaultForUnannotated; // Permissive default for unannotated code. if (isMethodAnnotated) { @@ -916,13 +918,15 @@ private Description checkReturnExpression( // type is @Nullable, and if so, bail out. if (getMethodReturnNullness(methodSymbol, state, Nullness.NULLABLE).equals(Nullness.NULLABLE)) { return Description.NO_MATCH; - } else if (config.isJSpecifyMode() - && lambdaTree != null - && GenericsChecks.getGenericMethodReturnTypeNullness( - methodSymbol, ASTHelpers.getType(lambdaTree), state, config) - .equals(Nullness.NULLABLE)) { - // In JSpecify mode, the return type of a lambda may be @Nullable via a type argument - return Description.NO_MATCH; + } else if (config.isJSpecifyMode() && lambdaTree != null) { + if (GenericsChecks.getGenericMethodReturnTypeNullness( + methodSymbol, ASTHelpers.getType(lambdaTree), state, config) + .equals(Nullness.NULLABLE) + || GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( + methodSymbol, lambdaTree, state, config, codeAnnotationInfo, handler)) { + // In JSpecify mode, the return type of a lambda may be @Nullable via a type argument + return Description.NO_MATCH; + } } // Return type is @NonNull. Check if the expression is @Nullable @@ -950,7 +954,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState // (like Rx nullability) run dataflow analysis updateEnvironmentMapping(state.getPath(), state); handler.onMatchLambdaExpression(this, tree, state, funcInterfaceMethod); - if (codeAnnotationInfo.isSymbolUnannotated(funcInterfaceMethod, config)) { + if (codeAnnotationInfo.isSymbolUnannotated(funcInterfaceMethod, config, handler)) { return Description.NO_MATCH; } Description description = @@ -1065,8 +1069,10 @@ private boolean overriddenMethodReturnsNonNull( // For a method reference, we get generic type arguments from javac's inferred type for the // tree, which properly preserves type-use annotations return GenericsChecks.getGenericMethodReturnTypeNullness( - overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config) - .equals(Nullness.NONNULL); + overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config) + .equals(Nullness.NONNULL) + && !GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( + overriddenMethod, memberReferenceTree, state, config, codeAnnotationInfo, handler); } else { // Use the enclosing class of the overriding method to find generic type arguments return GenericsChecks.getGenericMethodReturnTypeNullness( @@ -1176,7 +1182,7 @@ private boolean relevantInitializerMethodOrBlock( // in those cases, we won't even have a populated class2Entities map). We skip this check if // we are not inside a @NullMarked/annotated *class*: if (nullMarkingForTopLevelClass == NullMarking.PARTIALLY_MARKED - && !codeAnnotationInfo.isClassNullAnnotated(enclClassSymbol, config)) { + && !codeAnnotationInfo.isClassNullAnnotated(enclClassSymbol, config, handler)) { return false; } @@ -1697,7 +1703,8 @@ private Description handleInvocation( return Description.NO_MATCH; } - final boolean isMethodAnnotated = !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config); + final boolean isMethodAnnotated = + !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config, handler); // If argumentPositionNullness[i] == null, parameter i is unannotated Nullness[] argumentPositionNullness = new Nullness[formalParams.size()]; @@ -2287,7 +2294,7 @@ private boolean isExcludedClass(Symbol.ClassSymbol classSymbol) { if (config.isExcludedClass(className)) { return true; } - if (!codeAnnotationInfo.isClassNullAnnotated(classSymbol, config)) { + if (!codeAnnotationInfo.isClassNullAnnotated(classSymbol, config, handler)) { return true; } // check annotations @@ -2431,7 +2438,7 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) { private boolean mayBeNullMethodCall( Symbol.MethodSymbol exprSymbol, MethodInvocationTree invocationTree, VisitorState state) { - if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config)) { + if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config, handler)) { return false; } if (Nullness.hasNullableAnnotation(exprSymbol, config)) { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index b1528c40ea..6befea7b79 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -362,7 +362,7 @@ public static boolean mayBeNullFieldFromType( Symbol symbol, Config config, CodeAnnotationInfo codeAnnotationInfo) { return !(symbol.getSimpleName().toString().equals("class") || symbol.isEnum() - || codeAnnotationInfo.isSymbolUnannotated(symbol, config)) + || codeAnnotationInfo.isSymbolUnannotated(symbol, config, null)) && Nullness.hasNullableAnnotation(symbol, config); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index 2dfce9fc74..e05a53161e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -100,7 +100,8 @@ private static NullnessStore lambdaInitialStore( types.memberType(ASTHelpers.getType(code), fiMethodSymbol).getParameterTypes(); // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated Nullness[] fiArgumentPositionNullness = new Nullness[fiMethodParameters.size()]; - final boolean isFIAnnotated = !codeAnnotationInfo.isSymbolUnannotated(fiMethodSymbol, config); + final boolean isFIAnnotated = + !codeAnnotationInfo.isSymbolUnannotated(fiMethodSymbol, config, handler); if (isFIAnnotated) { for (int i = 0; i < fiMethodParameters.size(); i++) { if (Nullness.hasNullableAnnotation(fiMethodParameters.get(i), config)) { diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 4754ed92a0..aad9247b88 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -23,16 +23,19 @@ import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; +import com.uber.nullaway.CodeAnnotationInfo; import com.uber.nullaway.Config; import com.uber.nullaway.ErrorBuilder; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; +import com.uber.nullaway.handlers.Handler; import java.util.HashMap; import java.util.List; import java.util.Map; import javax.annotation.Nullable; import javax.lang.model.type.ExecutableType; +import javax.lang.model.type.TypeVariable; /** Methods for performing checks related to generic types and nullability. */ public final class GenericsChecks { @@ -56,9 +59,14 @@ private GenericsChecks() {} * @param state visitor state * @param analysis the analysis object * @param config the analysis config + * @param handler the handler instance */ public static void checkInstantiationForParameterizedTypedTree( - ParameterizedTypeTree tree, VisitorState state, NullAway analysis, Config config) { + ParameterizedTypeTree tree, + VisitorState state, + NullAway analysis, + Config config, + Handler handler) { if (!config.isJSpecifyMode()) { return; } @@ -95,7 +103,8 @@ public static void checkInstantiationForParameterizedTypedTree( com.sun.tools.javac.util.List annotationMirrors = upperBound.getAnnotationMirrors(); boolean hasNullableAnnotation = - Nullness.hasNullableAnnotation(annotationMirrors.stream(), config); + Nullness.hasNullableAnnotation(annotationMirrors.stream(), config) + || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i); // if base type argument does not have @Nullable annotation then the instantiation is // invalid if (!hasNullableAnnotation) { @@ -523,7 +532,7 @@ public static void checkTypeParameterNullnessForMethodOverriding( * } * * - * Within the context of class {@code C}, the method {@code Fn.apply} has a return type of + *

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}. @@ -607,7 +616,7 @@ public static Nullness getGenericMethodReturnTypeNullness( * } * * - * The declared type of {@code f} passes {@code Nullable String} as the type parameter for type + *

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. * @@ -657,7 +666,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( * } * * - * The declared type of {@code f} passes {@code Nullable String} as the type parameter for type + *

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 @@ -701,7 +710,7 @@ public static Nullness getGenericParameterNullnessAtInvocation( * } * * - * Within the context of class {@code C}, the method {@code Fn.apply} has a parameter type of + *

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. @@ -832,4 +841,47 @@ private static Nullness getTypeNullness(Type type, Config config) { public static String prettyTypeForError(Type type, VisitorState state) { return type.accept(new GenericTypePrettyPrintingVisitor(state), null); } + + /** + * Checks if a given expression e is a lambda or method reference such that (1) the + * declared return type of the method for e is a generic type variable, and (2) + * e is being passed as a parameter to an unannotated method. In such cases, the caller + * should treat e as being allowed to return a {@code Nullable} value, even if the + * locally-computed type of the expression is not {@code @Nullable}. This special treatment is + * necessary to properly avoid reporting errors when interacting with unannotated / unmarked code. + * + * @param methodSymbol the symbol for the method corresponding to e + * @param expressionTree the expression e + * @param state visitor state + * @param config NullAway configuration + * @param handler NullAway handler + * @param codeAnnotationInfo information on which code is annotated + */ + public static boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( + Symbol.MethodSymbol methodSymbol, + ExpressionTree expressionTree, + VisitorState state, + Config config, + CodeAnnotationInfo codeAnnotationInfo, + Handler handler) { + Type methodType = methodSymbol.type; + boolean returnsGeneric = methodType.getReturnType() instanceof TypeVariable; + if (!returnsGeneric) { + return false; + } + boolean callingUnannotated = false; + TreePath path = state.getPath(); + while (path != null && !path.getLeaf().equals(expressionTree)) { + path = path.getParentPath(); + } + verify(path != null, "did not find lambda or method reference tree in TreePath"); + Tree parentOfLambdaTree = path.getParentPath().getLeaf(); + if (parentOfLambdaTree instanceof MethodInvocationTree) { + Symbol.MethodSymbol parentMethodSymbol = + ASTHelpers.getSymbol((MethodInvocationTree) parentOfLambdaTree); + callingUnannotated = + codeAnnotationInfo.isSymbolUnannotated(parentMethodSymbol, config, handler); + } + return callingUnannotated; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java index 815b46d7aa..3cc0e92f81 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java @@ -214,6 +214,16 @@ public void onNonNullFieldAssignment( // NoOp } + @Override + public boolean onOverrideTypeParameterUpperBound(String className, int index) { + return false; + } + + @Override + public boolean onOverrideNullMarkedClasses(String className) { + return false; + } + @Override public MethodInvocationNode onCFGBuildPhase1AfterVisitMethodInvocation( NullAwayCFGBuilder.NullAwayCFGTranslationPhaseOne phase, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java index 2001269fbd..a8eec51ac7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -305,4 +305,30 @@ public Integer castToNonNullArgumentPositionsForMethod( } return previousArgumentPosition; } + + /** Returns true if any handler returns true. */ + @Override + public boolean onOverrideTypeParameterUpperBound(String className, int index) { + boolean result = false; + for (Handler h : handlers) { + result = h.onOverrideTypeParameterUpperBound(className, index); + if (result) { + break; + } + } + return result; + } + + /** Returns true if any handler returns true. */ + @Override + public boolean onOverrideNullMarkedClasses(String className) { + boolean result = false; + for (Handler h : handlers) { + result = h.onOverrideNullMarkedClasses(className); + if (result) { + break; + } + } + return result; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java index b2018ee294..ea084c3cb2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -402,6 +402,24 @@ Integer castToNonNullArgumentPositionsForMethod( List actualParams, @Nullable Integer previousArgumentPosition); + /** + * Method to override the nullability of the upper bound for a generic type variable on a class. + * + * @param className name of the class + * @param index index of the generic type variable (starting at 0) + * @return boolean true if the variable should be treated as having a {@code @Nullable} upper + * bound + */ + boolean onOverrideTypeParameterUpperBound(String className, int index); + + /** + * Method to override the null-markedness of a class. + * + * @param className name of the class + * @return boolean true if the class should be treated as {@code @NullMarked} + */ + boolean onOverrideNullMarkedClasses(String className); + /** * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate * their knowledge of the method return nullability to the rest of NullAway. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index fe87278edd..33d43ed96c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -168,7 +168,7 @@ public boolean onOverrideMayBeNullExpr( // and any of its overriding implementations. // see https://github.com/uber/NullAway/issues/445 for why this is needed. boolean isMethodUnannotated = - getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config); + getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config, null); if (exprMayBeNull) { // This is the only case in which we may switch the result from @Nullable to @NonNull: return !optLibraryModels.hasNonNullReturn( @@ -226,7 +226,7 @@ public NullnessHint onDataflowVisitMethodInvocation( AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { boolean isMethodAnnotated = - !getCodeAnnotationInfo(state.context).isSymbolUnannotated(callee, this.config); + !getCodeAnnotationInfo(state.context).isSymbolUnannotated(callee, this.config, null); setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, state, apContext); setConditionalArgumentNullness( thenUpdates, elseUpdates, node.getArguments(), callee, state, apContext); @@ -343,6 +343,17 @@ private void setUnconditionalArgumentNullness( } } + @Override + public boolean onOverrideTypeParameterUpperBound(String className, int index) { + ImmutableSet res = libraryModels.typeVariablesWithNullableUpperBounds().get(className); + return res.contains(index); + } + + @Override + public boolean onOverrideNullMarkedClasses(String className) { + return libraryModels.nullMarkedClasses().contains(className); + } + /** * Get all the stream specifications loaded from any of our library models. * @@ -826,7 +837,14 @@ private static class DefaultLibraryModels implements LibraryModels { "getDrawable(android.content.Context,int)")) .add(methodRef("android.support.design.widget.TextInputLayout", "getEditText()")) .build(); + private static final ImmutableSetMultimap NULLABLE_VARIABLE_TYPE_UPPER_BOUNDS = + new ImmutableSetMultimap.Builder() + .put("java.util.function.Function", 0) + .put("java.util.function.Function", 1) + .build(); + private static final ImmutableSet NULLMARKED_CLASSES = + new ImmutableSet.Builder().add("java.util.function.Function").build(); private static final ImmutableSetMultimap CAST_TO_NONNULL_METHODS = new ImmutableSetMultimap.Builder().build(); @@ -870,6 +888,16 @@ public ImmutableSet nonNullReturns() { return NONNULL_RETURNS; } + @Override + public ImmutableSetMultimap typeVariablesWithNullableUpperBounds() { + return NULLABLE_VARIABLE_TYPE_UPPER_BOUNDS; + } + + @Override + public ImmutableSet nullMarkedClasses() { + return NULLMARKED_CLASSES; + } + @Override public ImmutableSetMultimap castToNonNullMethods() { return CAST_TO_NONNULL_METHODS; @@ -902,6 +930,10 @@ private static class CombinedLibraryModels implements LibraryModels { private final ImmutableSet nonNullReturns; + private final ImmutableSetMultimap nullableVariableTypeUpperBounds; + + private final ImmutableSet nullMarkedClasses; + private final ImmutableSet nullableFields; private final ImmutableSetMultimap castToNonNullMethods; @@ -916,6 +948,9 @@ public CombinedLibraryModels(Iterable models, Config config) { new ImmutableSetMultimap.Builder<>(); ImmutableSetMultimap.Builder nonNullParametersBuilder = new ImmutableSetMultimap.Builder<>(); + ImmutableSetMultimap.Builder nullableVariableTypeUpperBoundsBuilder = + new ImmutableSetMultimap.Builder<>(); + ImmutableSet.Builder nullMarkedClassesBuilder = new ImmutableSet.Builder<>(); ImmutableSetMultimap.Builder nullImpliesTrueParametersBuilder = new ImmutableSetMultimap.Builder<>(); ImmutableSetMultimap.Builder nullImpliesFalseParametersBuilder = @@ -988,6 +1023,11 @@ public CombinedLibraryModels(Iterable models, Config config) { } castToNonNullMethodsBuilder.put(entry); } + nullableVariableTypeUpperBoundsBuilder.putAll( + libraryModels.typeVariablesWithNullableUpperBounds()); + for (String name : libraryModels.nullMarkedClasses()) { + nullMarkedClassesBuilder.add(name); + } for (StreamTypeRecord streamTypeRecord : libraryModels.customStreamNullabilitySpecs()) { customStreamNullabilitySpecsBuilder.add(streamTypeRecord); } @@ -1006,6 +1046,8 @@ public CombinedLibraryModels(Iterable models, Config config) { castToNonNullMethods = castToNonNullMethodsBuilder.build(); customStreamNullabilitySpecs = customStreamNullabilitySpecsBuilder.build(); nullableFields = nullableFieldsBuilder.build(); + nullableVariableTypeUpperBounds = nullableVariableTypeUpperBoundsBuilder.build(); + nullMarkedClasses = nullMarkedClassesBuilder.build(); } private boolean shouldSkipModel(MethodRef key) { @@ -1052,6 +1094,16 @@ public ImmutableSet nonNullReturns() { return nonNullReturns; } + @Override + public ImmutableSetMultimap typeVariablesWithNullableUpperBounds() { + return nullableVariableTypeUpperBounds; + } + + @Override + public ImmutableSet nullMarkedClasses() { + return nullMarkedClasses; + } + @Override public ImmutableSet nullableFields() { return nullableFields; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java index 561ac6ebae..f1398f237d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java @@ -62,7 +62,7 @@ public class RestrictiveAnnotationHandler extends BaseNoOpHandler { */ private boolean isSymbolRestrictivelyNullable(Symbol symbol, Context context) { CodeAnnotationInfo codeAnnotationInfo = getCodeAnnotationInfo(context); - return (codeAnnotationInfo.isSymbolUnannotated(symbol, config) + return (codeAnnotationInfo.isSymbolUnannotated(symbol, config, null) // with the generated-as-unannotated option enabled, we want to ignore annotations in // generated code no matter what && !(config.treatGeneratedAsUnannotated() && codeAnnotationInfo.isGenerated(symbol, config)) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index d146442e17..0b7e99d66c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1640,6 +1640,83 @@ public void overrideWithRawType() { .doTest(); } + @Test + public void testForNullableUpperBoundsInLibModel() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Function;", + "class Test {", + " static String foo(@Nullable String a) {", + " if(a!=null){ ", + " return a.replace(\"a\",\"\");", + " }else{", + " return \"\";", + " }", + " }", + " static void testPositiveMethodRef() {", + " Function removeA = Test::foo;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " removeA.apply(null);", + " }", + " static void testNegativeMethodRef() {", + " Function<@Nullable String,@Nullable String> removeA = Test::foo;", + " removeA.apply(null);", + " }", + " static void testPositiveLambda() {", + " Function removeA = a -> a.replace(\"a\",\"\");", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " removeA.apply(null);", + " }", + " static void testNegative() {", + " Function removeA = a -> a.replace(\"a\",\"\");", + " }", + " static @Nullable String foo2(@Nullable String b) {", + " return null;", + " }", + " static Function testPositiveReturn() {", + " // BUG: Diagnostic contains: referenced method returns @Nullable, but functional interface method", + " return Test::foo2;", + " }", + "}") + .doTest(); + } + + @Test + public void passAnnotatedLambdaOrMethodRefToUnannotatedCode() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.NullUnmarked;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static interface Fn {", + " T apply(S s);", + " }", + " @NullUnmarked", + " static class C1 {", + " static void foo(Fn f) {}", + " }", + " @NullMarked", + " static class C2 {", + " static void m1() {", + " // no error here since C1 is @NullUnmarked", + " C1.foo(s -> null);", + " }", + " static @Nullable String baz(String s) { return null; }", + " static void m2() {", + " // no error here since C1 is @NullUnmarked", + " C1.foo(C2::baz);", + " }", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( diff --git a/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java b/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java index 5c17aca966..a1aa99d79d 100644 --- a/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java +++ b/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java @@ -71,9 +71,4 @@ public ImmutableSet nonNullReturns() { public ImmutableSetMultimap castToNonNullMethods() { return ImmutableSetMultimap.of(); } - - @Override - public ImmutableSet nullableFields() { - return ImmutableSet.of(); - } }