diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index 9f84df1746..582c73d1c1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -139,6 +139,23 @@ default ImmutableSetMultimap ensuresNonNullIfTrueMethodCal return ImmutableMap.of(); } + /** + * Get the locations in library method signatures that have linked, polymorphic nullness. + * + *

At a call, NullAway introduces nullability inference variables at the modeled input + * locations and generates constraints from invocation arguments, including lambdas and method + * references. All input occurrences must infer the same nullness, which is then substituted at + * every modeled location. Explicit method type arguments seed the corresponding inference + * variables after substitution. A parameter index of {@code -1} denotes the return type. This + * model is used only in JSpecify mode, and the modeled method's enclosing class is expected to be + * modeled as {@code @NullMarked}. + * + * @return map from methods to signature locations with polymorphic nullness + */ + default ImmutableSetMultimap polyNullLocations() { + return ImmutableSetMultimap.of(); + } + /** * Get the (className, type argument index) pairs for library classes where the generic type * variable has a {@code @Nullable} upper bound. Only used in JSpecify mode. @@ -339,6 +356,23 @@ public String toString() { } } + /** + * A location within a method signature whose nullness is linked to other locations for the same + * method. + * + * @param parameterIndex zero-based parameter index, or {@code -1} for the return type + * @param typePath path within the parameter or return type; an empty path denotes its top level + */ + public record PolyNullLocation( + int parameterIndex, ImmutableList typePath) { + + public PolyNullLocation { + if (parameterIndex < -1) { + throw new IllegalArgumentException("parameter index must be -1 or greater"); + } + } + } + /** Representation of a field as a qualified class name + a field name */ record FieldRef(String enclosingClassName, String fieldName) { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 6e7c3cd4f8..734ddbf231 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -3009,7 +3009,9 @@ private boolean mayBeNullMethodCall( if (Nullness.hasNullableAnnotation(exprSymbol, config)) { return true; } - if (config.isJSpecifyMode() && exprSymbol.getReturnType().getKind().equals(TypeKind.TYPEVAR)) { + if (config.isJSpecifyMode() + && (exprSymbol.getReturnType().getKind().equals(TypeKind.TYPEVAR) + || genericsChecks.hasPolyNullModel(exprSymbol, state))) { // It is important to pass a correct TreePath to getGenericReturnNullnessAtInvocation. So, we // do a search under path to find invocationTree. This shouldn't be too costly in the common // case, and it's important for correctness. 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 ac8279c5fa..ecc42b6119 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -8,6 +8,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotatedTypeTree; @@ -49,6 +50,7 @@ import com.uber.nullaway.ErrorBuilder; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.InvocationArguments; +import com.uber.nullaway.LibraryModels.PolyNullLocation; import com.uber.nullaway.NullAway; import com.uber.nullaway.NullabilityUtil; import com.uber.nullaway.Nullness; @@ -57,9 +59,12 @@ import com.uber.nullaway.dataflow.NullnessStore; import com.uber.nullaway.generics.ConstraintSolver.UnsatisfiableConstraintsException; import com.uber.nullaway.generics.GenericsUtils.MethodRefTypeRelationKind; +import com.uber.nullaway.generics.PolyNullInference.PolyNullInferenceContext; +import com.uber.nullaway.generics.PolyNullInference.PolyNullInferenceResult; import com.uber.nullaway.handlers.Handler; import java.util.ArrayList; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -86,12 +91,19 @@ private interface CallInferenceResult {} /** * Indicates successful inference of nullability of type variables at a call. Stores the inferred - * type variable nullability. + * type variable nullability and any PolyNull resolutions computed by the same solver run. */ private record InferenceSuccess( - Map typeVarNullability) + Map typeVarNullability, + IdentityHashMap polyNullResults) implements CallInferenceResult {} + /** + * A generic method type after substitution, together with any jointly inferred PolyNull value. + */ + private record MethodTypeSubstitution( + Type.MethodType methodType, @Nullable PolyNullInferenceResult polyNullResult) {} + /** Indicates failed inference of nullability of type variables at a call */ private record InferenceFailure(@SuppressWarnings("UnusedVariable") @Nullable String errorMessage) implements CallInferenceResult { @@ -123,6 +135,10 @@ private InferenceFailure(@Nullable String errorMessage) { /** Maps each {@code var}-declared local to its declaration tree */ private final Map varLocalDeclarations = new LinkedHashMap<>(); + /** Successfully inferred polymorphic nullness values cached by invocation identity. */ + private final IdentityHashMap polyNullResolutions = + new IdentityHashMap<>(); + /** * Tracks generic method invocations currently undergoing nested-nullability repair so re-entrant * requests for the same invocation can use the already inferred call-site method type rather than @@ -1277,11 +1293,24 @@ private Type inferCallType( typeVarNullability = ((InferenceSuccess) result).typeVarNullability; } Type typeAtCallSite = castToNonNull(ASTHelpers.getType(callTree)); - if (callTree instanceof MethodInvocationTree) { + if (callTree instanceof MethodInvocationTree invocationTree) { Type methodReturnType = getExecutableTypeForInference(callTree, path, state, calledFromDataflow).getReturnType(); - return TypeSubstitutionUtils.updateTypeWithInferredNullability( - typeAtCallSite, methodReturnType, typeVarNullability, state, config); + Type inferredCallType = + TypeSubstitutionUtils.updateTypeWithInferredNullability( + typeAtCallSite, methodReturnType, typeVarNullability, state, config); + if (result instanceof InferenceSuccess successResult) { + PolyNullInferenceResult polyNullResult = + successResult.polyNullResults().get(invocationTree); + if (polyNullResult != null && polyNullResult.nullness() != null) { + inferredCallType = + PolyNullInference.applyToReturnType( + inferredCallType, + handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state), + polyNullAnnotationType(polyNullResult.nullness(), state)); + } + } + return inferredCallType; } Verify.verify(callTree instanceof NewClassTree); Type constructedTypeAtCallSite = getConstructedTypeAtCallSite((NewClassTree) callTree); @@ -1316,6 +1345,8 @@ private CallInferenceResult runInferenceForCall( // allCalls tracks the top-level call and any nested calls that also require inference Set allCalls = new LinkedHashSet<>(); allCalls.add(callTree); + IdentityHashMap polyNullContexts = + new IdentityHashMap<>(); Map typeVarNullability; try { generateConstraintsForCall( @@ -1326,6 +1357,7 @@ private CallInferenceResult runInferenceForCall( solver, callTree, allCalls, + polyNullContexts, calledFromDataflow); typeVarNullability = new LinkedHashMap<>(solver.solve()); // The solver only computes a solution for variables that appear in constraints. For @@ -1335,16 +1367,32 @@ private CallInferenceResult runInferenceForCall( typeVarNullability.putIfAbsent(typeVar, ConstraintSolver.InferredNullability.NONNULL); } - InferenceSuccess successResult = new InferenceSuccess(typeVarNullability); + IdentityHashMap polyNullResults = + PolyNullInference.resolveContexts(polyNullContexts, typeVarNullability); + InferenceSuccess successResult = new InferenceSuccess(typeVarNullability, polyNullResults); // don't cache result if we were called from dataflow, since the result may rely on dataflow // facts that do not reflect the fixed point if (!calledFromDataflow) { + for (Map.Entry entry : + polyNullResults.entrySet()) { + if (entry.getValue().nullness() != null) { + polyNullResolutions.put(entry.getKey(), entry.getValue().nullness()); + } + } for (Tree inferredCall : allCalls) { inferredTypeVarNullabilityForGenericCalls.put(inferredCall, successResult); } // Store inferred types for lambda or method reference arguments - Type.MethodType callMethodType = - getExecutableTypeForInference(callTree, path, state, calledFromDataflow); + Type.MethodType callMethodType; + PolyNullInferenceContext polyNullContext = + callTree instanceof MethodInvocationTree invocationTree + ? polyNullContexts.get(invocationTree) + : null; + if (polyNullContext != null) { + callMethodType = polyNullContext.inferenceMethodType(); + } else { + callMethodType = getExecutableTypeForInference(callTree, path, state, calledFromDataflow); + } new InvocationArguments(callTree, callMethodType) .forEach( (argument, argPos, formalParamType, unused) -> { @@ -1368,7 +1416,7 @@ private CallInferenceResult runInferenceForCall( } return successResult; } catch (UnsatisfiableConstraintsException e) { - String inferenceFailureMessage = inferenceFailureMessage(e); + String inferenceFailureMessage = inferenceFailureMessage(e, polyNullContexts); if (config.warnOnGenericInferenceFailure() && callsWithReportedInferenceFailures.add(callTree)) { ErrorBuilder errorBuilder = analysis.getErrorBuilder(); @@ -1391,15 +1439,22 @@ private CallInferenceResult runInferenceForCall( } } - private String inferenceFailureMessage(UnsatisfiableConstraintsException e) { + /** Formats an inference failure without exposing synthetic PolyNull variable names to users. */ + private String inferenceFailureMessage( + UnsatisfiableConstraintsException e, + IdentityHashMap polyNullContexts) { + Element typeVariable = e.getTypeVariable(); + if (PolyNullInference.containsInferenceVariable(polyNullContexts, typeVariable)) { + return PolyNullInference.INFERENCE_FAILURE_MESSAGE; + } if (e.isCausedByNonNullUpperBound()) { return String.format( "inference failure: type variable %s is constrained to be @Nullable, but its upper bound requires it to be @NonNull", - e.getTypeVariable()); + typeVariable); } return String.format( "inference failure: type variable %s constrained to be both @NonNull and @Nullable", - e.getTypeVariable()); + typeVariable); } /** Returns the type parameters whose nullability is inferred for {@code callTree}. */ @@ -1471,6 +1526,7 @@ private Symbol.MethodSymbol getMethodSymbolForCall(ExpressionTree callTree) { * @param callTree the call tree representing the generic method call or diamond constructor call * @param allCalls a set of all calls that require inference, including nested ones. This is an * output parameter that gets mutated while generating the constraints to add nested calls. + * @param polyNullContexts PolyNull inference contexts created for calls in this inference session * @param calledFromDataflow whether this method is being called from dataflow analysis * @throws UnsatisfiableConstraintsException if the constraints are determined to be unsatisfiable */ @@ -1482,10 +1538,30 @@ private void generateConstraintsForCall( ConstraintSolver solver, ExpressionTree callTree, Set allCalls, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) throws UnsatisfiableConstraintsException { Type.MethodType methodType = getExecutableTypeForInference(callTree, path, state, calledFromDataflow); + PolyNullInferenceContext polyNullContext = null; + ImmutableSet polyNullLocations = ImmutableSet.of(); + if (callTree instanceof MethodInvocationTree invocationTree) { + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(invocationTree); + polyNullLocations = handler.onGetPolyNullLocations(methodSymbol, state); + if (!polyNullLocations.isEmpty()) { + ImmutableSet locations = polyNullLocations; + polyNullContext = + polyNullContexts.computeIfAbsent( + invocationTree, + unused -> + PolyNullInference.createContext( + methodSymbol, + methodType, + locations, + getSyntheticNullableAnnotType(state), + state)); + } + } // first, handle the call result flow if (typeFromAssignmentContext != null) { Type callResultType = @@ -1493,21 +1569,27 @@ private void generateConstraintsForCall( ? methodType.getReturnType() : getConstructedTypeAtCallSite((NewClassTree) callTree).tsym.type; solver.addSubtypeConstraint(callResultType, typeFromAssignmentContext, assignedToLocal); + if (polyNullContext != null) { + PolyNullInference.addResultConstraints( + solver, polyNullContext, typeFromAssignmentContext, assignedToLocal); + } } // then, handle parameters - TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), callTree); - new InvocationArguments(callTree, methodType) - .forEach( - (argument, argPos, formalParamType, unused) -> { - TreePath pathToArgument = new TreePath(pathToCall, argument); - generateConstraintsForPseudoAssignment( - state.withPath(pathToArgument), - solver, - allCalls, - argument, - formalParamType, - calledFromDataflow); - }); + generateArgumentConstraintsForCall( + state, path, solver, callTree, methodType, allCalls, polyNullContexts, calledFromDataflow); + if (callTree instanceof MethodInvocationTree invocationTree + && polyNullContext != null + && polyNullContext.hasInputLocations()) { + generateArgumentConstraintsForCall( + state, + path, + solver, + invocationTree, + polyNullContext.inferenceMethodType(), + allCalls, + polyNullContexts, + calledFromDataflow); + } } /** @@ -1520,6 +1602,7 @@ private void generateConstraintsForCall( * output parameter that gets mutated while generating the constraints to add nested calls. * @param rhsExpr the right-hand side expression of the pseudo-assignment * @param lhsType the left-hand side type of the pseudo-assignment + * @param polyNullContexts PolyNull inference contexts created for calls in this inference session * @param calledFromDataflow whether this method is being called from dataflow analysis */ private void generateConstraintsForPseudoAssignment( @@ -1528,6 +1611,7 @@ private void generateConstraintsForPseudoAssignment( Set allCalls, ExpressionTree rhsExpr, Type lhsType, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) { NullabilityUtil.ExprTreeAndState exprTreeAndState = NullabilityUtil.stripParensAndUpdateTreePath(rhsExpr, state); @@ -1538,7 +1622,15 @@ private void generateConstraintsForPseudoAssignment( if (isCallNeedingInference(rhsExpr)) { allCalls.add(rhsExpr); generateConstraintsForCall( - state, state.getPath(), lhsType, false, solver, rhsExpr, allCalls, calledFromDataflow); + state, + state.getPath(), + lhsType, + false, + solver, + rhsExpr, + allCalls, + polyNullContexts, + calledFromDataflow); } else if (rhsExpr instanceof ConditionalExpressionTree conditionalExpressionTree) { // generate constraints for both the true and false sub-expressions of the conditional // expression @@ -1550,6 +1642,7 @@ private void generateConstraintsForPseudoAssignment( allCalls, trueExpression, lhsType, + polyNullContexts, calledFromDataflow); ExpressionTree falseExpression = conditionalExpressionTree.getFalseExpression(); TreePath pathToFalseExpression = new TreePath(state.getPath(), falseExpression); @@ -1559,10 +1652,18 @@ private void generateConstraintsForPseudoAssignment( allCalls, falseExpression, lhsType, + polyNullContexts, calledFromDataflow); } else if (rhsExpr instanceof LambdaExpressionTree lambda) { handleLambdaInGenericMethodInference( - state, state.getPath(), solver, allCalls, lhsType, lambda, calledFromDataflow); + state, + state.getPath(), + solver, + allCalls, + lhsType, + lambda, + polyNullContexts, + calledFromDataflow); } else if (rhsExpr instanceof MemberReferenceTree memberReferenceTree) { handleMethodRefInGenericMethodInference(state, solver, lhsType, memberReferenceTree); } else { // all other cases @@ -1589,6 +1690,7 @@ private void generateConstraintsForPseudoAssignment( * output parameter that gets mutated while generating the constraints to add nested calls. * @param lhsType the type to which the lambda is being assigned * @param lambda The lambda argument + * @param polyNullContexts PolyNull inference contexts created for calls in this inference session * @param calledFromDataflow whether this method is being called from dataflow analysis */ private void handleLambdaInGenericMethodInference( @@ -1598,6 +1700,7 @@ private void handleLambdaInGenericMethodInference( Set allCalls, Type lhsType, LambdaExpressionTree lambda, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) { Symbol.MethodSymbol fiMethod = NullabilityUtil.getFunctionalInterfaceMethod(lambda, state.getTypes()); @@ -1622,6 +1725,7 @@ private void handleLambdaInGenericMethodInference( allCalls, returnedExpression, fiReturnType, + polyNullContexts, calledFromDataflow); } else if (body instanceof BlockTree) { // Case 2: Block body, e.g., () -> { return null; } @@ -1637,6 +1741,7 @@ private void handleLambdaInGenericMethodInference( allCalls, returnExpr, fiReturnType, + polyNullContexts, calledFromDataflow); } } @@ -2934,8 +3039,8 @@ public Nullness getGenericMethodReturnTypeNullness( * @param path the path to the invocation tree * @param state the visitor state * @param calledFromDataflow whether this method is being called from dataflow analysis - * @return Nullness of invocation's return type, or {@code NONNULL} if the call does not invoke an - * instance method + * @return nullness of the resolved invocation return type, or {@code NONNULL} when neither + * generic substitution nor a PolyNull model can affect it */ public Nullness getGenericReturnNullnessAtInvocation( Symbol.MethodSymbol invokedMethodSymbol, @@ -2943,34 +3048,17 @@ public Nullness getGenericReturnNullnessAtInvocation( TreePath path, VisitorState state, boolean calledFromDataflow) { + boolean polyNullModeled = hasPolyNullModel(invokedMethodSymbol, state); // If the return type is not a type variable, just return NONNULL (explicit @Nullable should - // have been handled by the caller) - if (!invokedMethodSymbol.getReturnType().getKind().equals(TypeKind.TYPEVAR)) { + // have been handled by the caller), unless a PolyNull model can change this invocation's + // return qualifier. + if (!invokedMethodSymbol.getReturnType().getKind().equals(TypeKind.TYPEVAR) + && !polyNullModeled) { return Nullness.NONNULL; } - // If generic method invocation - if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { - // Substitute type arguments inside the return type - Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type; - Type substitutedReturnType = - substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow) - .getReturnType(); - // If this condition evaluates to false, we fall through to the subsequent logic, to handle - // type variables declared on the enclosing class - if (substitutedReturnType != null - && Objects.equals(getTypeNullness(substitutedReturnType), Nullness.NULLABLE)) { - return Nullness.NULLABLE; - } - } - - Type enclosingType = - getEnclosingTypeForCallExpression( - invokedMethodSymbol, tree, path, state, calledFromDataflow); - if (enclosingType == null) { - return Nullness.NONNULL; - } else { - return getGenericMethodReturnTypeNullness(invokedMethodSymbol, enclosingType, state); - } + Type.MethodType invokedMethodType = + getInvokedMethodTypeAtCall(invokedMethodSymbol, tree, path, state, calledFromDataflow); + return getTypeNullnessForRead(invokedMethodType.getReturnType(), state); } private static com.sun.tools.javac.util.List convertTreesToTypes( @@ -2992,9 +3080,9 @@ private static com.sun.tools.javac.util.List convertTreesToTypes( * @param path the path to the invocation tree, or null if not available * @param state the visitor state * @param calledFromDataflow whether this method is being called from dataflow analysis - * @return the substituted method type for the generic method + * @return the substituted method type and any PolyNull value inferred in the same solver run */ - private Type substituteTypeArgsInGenericMethodType( + private MethodTypeSubstitution substituteTypeArgsInGenericMethodType( Tree tree, Type.ForAll forAllType, @Nullable TreePath path, @@ -3048,15 +3136,25 @@ private Type substituteTypeArgsInGenericMethodType( nestedNullabilityRepairInProgress.remove(invocationTree); } } - return TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( - methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); + Type.MethodType substitutedMethodType = + TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( + methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); + return new MethodTypeSubstitution( + substitutedMethodType, successResult.polyNullResults().get(invocationTree)); } else { // inference failed; just return the method type at the call site with no substitutions - return methodTypeAtCallSite; + PolyNullInferenceResult failedPolyNullInference = + handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state).isEmpty() + ? null + : new PolyNullInferenceResult(null); + return new MethodTypeSubstitution(methodTypeAtCallSite, failedPolyNullInference); } } - return TypeSubstitutionUtils.subst( - state.getTypes(), methodType, forAllType.tvars, explicitTypeArgs, config); + return new MethodTypeSubstitution( + TypeSubstitutionUtils.subst( + state.getTypes(), methodType, forAllType.tvars, explicitTypeArgs, config) + .asMethodType(), + null); } /** @@ -3276,16 +3374,219 @@ private Type.MethodType getInvokedMethodTypeAtCall( invokedMethodType = TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, methodSymbol, config); } + PolyNullInferenceResult jointlyInferredPolyNull = null; if (tree instanceof MethodInvocationTree && invokedMethodType instanceof Type.ForAll forAllType) { - invokedMethodType = + MethodTypeSubstitution substitution = substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow); + invokedMethodType = substitution.methodType(); + jointlyInferredPolyNull = substitution.polyNullResult(); } - return handler.onOverrideMethodType( - methodSymbol, - invokedMethodType.asMethodType(), - state, - tree instanceof MethodInvocationTree invocationTree ? invocationTree : null); + Type.MethodType modeledMethodType = + handler.onOverrideMethodType( + methodSymbol, + invokedMethodType.asMethodType(), + state, + tree instanceof MethodInvocationTree invocationTree ? invocationTree : null); + return tree instanceof MethodInvocationTree invocationTree + ? applyPolyNullModel( + methodSymbol, + invocationTree, + modeledMethodType, + jointlyInferredPolyNull, + path, + state, + calledFromDataflow) + : modeledMethodType; + } + + /** Returns whether {@code methodSymbol} has a polymorphic-nullness library model. */ + public boolean hasPolyNullModel(Symbol.MethodSymbol methodSymbol, VisitorState state) { + return !handler.onGetPolyNullLocations(methodSymbol, state).isEmpty(); + } + + /** + * Applies a polymorphic-nullness model after receiver and method type arguments have been + * substituted into the invoked method type. The linked nullness is inferred from all modeled + * inputs and any available result target. + */ + private Type.MethodType applyPolyNullModel( + Symbol.MethodSymbol methodSymbol, + MethodInvocationTree invocationTree, + Type.MethodType substitutedMethodType, + @Nullable PolyNullInferenceResult jointlyInferredPolyNull, + @Nullable TreePath path, + VisitorState state, + boolean calledFromDataflow) { + ImmutableSet locations = handler.onGetPolyNullLocations(methodSymbol, state); + if (locations.isEmpty()) { + return substitutedMethodType; + } + Nullness polyNullness = + jointlyInferredPolyNull != null + ? jointlyInferredPolyNull.nullness() + : inferPolyNullness( + methodSymbol, + invocationTree, + substitutedMethodType, + locations, + path, + state, + calledFromDataflow); + if (polyNullness == null) { + return substitutedMethodType; + } + return PolyNullInference.applyToMethodType( + substitutedMethodType, locations, polyNullAnnotationType(polyNullness, state)); + } + + /** + * Infers the nullness shared by all PolyNull occurrences using the generic constraint solver. + * + *

All modeled locations share one synthetic type variable. Ordinary assignment-compatibility + * constraints therefore select the most specific qualifier that makes every argument and the + * invocation result compatible with the instantiated method signature. + */ + private @Nullable Nullness inferPolyNullness( + Symbol.MethodSymbol methodSymbol, + MethodInvocationTree invocationTree, + Type.MethodType substitutedMethodType, + ImmutableSet locations, + @Nullable TreePath path, + VisitorState state, + boolean calledFromDataflow) { + Nullness cached = polyNullResolutions.get(invocationTree); + if (cached != null) { + return cached; + } + PolyNullInferenceContext inferenceContext = + PolyNullInference.createContext( + methodSymbol, + substitutedMethodType, + locations, + getSyntheticNullableAnnotType(state), + state); + if (!inferenceContext.hasInputLocations()) { + return null; + } + ConstraintSolver solver = makeSolver(state, analysis); + Set nestedCalls = new LinkedHashSet<>(); + try { + addPolyNullResultConstraintsFromDirectAssignmentContext( + invocationTree, inferenceContext, path, solver, state, calledFromDataflow); + generateArgumentConstraintsForCall( + state, + path, + solver, + invocationTree, + inferenceContext.inferenceMethodType(), + nestedCalls, + new IdentityHashMap<>(), + calledFromDataflow); + Map solution = solver.solve(); + Nullness resolved = PolyNullInference.resolveContext(inferenceContext, solution); + if (resolved != null && !calledFromDataflow) { + polyNullResolutions.put(invocationTree, resolved); + } + return resolved; + } catch (UnsatisfiableConstraintsException e) { + reportPolyNullInferenceFailure(invocationTree, state); + return null; + } + } + + /** + * Constrains a standalone PolyNull call's modeled result using a directly enclosing assignment, + * variable initialization, method return, or conditional-expression target. + */ + private void addPolyNullResultConstraintsFromDirectAssignmentContext( + MethodInvocationTree invocationTree, + PolyNullInferenceContext inferenceContext, + @Nullable TreePath path, + ConstraintSolver solver, + VisitorState state, + boolean calledFromDataflow) { + if (path == null) { + return; + } + TreePath invocationPath = pathWithLeaf(path, invocationTree); + TreePath parentPath = invocationPath.getParentPath(); + if (parentPath == null) { + return; + } + Tree parent = parentPath.getLeaf(); + while (parent instanceof ParenthesizedTree) { + parentPath = parentPath.getParentPath(); + if (parentPath == null) { + return; + } + parent = parentPath.getLeaf(); + } + if (!(parent instanceof AssignmentTree + || parent instanceof VariableTree + || parent instanceof ReturnTree + || parent instanceof ConditionalExpressionTree)) { + return; + } + CallAndContext callAndContext = + getDirectCallContextForInference(invocationPath, state, calledFromDataflow); + if (callAndContext.typeFromAssignmentContext() != null) { + PolyNullInference.addResultConstraints( + solver, + inferenceContext, + callAndContext.typeFromAssignmentContext(), + callAndContext.assignedToLocal()); + } + } + + /** Generates argument constraints for a call against the supplied method type. */ + private void generateArgumentConstraintsForCall( + VisitorState state, + @Nullable TreePath path, + ConstraintSolver solver, + ExpressionTree callTree, + Type.MethodType methodType, + Set allCalls, + IdentityHashMap polyNullContexts, + boolean calledFromDataflow) { + TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), callTree); + new InvocationArguments(callTree, methodType) + .forEach( + (argument, argPos, formalParamType, unused) -> { + TreePath pathToArgument = new TreePath(pathToCall, argument); + generateConstraintsForPseudoAssignment( + state.withPath(pathToArgument), + solver, + allCalls, + argument, + formalParamType, + polyNullContexts, + calledFromDataflow); + }); + } + + /** Reports unsatisfiable constraints on a PolyNull invocation. */ + private void reportPolyNullInferenceFailure( + MethodInvocationTree invocationTree, VisitorState state) { + if (!callsWithReportedInferenceFailures.add(invocationTree)) { + return; + } + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.GENERIC_INFERENCE_FAILURE, + PolyNullInference.INFERENCE_FAILURE_MESSAGE); + state.reportMatch( + analysis + .getErrorBuilder() + .createErrorDescription( + errorMessage, analysis.buildDescription(invocationTree), state, null)); + } + + /** Returns the synthetic annotation type representing the resolved PolyNull qualifier. */ + private static Type polyNullAnnotationType(Nullness nullness, VisitorState state) { + return nullness == Nullness.NULLABLE + ? getSyntheticNullableAnnotType(state) + : getSyntheticNonNullAnnotType(state); } /** @@ -3332,6 +3633,7 @@ public Nullness getGenericParameterNullnessAtInvocation( Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type; List substitutedParamTypes = substituteTypeArgsInGenericMethodType(tree, forAllType, null, state, false) + .methodType() .getParameterTypes(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class @@ -3607,6 +3909,12 @@ private Nullness getTypeNullnessForRead( if (getTypeNullness(type).equals(Nullness.NULLABLE)) { return Nullness.NULLABLE; } + // A call-specific inference result can explicitly make a captured return type @NonNull even + // when the capture has a nullable upper bound. Honor that resolved qualifier before falling + // back to the upper bound of an otherwise unqualified wildcard or capture. + if (Nullness.hasNonNullAnnotation(type.getAnnotationMirrors().stream(), config)) { + return Nullness.NONNULL; + } if (config.handleWildcardGenerics() && GenericsUtils.asWildcard(type) != null) { Type effectiveUpperBound = GenericsUtils.effectiveWildcardUpperBound(type, state, config, handler); diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java new file mode 100644 index 0000000000..1ea4bddcda --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java @@ -0,0 +1,215 @@ +package com.uber.nullaway.generics; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.VisitorState; +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.util.ListBuffer; +import com.uber.nullaway.LibraryModels.PolyNullLocation; +import com.uber.nullaway.Nullness; +import com.uber.nullaway.librarymodel.NestedTypePathUpdater; +import java.util.ArrayList; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import javax.lang.model.element.Element; +import org.jspecify.annotations.Nullable; + +/** Constructs and applies polymorphic-nullness constraints for modeled method locations. */ +final class PolyNullInference { + + /** Diagnostic for incompatible constraints on modeled PolyNull locations. */ + static final String INFERENCE_FAILURE_MESSAGE = + "inference failure: polymorphic nullness constrained to both @NonNull and @Nullable"; + + /** The result of resolving PolyNull for one invocation in a generic inference session. */ + record PolyNullInferenceResult(@Nullable Nullness nullness) {} + + /** The modeled method-type overlay and shared PolyNull variable for one call. */ + record PolyNullInferenceContext( + Type.MethodType inferenceMethodType, + ImmutableList locations, + Type.TypeVar inferenceVariable) { + + /** Returns whether the overlay contains at least one modeled parameter location. */ + boolean hasInputLocations() { + return locations.stream().anyMatch(location -> location.parameterIndex() >= 0); + } + + /** Returns whether the overlay contains at least one modeled return location. */ + boolean hasReturnLocations() { + return locations.stream().anyMatch(location -> location.parameterIndex() == -1); + } + } + + private PolyNullInference() {} + + /** Applies a resolved PolyNull annotation to every modeled parameter and return location. */ + @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks + static Type.MethodType applyToMethodType( + Type.MethodType methodType, ImmutableSet locations, Type annotationType) { + boolean changed = false; + ListBuffer updatedParameterTypes = new ListBuffer<>(); + int parameterIndex = 0; + for (com.sun.tools.javac.util.List remaining = methodType.argtypes; + remaining.nonEmpty(); + remaining = remaining.tail, parameterIndex++) { + Type parameterType = remaining.head; + Type updatedParameterType = + applyToType(parameterType, parameterIndex, locations, annotationType); + updatedParameterTypes.append(updatedParameterType); + changed |= updatedParameterType != parameterType; + } + Type returnType = methodType.restype; + Type updatedReturnType = applyToReturnType(returnType, locations, annotationType); + changed |= updatedReturnType != returnType; + return changed + ? new Type.MethodType( + updatedParameterTypes.toList(), updatedReturnType, methodType.thrown, methodType.tsym) + : methodType; + } + + /** Applies a resolved PolyNull annotation to every modeled location within a return type. */ + static Type applyToReturnType( + Type returnType, ImmutableSet locations, Type annotationType) { + return applyToType(returnType, -1, locations, annotationType); + } + + /** Resolves all PolyNull contexts after a shared generic-inference solver run. */ + static IdentityHashMap resolveContexts( + IdentityHashMap contexts, + Map solution) { + IdentityHashMap results = + new IdentityHashMap<>(); + for (Map.Entry entry : contexts.entrySet()) { + results.put( + entry.getKey(), new PolyNullInferenceResult(resolveContext(entry.getValue(), solution))); + } + return results; + } + + /** Resolves the shared PolyNull variable for one invocation. */ + static Nullness resolveContext( + PolyNullInferenceContext inferenceContext, + Map solution) { + ConstraintSolver.InferredNullability inferred = + solution.getOrDefault( + inferenceContext.inferenceVariable().asElement(), + ConstraintSolver.InferredNullability.NONNULL); + return inferred == ConstraintSolver.InferredNullability.NULLABLE + ? Nullness.NULLABLE + : Nullness.NONNULL; + } + + /** Creates a method-type overlay with one shared variable at every modeled PolyNull location. */ + @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks + static PolyNullInferenceContext createContext( + Symbol.MethodSymbol methodSymbol, + Type.MethodType methodType, + ImmutableSet locations, + Type nullableAnnotationType, + VisitorState state) { + Map> locationsByParameter = new LinkedHashMap<>(); + ImmutableList.Builder appliedLocations = ImmutableList.builder(); + Type.TypeVar inferenceVariable = + createInferenceVariable(methodSymbol, 0, nullableAnnotationType, state); + for (PolyNullLocation location : locations) { + int parameterIndex = location.parameterIndex(); + if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { + continue; + } + locationsByParameter + .computeIfAbsent(parameterIndex, unused -> new ArrayList<>()) + .add(location); + } + ListBuffer updatedParameterTypes = new ListBuffer<>(); + int parameterIndex = 0; + for (com.sun.tools.javac.util.List remaining = methodType.argtypes; + remaining.nonEmpty(); + remaining = remaining.tail, parameterIndex++) { + Type updated = remaining.head; + for (PolyNullLocation location : + locationsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { + Type replaced = + NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); + if (replaced != updated) { + updated = replaced; + appliedLocations.add(location); + } + } + updatedParameterTypes.append(updated); + } + Type updatedReturnType = methodType.restype; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + Type replaced = + NestedTypePathUpdater.replaceType( + updatedReturnType, location.typePath(), inferenceVariable); + if (replaced != updatedReturnType) { + updatedReturnType = replaced; + appliedLocations.add(location); + } + } + } + return new PolyNullInferenceContext( + new Type.MethodType( + updatedParameterTypes.toList(), updatedReturnType, methodType.thrown, methodType.tsym), + appliedLocations.build(), + inferenceVariable); + } + + /** Adds a call-result subtype constraint using the invocation's shared PolyNull variable. */ + static void addResultConstraints( + ConstraintSolver solver, + PolyNullInferenceContext inferenceContext, + Type targetType, + boolean assignedToLocal) { + if (!inferenceContext.hasReturnLocations()) { + return; + } + solver.addSubtypeConstraint( + inferenceContext.inferenceMethodType().getReturnType(), targetType, assignedToLocal); + } + + /** Returns whether {@code typeVariable} is a PolyNull variable from one of {@code contexts}. */ + static boolean containsInferenceVariable( + IdentityHashMap contexts, + Element typeVariable) { + return contexts.values().stream() + .anyMatch(context -> Objects.equals(context.inferenceVariable().asElement(), typeVariable)); + } + + /** Creates the nullable-bounded synthetic type variable for one PolyNull invocation. */ + private static Type.TypeVar createInferenceVariable( + Symbol.MethodSymbol methodSymbol, + int group, + Type nullableAnnotationType, + VisitorState state) { + Symbol.TypeVariableSymbol symbol = + new Symbol.TypeVariableSymbol( + 0, state.getName("$PolyNull$" + group), Type.noType, methodSymbol); + Type nullableObject = + TypeSubstitutionUtils.typeWithAnnot(state.getSymtab().objectType, nullableAnnotationType); + Type.TypeVar variable = new Type.TypeVar(symbol, nullableObject, state.getSymtab().botType); + symbol.type = variable; + return variable; + } + + /** Applies {@code annotationType} at modeled locations within one method-type component. */ + private static Type applyToType( + Type type, + int parameterIndex, + ImmutableSet locations, + Type annotationType) { + Type updated = type; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == parameterIndex) { + updated = NestedTypePathUpdater.addAnnotation(updated, location.typePath(), annotationType); + } + } + return updated; + } +} 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 1badeb2494..f2d3d5df5f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -41,6 +41,7 @@ import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Context; import com.uber.nullaway.ErrorMessage; +import com.uber.nullaway.LibraryModels.PolyNullLocation; import com.uber.nullaway.MethodParameterNullness; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; @@ -376,6 +377,16 @@ public Type.MethodType onOverrideMethodType( return currentType; } + @Override + public ImmutableSet onGetPolyNullLocations( + Symbol.MethodSymbol methodSymbol, VisitorState state) { + ImmutableSet.Builder result = ImmutableSet.builder(); + for (Handler h : handlers) { + result.addAll(h.onGetPolyNullLocations(methodSymbol, state)); + } + return result.build(); + } + @Override public FieldSkipResult shouldSkipFieldInitializationCheck( Symbol.ClassSymbol classSymbol, Symbol fieldSymbol, VisitorState state) { 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 60a729168e..9833629a14 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -38,6 +38,7 @@ import com.sun.tools.javac.util.Context; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.LibraryModels; +import com.uber.nullaway.LibraryModels.PolyNullLocation; import com.uber.nullaway.MethodParameterNullness; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; @@ -505,6 +506,18 @@ default Type.MethodType onOverrideMethodType( return methodType; } + /** + * Returns modeled polymorphic-nullness locations for {@code methodSymbol}. + * + *

The generic type-checking machinery uses these locations when resolving an invocation. A + * handler should only provide model metadata here; substitution and call-site reasoning belong in + * {@code GenericsChecks}. + */ + default ImmutableSet onGetPolyNullLocations( + Symbol.MethodSymbol methodSymbol, VisitorState state) { + return ImmutableSet.of(); + } + enum FieldSkipResult { /** do not skip the check */ NO, 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 029f754e07..7b0a60182e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -53,6 +53,7 @@ import com.uber.nullaway.Config; import com.uber.nullaway.LibraryModels; import com.uber.nullaway.LibraryModels.MethodRef; +import com.uber.nullaway.LibraryModels.PolyNullLocation; import com.uber.nullaway.MethodParameterNullness; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; @@ -63,7 +64,7 @@ import com.uber.nullaway.handlers.stream.StreamTypeRecord; import com.uber.nullaway.libmodel.NestedAnnotationInfo; import com.uber.nullaway.libmodel.NestedAnnotationInfo.Annotation; -import com.uber.nullaway.librarymodel.AddAnnotationToNestedTypeVisitor; +import com.uber.nullaway.librarymodel.NestedTypePathUpdater; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; @@ -223,8 +224,8 @@ public boolean onOverrideMayBeNullExpr( if (isNullableFieldInLibraryModels(exprSymbol)) { return true; } - if (!(expr instanceof MethodInvocationTree - && exprSymbol instanceof Symbol.MethodSymbol methodSymbol)) { + if (!(expr instanceof MethodInvocationTree) + || !(exprSymbol instanceof Symbol.MethodSymbol methodSymbol)) { return exprMayBeNull; } OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context); @@ -236,6 +237,11 @@ public boolean onOverrideMayBeNullExpr( boolean isMethodUnannotated = getCodeAnnotationInfo(state.context) .isSymbolUnannotated(methodSymbol, this.config, mainHandler); + if (!optLibraryModels.polyNullLocations(methodSymbol, state.getTypes()).isEmpty()) { + // GenericsChecks has already resolved the call-site return type. A fixed nullable/non-null + // return model for the same method must not override that result. + return exprMayBeNull; + } if (exprMayBeNull) { // This is the only case in which we may switch the result from @Nullable to @NonNull: return !optLibraryModels.hasNonNullReturn( @@ -313,6 +319,10 @@ public NullnessHint onDataflowVisitMethodInvocation( } return anyNull ? NullnessHint.HINT_NULLABLE : NullnessHint.FORCE_NONNULL; } + if (!optLibraryModels.polyNullLocations(callee, types).isEmpty()) { + // Let the resolved call-site method type computed by GenericsChecks determine the return. + return NullnessHint.UNKNOWN; + } if (optLibraryModels.hasNonNullReturn(callee, types, !isMethodAnnotated)) { return NullnessHint.FORCE_NONNULL; } else if (optLibraryModels.hasNullableReturn(callee, types, !isMethodAnnotated)) { @@ -500,6 +510,12 @@ public boolean onOverrideMethodNullMarkedness( return config.isSkippedLibraryModel(classDotMethod) ? false : currentNullMarkedness; } + @Override + public ImmutableSet onGetPolyNullLocations( + Symbol.MethodSymbol methodSymbol, VisitorState state) { + return getOptLibraryModels(state.context).polyNullLocations(methodSymbol, state.getTypes()); + } + @Override public boolean isSingleArgNullImpliesFalseMethod( Symbol.MethodSymbol methodSymbol, VisitorState state) { @@ -588,9 +604,8 @@ public Type.MethodType onOverrideMethodType( * represented as {@link NestedAnnotationInfo} library models. */ private static Type applyTopLevelNullableAnnotation(Type type, VisitorState state) { - return new AddAnnotationToNestedTypeVisitor( - ImmutableList.of(), GenericsChecks.getSyntheticNullableAnnotType(state)) - .apply(type); + return NestedTypePathUpdater.addAnnotation( + type, ImmutableList.of(), GenericsChecks.getSyntheticNullableAnnotType(state)); } /** @@ -609,9 +624,7 @@ private static Type applyNestedAnnotations( info.annotation() == Annotation.NULLABLE ? GenericsChecks.getSyntheticNullableAnnotType(state) : GenericsChecks.getSyntheticNonNullAnnotType(state); - AddAnnotationToNestedTypeVisitor addAnnotationToNestedTypeVisitor = - new AddAnnotationToNestedTypeVisitor(info.typePath(), annotType); - updated = addAnnotationToNestedTypeVisitor.apply(updated); + updated = NestedTypePathUpdater.addAnnotation(updated, info.typePath(), annotType); } return updated; } @@ -1205,19 +1218,37 @@ private static class DefaultLibraryModels implements LibraryModels { ImmutableList.of( new NestedAnnotationInfo.TypePathEntry(TYPE_ARGUMENT, 1), new NestedAnnotationInfo.TypePathEntry(WILDCARD_BOUND, 0))))) - // https://github.com/uber/NullAway/issues/1616 - /*.put( + .build(); + + private static final ImmutableSetMultimap POLY_NULL_LOCATIONS = + new ImmutableSetMultimap.Builder() + .put( methodRef( - "java.util.Optional", - "orElseGet(java.util.function.Supplier)"), - ImmutableSetMultimap.of( + "java.util.Optional", "orElseGet(java.util.function.Supplier)"), + new PolyNullLocation( 0, - new NestedAnnotationInfo( - Annotation.NULLABLE, - ImmutableList.of( - new NestedAnnotationInfo.TypePathEntry(TYPE_ARGUMENT, 0), - new NestedAnnotationInfo.TypePathEntry(WILDCARD_BOUND, 0)))))*/ - .build(); + ImmutableList.of( + new NestedAnnotationInfo.TypePathEntry(TYPE_ARGUMENT, 0), + new NestedAnnotationInfo.TypePathEntry(WILDCARD_BOUND, 0)))) + .put( + methodRef( + "java.util.Optional", "orElseGet(java.util.function.Supplier)"), + new PolyNullLocation(-1, ImmutableList.of())) + .put( + methodRef( + "java.util.Map", + "computeIfAbsent(K,java.util.function.Function)"), + new PolyNullLocation( + 1, + ImmutableList.of( + new NestedAnnotationInfo.TypePathEntry(TYPE_ARGUMENT, 1), + new NestedAnnotationInfo.TypePathEntry(WILDCARD_BOUND, 0)))) + .put( + methodRef( + "java.util.Map", + "computeIfAbsent(K,java.util.function.Function)"), + new PolyNullLocation(-1, ImmutableList.of())) + .build(); private static final ImmutableSet NULLMARKED_CLASSES = new ImmutableSet.Builder() @@ -1305,6 +1336,11 @@ public ImmutableSetMultimap methodTypeVariablesWithNullableU return NESTED_ANNOTATIONS_FOR_METHODS; } + @Override + public ImmutableSetMultimap polyNullLocations() { + return POLY_NULL_LOCATIONS; + } + @Override public ImmutableSet nullMarkedClasses() { return NULLMARKED_CLASSES; @@ -1360,6 +1396,8 @@ private static class CombinedLibraryModels implements LibraryModels { private final ImmutableMap> nestedAnnotationsForMethods; + private final ImmutableSetMultimap polyNullLocations; + CombinedLibraryModels(Iterable models, Config config) { this.config = config; ImmutableSetMultimap.Builder failIfNullParametersBuilder = @@ -1390,6 +1428,8 @@ private static class CombinedLibraryModels implements LibraryModels { ImmutableSet.Builder nullableFieldsBuilder = new ImmutableSet.Builder<>(); Map> nestedAnnotationsBuilder = new LinkedHashMap<>(); + ImmutableSetMultimap.Builder polyNullLocationsBuilder = + new ImmutableSetMultimap.Builder<>(); for (LibraryModels libraryModels : models) { for (Map.Entry entry : libraryModels.failIfNullParameters().entries()) { if (shouldSkipModel(entry.getKey())) { @@ -1479,6 +1519,13 @@ private static class CombinedLibraryModels implements LibraryModels { entry.getKey(), key -> new ImmutableSetMultimap.Builder<>()); builder.putAll(entry.getValue()); } + for (Map.Entry entry : + libraryModels.polyNullLocations().entries()) { + if (shouldSkipModel(entry.getKey())) { + continue; + } + polyNullLocationsBuilder.put(entry); + } } failIfNullParameters = failIfNullParametersBuilder.build(); explicitlyNullableParameters = explicitlyNullableParametersBuilder.build(); @@ -1503,6 +1550,7 @@ private static class CombinedLibraryModels implements LibraryModels { nestedAnnotationsForMethodsBuilder.put(entry.getKey(), entry.getValue().build()); } nestedAnnotationsForMethods = nestedAnnotationsForMethodsBuilder.build(); + polyNullLocations = polyNullLocationsBuilder.build(); } private boolean shouldSkipModel(MethodRef key) { @@ -1589,6 +1637,11 @@ public ImmutableList customStreamNullabilitySpecs() { nestedAnnotationsForMethods() { return nestedAnnotationsForMethods; } + + @Override + public ImmutableSetMultimap polyNullLocations() { + return polyNullLocations; + } } /** @@ -1633,6 +1686,7 @@ boolean nameNotPresent(Symbol.MethodSymbol symbol) { private final NameIndexedMap> methodTypeVariablesWithNullableUpperBounds; private final NameIndexedMap> nestedAnnotationsForMethods; + private final NameIndexedMap> polyNullLocations; OptimizedLibraryModels(LibraryModels models, Context context) { Names names = Names.instance(context); @@ -1652,6 +1706,7 @@ boolean nameNotPresent(Symbol.MethodSymbol symbol) { makeOptimizedSetLookup(names, models.methodTypeVariablesWithNullableUpperBounds()); nestedAnnotationsForMethods = makeOptimizedNestedAnnotationLookup(names, models.nestedAnnotationsForMethods()); + polyNullLocations = makeOptimizedSetLookup(names, models.polyNullLocations()); } boolean hasNonNullReturn(Symbol.MethodSymbol symbol, Types types, boolean allowInherited) { @@ -1710,6 +1765,15 @@ ImmutableSetMultimap nestedAnnotationsForMethods( return (result == null) ? ImmutableSetMultimap.of() : result; } + ImmutableSet polyNullLocations(Symbol.MethodSymbol symbol, Types types) { + Symbol.MethodSymbol modelSymbol = + lookupHandlingOverrides( + symbol, types, polyNullLocations, /* allowInheritedModelLookup= */ true); + return modelSymbol == null + ? ImmutableSet.of() + : lookupImmutableSet(modelSymbol, polyNullLocations); + } + private ImmutableSet lookupImmutableSet( Symbol.MethodSymbol symbol, NameIndexedMap> lookup) { ImmutableSet result = lookup.get(symbol); diff --git a/nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java similarity index 76% rename from nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java rename to nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java index ce0c58f881..dd41e5d238 100644 --- a/nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java @@ -11,43 +11,55 @@ import com.uber.nullaway.generics.TypeSubstitutionUtils; import com.uber.nullaway.libmodel.NestedAnnotationInfo; -/** - * A visitor to add an annotation to a type at a specified type path. The desired annotation and - * type path are specified in the constructor. Then, calling {@link #apply(Type)} on a type will - * return a new type with the annotation added at the specified nested location, or the original - * type if no change was made. - */ +/** Updates a type at a nested location identified by a library-model type path. */ @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks -public final class AddAnnotationToNestedTypeVisitor extends Types.MapVisitor { +public final class NestedTypePathUpdater extends Types.MapVisitor { + + private enum UpdateKind { + ADD_ANNOTATION, + REPLACE_TYPE + } + private final ImmutableList typePath; - private final Type annotationType; + private final Type updateType; + private final UpdateKind updateKind; - /** - * Constructor. - * - * @param typePath the type path to the nested type where the annotation should be added - * @param annotationType the annotation type to add - */ - public AddAnnotationToNestedTypeVisitor( - ImmutableList typePath, Type annotationType) { + private NestedTypePathUpdater( + ImmutableList typePath, + Type updateType, + UpdateKind updateKind) { this.typePath = typePath; - this.annotationType = annotationType; + this.updateType = updateType; + this.updateKind = updateKind; } - /** - * Applies this visitor to the given type. - * - * @param type the type to apply the visitor to - * @return the resulting type with the annotation added at the specified nested location - */ - public Type apply(Type type) { + /** Adds {@code annotationType} to {@code type} at {@code typePath}. */ + public static Type addAnnotation( + Type type, ImmutableList typePath, Type annotationType) { + return new NestedTypePathUpdater(typePath, annotationType, UpdateKind.ADD_ANNOTATION) + .apply(type); + } + + /** Replaces the nested type at {@code typePath} with {@code replacement}. */ + public static Type replaceType( + Type type, ImmutableList typePath, Type replacement) { + return new NestedTypePathUpdater(typePath, replacement, UpdateKind.REPLACE_TYPE).apply(type); + } + + private Type apply(Type type) { return type.accept(this, 0); } + private Type updateLeaf(Type type) { + return updateKind == UpdateKind.ADD_ANNOTATION + ? TypeSubstitutionUtils.typeWithAnnot(type, updateType) + : updateType; + } + @Override public Type visitClassType(Type.ClassType t, Integer pathIndex) { if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); + return updateLeaf(t); } NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.TYPE_ARGUMENT) { @@ -75,7 +87,7 @@ public Type visitClassType(Type.ClassType t, Integer pathIndex) { @Override public Type visitArrayType(Type.ArrayType t, Integer pathIndex) { if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); + return updateLeaf(t); } NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.ARRAY_ELEMENT) { @@ -91,10 +103,10 @@ public Type visitArrayType(Type.ArrayType t, Integer pathIndex) { @Override public Type visitWildcardType(Type.WildcardType t, Integer pathIndex) { if (pathIndex == typePath.size()) { - // Nullness annotations directly on wildcards are not legal under JSpecify. This case can + // Nullness annotations directly on wildcards are not legal under JSpecify. This case can // arise when member-type substitution replaces an annotated type variable with a wildcard; // leave the wildcard unchanged and rely on the dedicated top-level parameter/return model. - return t; + return updateKind == UpdateKind.ADD_ANNOTATION ? t : updateLeaf(t); } NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.WILDCARD_BOUND) { @@ -126,16 +138,12 @@ public Type visitWildcardType(Type.WildcardType t, Integer pathIndex) { return t; } - /** - * Updates a captured type while preserving the backing wildcard used by NullAway's wildcard-bound - * reasoning. - * - *

javac represents a captured wildcard as a type variable plus its original wildcard. A direct - * annotation on the capture is insufficient because effective-bound computations unwrap the - * backing wildcard, so updates must be reflected there. - */ + /** Updates a captured type while preserving its backing wildcard when adding an annotation. */ @Override public Type visitCapturedType(Type.CapturedType t, Integer pathIndex) { + if (updateKind == UpdateKind.REPLACE_TYPE && pathIndex == typePath.size()) { + return updateLeaf(t); + } Type.WildcardType wildcard = t.wildcard; if (wildcard.kind == BoundKind.UNBOUND && wildcard.bound == null) { // javac can omit the formal type variable on a captured wildcard. Use the capture's upper @@ -154,11 +162,11 @@ public Type visitCapturedType(Type.CapturedType t, Integer pathIndex) { Verify.verifyNotNull( wildcard.bound, "unbounded wildcard has no corresponding formal type variable"); Type updatedUpperBound = - TypeSubstitutionUtils.typeWithAnnot(formalTypeVariable.getUpperBound(), annotationType); + TypeSubstitutionUtils.typeWithAnnot(formalTypeVariable.getUpperBound(), updateType); updatedWildcard = TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(wildcard, updatedUpperBound); } else { - Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(wildcard.type, annotationType); + Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(wildcard.type, updateType); updatedWildcard = TYPE_METADATA_BUILDER.createWildcardType(wildcard, updatedBound); } } @@ -171,7 +179,7 @@ public Type visitCapturedType(Type.CapturedType t, Integer pathIndex) { @Override public Type visitType(Type t, Integer pathIndex) { if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); + return updateLeaf(t); } return t; } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyLibraryModelsTests.java index a6dcd52dd2..63d13ab42c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyLibraryModelsTests.java @@ -4,7 +4,6 @@ import com.uber.nullaway.NullAwayTestsBase; import com.uber.nullaway.generics.JSpecifyJavacConfig; import java.util.Arrays; -import org.junit.Ignore; import org.junit.Test; public class JSpecifyLibraryModelsTests extends NullAwayTestsBase { @@ -98,7 +97,6 @@ Optional mapCanReturnNullable(Optional value) { .doTest(); } - @Ignore("https://github.com/uber/NullAway/issues/1616") @Test public void optionalOrElseGet() { makeHelper() @@ -112,6 +110,8 @@ public void optionalOrElseGet() { @NullMarked class Test { void orElseGetReturnsNullable(Optional value) { + value.orElseGet(() -> "fallback").hashCode(); + // BUG: Diagnostic contains: dereferenced expression 'value.orElseGet(() -> null)' is @Nullable value.orElseGet(() -> null).hashCode(); } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java new file mode 100644 index 0000000000..c349f39354 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -0,0 +1,486 @@ +package com.uber.nullaway.jspecify; + +import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; +import com.uber.nullaway.generics.JSpecifyJavacConfig; +import java.util.List; +import org.junit.Test; + +/** + * Specification tests for {@code @PolyNull}-like library models. + * + *

These tests cover named functional-interface values, lambda and method-reference inference, + * inferred and explicit method type arguments, and propagation through {@code var} locals. + * + * @see NullAway issue #1616 + */ +public class PolyNullLibraryModelsTests extends NullAwayTestsBase { + + @Test + public void optionalOrElseGetWithExplicitSupplierTypeArgument() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Optional; + import java.util.function.Supplier; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + Optional optional, + Supplier nonNullSupplier, + Supplier<@Nullable String> nullableSupplier) { + optional.orElseGet(nonNullSupplier).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'optional.orElseGet(nullableSupplier)' is @Nullable + optional.orElseGet(nullableSupplier).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void optionalOrElseGetWithExplicitSupplierTypeArgumentAndVarResult() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Optional; + import java.util.function.Supplier; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + Optional optional, + Supplier nonNullSupplier, + Supplier<@Nullable String> nullableSupplier) { + var nonNullResult = optional.orElseGet(nonNullSupplier); + nonNullResult.hashCode(); + + var nullableResult = optional.orElseGet(nullableSupplier); + // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable + nullableResult.hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void optionalOrElseGetUsesFieldAssignmentTarget() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Optional; + import java.util.function.Supplier; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + private String field = "initial"; + + void test( + Optional optional, + Supplier nonNullSupplier, + Supplier<@Nullable String> nullableSupplier) { + field = optional.orElseGet(nonNullSupplier); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + field = optional.orElseGet(nullableSupplier); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentWithExplicitFunctionTypeArgument() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import java.util.function.Function; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + Map map, + Function nonNullFunction, + Function nullableFunction) { + map.computeIfAbsent("key", nonNullFunction).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'map.computeIfAbsent("key", nullableFunction)' is @Nullable + map.computeIfAbsent("key", nullableFunction).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentWithExplicitFunctionTypeArgumentAndVarResult() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import java.util.function.Function; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + Map map, + Function nonNullFunction, + Function nullableFunction) { + var nonNullResult = map.computeIfAbsent("key", nonNullFunction); + nonNullResult.hashCode(); + + var nullableResult = map.computeIfAbsent("key", nullableFunction); + // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable + nullableResult.hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentModelAppliesToOverride() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.HashMap; + import java.util.function.Function; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + HashMap map, + Function nonNullFunction, + Function nullableFunction) { + map.computeIfAbsent("key", nonNullFunction).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'map.computeIfAbsent("key", nullableFunction)' is @Nullable + map.computeIfAbsent("key", nullableFunction).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentWithLambdaAndVarResult() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import org.jspecify.annotations.NullMarked; + + @NullMarked + class Test { + void test(Map map) { + var nonNullResult = map.computeIfAbsent("key", unused -> "value"); + nonNullResult.hashCode(); + + var nullableResult = map.computeIfAbsent("key", unused -> null); + // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable + nullableResult.hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentOverridesNullableMapValueType() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test(Map map) { + map.computeIfAbsent("foo", unused -> "bar").hashCode(); + + var result = map.computeIfAbsent("foo", unused -> "bar"); + result.hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'map.computeIfAbsent("foo", unused -> null)' is @Nullable + map.computeIfAbsent("foo", unused -> null).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentUsesFieldAssignmentTarget() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + private String field = "initial"; + + void test(Map map) { + field = map.computeIfAbsent("nonNull", unused -> "value"); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + field = map.computeIfAbsent("nullable", unused -> null); + } + } + """) + .doTest(); + } + + @Test + public void mapComputeIfAbsentWithMethodReference() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import java.util.Map; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + static String nonNullMapping(String unused) { + return "value"; + } + + static @Nullable String nullableMapping(String unused) { + return null; + } + + void test(Map map) { + map.computeIfAbsent("key", Test::nonNullMapping).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'map.computeIfAbsent("key", Test::nullableMapping)' is @Nullable + map.computeIfAbsent("key", Test::nullableMapping).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void customLibraryModelWithMultipleExplicitTypeArguments() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import java.util.List; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test( + List firstNonNull, + List secondNonNull, + List<@Nullable Object> firstNullable, + List<@Nullable Object> secondNullable) { + PolyNullMethods.first(firstNonNull, secondNonNull).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.first(firstNullable, secondNullable)' is @Nullable + PolyNullMethods.first(firstNullable, secondNullable).hashCode(); + + var nonNullResult = PolyNullMethods.first(firstNonNull, secondNonNull); + nonNullResult.hashCode(); + + var nullableResult = PolyNullMethods.first(firstNullable, secondNullable); + // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable + nullableResult.hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void customLibraryModelRejectsIncompatibleInvariantArguments() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import java.util.List; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test(List nonNull, List<@Nullable Object> nullable) { + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.first(nonNull, nullable); + } + } + """) + .doTest(); + } + + @Test + public void genericMethodAcceptsCompatibleExplicitTypeArguments() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test(String nonNull, @Nullable String nullable) { + PolyNullMethods.twoTypeVariables(nonNull, nonNull); + PolyNullMethods.<@Nullable String, @Nullable String>twoTypeVariables( + nonNull, nonNull); + PolyNullMethods.twoTypeVariables(nonNull, nonNull); + PolyNullMethods.<@Nullable String, String>twoTypeVariables(nonNull, nonNull); + PolyNullMethods.twoTypeVariables(nonNull, nullable); + PolyNullMethods.<@Nullable String, String>twoTypeVariables(nullable, nonNull); + } + } + """) + .doTest(); + } + + @Test + public void genericMethodInfersTypeArgumentsAndPolyNullTogether() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + void test(String nonNull, @Nullable String nullable) { + PolyNullMethods.genericFirst(nonNull, nonNull).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFirst(nullable, nullable)' is @Nullable + PolyNullMethods.genericFirst(nullable, nullable).hashCode(); + + var nonNullResult = PolyNullMethods.genericFirst(nonNull, nonNull); + nonNullResult.hashCode(); + + var nullableResult = PolyNullMethods.genericFirst(nullable, nullable); + // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable + nullableResult.hashCode(); + + PolyNullMethods.twoTypeVariables(nonNull, nullable); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFirst(nonNull, nullable)' is @Nullable + PolyNullMethods.genericFirst(nonNull, nullable).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFirst(nullable, nonNull)' is @Nullable + PolyNullMethods.genericFirst(nullable, nonNull).hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void genericMethodUsesFieldAssignmentTargetForPolyNull() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + private Object field = new Object(); + + void test(Object nonNull, @Nullable Object nullable) { + field = PolyNullMethods.genericObject(nonNull, nonNull); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + field = PolyNullMethods.genericObject(nullable, nullable); + } + } + """) + .doTest(); + } + + @Test + public void genericMethodInfersPolyNullFromLambdasAndMethodReferences() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import com.uber.lib.unannotated.PolyNullMethods; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + + @NullMarked + class Test { + static String nonNullValue() { + return "value"; + } + + static @Nullable String nullableValue() { + return null; + } + + void test() { + PolyNullMethods.genericFromSuppliers(() -> "first", () -> "second").hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFromSuppliers(() -> null, () -> null)' is @Nullable + PolyNullMethods.genericFromSuppliers(() -> null, () -> null).hashCode(); + + PolyNullMethods.genericFromSuppliers( + Test::nonNullValue, Test::nonNullValue).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFromSuppliers(Test::nullableValue, Test::nullableValue)' is @Nullable + PolyNullMethods.genericFromSuppliers(Test::nullableValue, Test::nullableValue).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFromSuppliers(() -> "first", () -> null)' is @Nullable + PolyNullMethods.genericFromSuppliers(() -> "first", () -> null).hashCode(); + + // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFromSuppliers(Test::nonNullValue, Test::nullableValue)' is @Nullable + PolyNullMethods.genericFromSuppliers(Test::nonNullValue, Test::nullableValue).hashCode(); + } + } + """) + .doTest(); + } + + private CompilationTestHelper makeHelper() { + return makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs(List.of("-XepOpt:NullAway:OnlyNullMarked=true"))); + } +} diff --git a/test-java-lib/src/main/java/com/uber/lib/unannotated/PolyNullMethods.java b/test-java-lib/src/main/java/com/uber/lib/unannotated/PolyNullMethods.java new file mode 100644 index 0000000000..565404e75b --- /dev/null +++ b/test-java-lib/src/main/java/com/uber/lib/unannotated/PolyNullMethods.java @@ -0,0 +1,37 @@ +package com.uber.lib.unannotated; + +import java.util.List; +import java.util.function.Supplier; + +/* @NullMarked */ +public final class PolyNullMethods { + + private PolyNullMethods() {} + + /** Returns the first element available from either list. */ + public static /* @PolyNull */ Object first( + List first, List second) { + return first.isEmpty() ? second.get(0) : first.get(0); + } + + /** Accepts two independently typed arguments. */ + public static void twoTypeVariables(/* @PolyNull */ T first, /* @PolyNull */ U second) {} + + /** Returns the first of two independently typed arguments. */ + public static /* @PolyNull */ T genericFirst( + /* @PolyNull */ T first, /* @PolyNull */ U second) { + return first; + } + + /** Returns the first argument as an object. */ + public static /* @PolyNull */ Object genericObject( + /* @PolyNull */ T first, /* @PolyNull */ U second) { + return first; + } + + /** Returns a value from the first of two independently typed suppliers. */ + public static /* @PolyNull */ T genericFromSuppliers( + Supplier first, Supplier second) { + return first.get(); + } +} diff --git a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java index a4b57389f6..bed0840aa2 100644 --- a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java +++ b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.uber.nullaway.LibraryModels; +import com.uber.nullaway.LibraryModels.PolyNullLocation; import com.uber.nullaway.handlers.stream.StreamModelBuilder; import com.uber.nullaway.handlers.stream.StreamTypeRecord; import com.uber.nullaway.libmodel.NestedAnnotationInfo; @@ -59,6 +60,8 @@ public class TestLibraryModels implements LibraryModels { createMethodTypeVariablesWithNullableUpperBounds(); private static final ImmutableMap> NESTED_ANNOTATIONS_FOR_METHODS = createNestedAnnotationsForMethods(); + private static final ImmutableSetMultimap POLY_NULL_LOCATIONS = + createPolyNullLocations(); @Override public ImmutableSetMultimap failIfNullParameters() { @@ -263,6 +266,7 @@ private static ImmutableSet createNullMarkedClasses() { "com.uber.lib.unannotated.LambdaModel", "com.uber.lib.unannotated.NestedAnnots", "com.uber.lib.unannotated.NullMarkedVarargsWithModel", + "com.uber.lib.unannotated.PolyNullMethods", "com.uber.lib.unannotated.UnboundWildcards"); } @@ -274,11 +278,29 @@ public ImmutableSetMultimap methodTypeVariablesWithNullableU /** Creates the immutable method type-variable models used by this test provider. */ private static ImmutableSetMultimap createMethodTypeVariablesWithNullableUpperBounds() { - return ImmutableSetMultimap.of( - methodRef("com.uber.lib.unannotated.ProviderNullMarkedViaModel", "of(U)"), - 0, - methodRef("com.uber.lib.unannotated.NestedAnnots", "genericMethod(java.lang.Class)"), - 0); + return new ImmutableSetMultimap.Builder() + .put(methodRef("com.uber.lib.unannotated.ProviderNullMarkedViaModel", "of(U)"), 0) + .put( + methodRef( + "com.uber.lib.unannotated.NestedAnnots", "genericMethod(java.lang.Class)"), + 0) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "twoTypeVariables(T,U)"), 0) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "twoTypeVariables(T,U)"), 1) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericFirst(T,U)"), 0) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericFirst(T,U)"), 1) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericObject(T,U)"), 0) + .put(methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericObject(T,U)"), 1) + .put( + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "genericFromSuppliers(java.util.function.Supplier,java.util.function.Supplier)"), + 0) + .put( + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "genericFromSuppliers(java.util.function.Supplier,java.util.function.Supplier)"), + 1) + .build(); } @Override @@ -287,6 +309,69 @@ public ImmutableSetMultimap methodTypeVariablesWithNullableU return NESTED_ANNOTATIONS_FOR_METHODS; } + @Override + public ImmutableSetMultimap polyNullLocations() { + return POLY_NULL_LOCATIONS; + } + + /** Creates polymorphic-nullness models used to test custom library-model providers. */ + private static ImmutableSetMultimap createPolyNullLocations() { + MethodRef method = + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "first(java.util.List,java.util.List)"); + return new ImmutableSetMultimap.Builder() + .put(method, new PolyNullLocation(0, ImmutableList.of(new TypePathEntry(TYPE_ARGUMENT, 0)))) + .put(method, new PolyNullLocation(1, ImmutableList.of(new TypePathEntry(TYPE_ARGUMENT, 0)))) + .put(method, new PolyNullLocation(-1, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "twoTypeVariables(T,U)"), + new PolyNullLocation(0, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "twoTypeVariables(T,U)"), + new PolyNullLocation(1, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericFirst(T,U)"), + new PolyNullLocation(0, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericFirst(T,U)"), + new PolyNullLocation(1, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericFirst(T,U)"), + new PolyNullLocation(-1, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericObject(T,U)"), + new PolyNullLocation(0, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericObject(T,U)"), + new PolyNullLocation(1, ImmutableList.of())) + .put( + methodRef("com.uber.lib.unannotated.PolyNullMethods", "genericObject(T,U)"), + new PolyNullLocation(-1, ImmutableList.of())) + .put( + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "genericFromSuppliers(java.util.function.Supplier,java.util.function.Supplier)"), + new PolyNullLocation( + 0, + ImmutableList.of( + new TypePathEntry(TYPE_ARGUMENT, 0), new TypePathEntry(WILDCARD_BOUND, 0)))) + .put( + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "genericFromSuppliers(java.util.function.Supplier,java.util.function.Supplier)"), + new PolyNullLocation( + 1, + ImmutableList.of( + new TypePathEntry(TYPE_ARGUMENT, 0), new TypePathEntry(WILDCARD_BOUND, 0)))) + .put( + methodRef( + "com.uber.lib.unannotated.PolyNullMethods", + "genericFromSuppliers(java.util.function.Supplier,java.util.function.Supplier)"), + new PolyNullLocation(-1, ImmutableList.of())) + .build(); + } + /** Creates the immutable nested-annotation models used by this test provider. */ private static ImmutableMap> createNestedAnnotationsForMethods() {