From 86c3399643a4b30daa6728c06fecb0862a7cf34d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 07:27:09 -0700 Subject: [PATCH 01/17] WIP --- .../java/com/uber/nullaway/LibraryModels.java | 34 ++ .../main/java/com/uber/nullaway/NullAway.java | 4 +- .../nullaway/generics/ConstraintSolver.java | 9 + .../generics/ConstraintSolverImpl.java | 8 + .../nullaway/generics/GenericsChecks.java | 412 +++++++++++++++++- .../nullaway/handlers/CompositeHandler.java | 11 + .../com/uber/nullaway/handlers/Handler.java | 13 + .../handlers/LibraryModelsHandler.java | 93 +++- .../jspecify/JSpecifyLibraryModelsTests.java | 4 +- .../jspecify/PolyNullLibraryModelsTests.java | 328 ++++++++++++++ .../uber/lib/unannotated/PolyNullMethods.java | 17 + .../testlibrarymodels/TestLibraryModels.java | 42 +- 12 files changed, 949 insertions(+), 26 deletions(-) create mode 100644 nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java create mode 100644 test-java-lib/src/main/java/com/uber/lib/unannotated/PolyNullMethods.java 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/ConstraintSolver.java b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java index 5b74181b70..325a9c4f98 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java @@ -64,6 +64,15 @@ public boolean isCausedByNonNullUpperBound() { void addSubtypeConstraint(Type subtype, Type supertype, boolean localVariableType) throws UnsatisfiableConstraintsException; + /** + * Constrains {@code first} and {@code second} to have identical nullability. + * + *

This is stronger than Java subtyping and is used when an explicitly written type argument + * fixes the value of a qualifier-inference variable. + */ + void addNullabilityEqualityConstraint(Type first, Type second) + throws UnsatisfiableConstraintsException; + enum InferredNullability { NONNULL, NULLABLE diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java index f1f9d3b51d..8fe393770f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java @@ -97,6 +97,14 @@ public void addSubtypeConstraint(Type subtype, Type supertype, boolean localVari subtype.accept(new AddSubtypeConstraintsVisitor(localVariableType), supertype); } + @Override + public void addNullabilityEqualityConstraint(Type first, Type second) + throws UnsatisfiableConstraintsException { + AddSubtypeConstraintsVisitor visitor = new AddSubtypeConstraintsVisitor(false); + first.accept(visitor, second); + second.accept(visitor, first); + } + class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Void, Type> { private boolean localVariableType; 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 1117364b9d..853b7c3b8b 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,8 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotatedTypeTree; @@ -42,6 +44,7 @@ import com.sun.tools.javac.code.Types; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.TreeInfo; +import com.sun.tools.javac.util.ListBuffer; import com.sun.tools.javac.util.Name; import com.sun.tools.javac.util.Names; import com.uber.nullaway.CodeAnnotationInfo; @@ -49,6 +52,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; @@ -58,8 +62,11 @@ import com.uber.nullaway.generics.ConstraintSolver.UnsatisfiableConstraintsException; import com.uber.nullaway.generics.GenericsUtils.MethodRefTypeRelationKind; import com.uber.nullaway.handlers.Handler; +import com.uber.nullaway.libmodel.NestedAnnotationInfo.TypePathEntry; +import com.uber.nullaway.librarymodel.AddAnnotationToNestedTypeVisitor; import java.util.ArrayList; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -123,6 +130,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 @@ -2963,6 +2974,11 @@ public Nullness getGenericReturnNullnessAtInvocation( TreePath path, VisitorState state, boolean calledFromDataflow) { + if (hasPolyNullModel(invokedMethodSymbol, state)) { + Type.MethodType invokedMethodType = + getInvokedMethodTypeAtCall(invokedMethodSymbol, tree, path, state, calledFromDataflow); + return getTypeNullnessForRead(invokedMethodType.getReturnType(), 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)) { @@ -3301,11 +3317,397 @@ private Type.MethodType getInvokedMethodTypeAtCall( invokedMethodType = substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow); } - 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, 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 + * input occurrences. + */ + @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) + private Type.MethodType applyPolyNullModel( + Symbol.MethodSymbol methodSymbol, + MethodInvocationTree invocationTree, + Type.MethodType substitutedMethodType, + @Nullable TreePath path, + VisitorState state, + boolean calledFromDataflow) { + ImmutableSet locations = handler.onGetPolyNullLocations(methodSymbol, state); + if (locations.isEmpty()) { + return substitutedMethodType; + } + Nullness polyNullness = + inferPolyNullness( + methodSymbol, + invocationTree, + substitutedMethodType, + locations, + path, + state, + calledFromDataflow); + if (polyNullness == null) { + return substitutedMethodType; + } + boolean changed = false; + ListBuffer updatedParameterTypes = new ListBuffer<>(); + int parameterIndex = 0; + for (com.sun.tools.javac.util.List remaining = substitutedMethodType.argtypes; + remaining.nonEmpty(); + remaining = remaining.tail, parameterIndex++) { + Type parameterType = remaining.head; + Type updatedParameterType = parameterType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == parameterIndex) { + updatedParameterType = + applyPolyNullAnnotation( + updatedParameterType, location.typePath(), polyNullness, state); + } + } + updatedParameterTypes.append(updatedParameterType); + changed |= updatedParameterType != parameterType; + } + Type returnType = substitutedMethodType.restype; + Type updatedReturnType = returnType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + updatedReturnType = + applyPolyNullAnnotation(updatedReturnType, location.typePath(), polyNullness, state); + } + } + changed |= updatedReturnType != returnType; + return changed + ? new Type.MethodType( + updatedParameterTypes.toList(), + updatedReturnType, + substitutedMethodType.thrown, + substitutedMethodType.tsym) + : substitutedMethodType; + } + + /** One modeled input occurrence and the synthetic inference variable that represents it. */ + private record PolyNullInput( + PolyNullLocation location, Type.TypeVar inferenceVariable, Type substitutedLocationType) {} + + /** + * The method type and occurrence variables used to generate PolyNull constraints for one call. + */ + private record PolyNullInferenceContext( + Type.MethodType inferenceMethodType, ImmutableList inputs) {} + + /** + * Infers the nullness shared by all PolyNull occurrences using the generic constraint solver. + * + *

Each modeled input occurrence receives a separate synthetic type variable while constraints + * are generated. This lets a lambda with several return expressions infer one joined nullness for + * its occurrence. After solving, every occurrence must have the same solution, implementing the + * linked-overload semantics of PolyNull. + */ + 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 = + createPolyNullInferenceContext(methodSymbol, substitutedMethodType, locations, state); + if (inferenceContext.inputs().isEmpty()) { + return null; + } + ConstraintSolver solver = makeSolver(state, analysis); + Set nestedCalls = new LinkedHashSet<>(); + try { + seedPolyNullFromExplicitMethodTypeArguments( + methodSymbol, invocationTree, inferenceContext.inputs(), solver); + generateConstraintsForCallWithMethodType( + state, + path, + solver, + invocationTree, + inferenceContext.inferenceMethodType(), + nestedCalls, + calledFromDataflow); + Map solution = solver.solve(); + Nullness resolved = null; + for (PolyNullInput input : inferenceContext.inputs()) { + ConstraintSolver.InferredNullability inferred = + solution.getOrDefault( + input.inferenceVariable().asElement(), + ConstraintSolver.InferredNullability.NONNULL); + Nullness current = + inferred == ConstraintSolver.InferredNullability.NULLABLE + ? Nullness.NULLABLE + : Nullness.NONNULL; + if (resolved != null && resolved != current) { + reportPolyNullInferenceFailure(invocationTree, state); + return null; + } + resolved = current; + } + if (resolved != null && !calledFromDataflow) { + polyNullResolutions.put(invocationTree, resolved); + } + return resolved; + } catch (UnsatisfiableConstraintsException e) { + reportPolyNullInferenceFailure(invocationTree, state); + return null; + } + } + + /** Creates independent inference variables at all modeled PolyNull input locations. */ + private PolyNullInferenceContext createPolyNullInferenceContext( + Symbol.MethodSymbol methodSymbol, + Type.MethodType substitutedMethodType, + ImmutableSet locations, + VisitorState state) { + Map> inputsByParameter = new LinkedHashMap<>(); + ImmutableList.Builder allInputs = ImmutableList.builder(); + int occurrence = 0; + for (PolyNullLocation location : locations) { + int parameterIndex = location.parameterIndex(); + if (parameterIndex < 0 || parameterIndex >= substitutedMethodType.argtypes.size()) { + continue; + } + Type parameterType = substitutedMethodType.argtypes.get(parameterIndex); + Type locationType = typeAtPath(parameterType, location.typePath(), 0); + if (locationType == null) { + continue; + } + Type.TypeVar inferenceVariable = + createPolyNullInferenceVariable(methodSymbol, occurrence++, state); + PolyNullInput input = new PolyNullInput(location, inferenceVariable, locationType); + inputsByParameter.computeIfAbsent(parameterIndex, unused -> new ArrayList<>()).add(input); + allInputs.add(input); + } + ListBuffer updatedParameterTypes = new ListBuffer<>(); + int parameterIndex = 0; + for (com.sun.tools.javac.util.List remaining = substitutedMethodType.argtypes; + remaining.nonEmpty(); + remaining = remaining.tail, parameterIndex++) { + Type updated = remaining.head; + for (PolyNullInput input : + inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { + updated = + replaceTypeAtPath(updated, input.location().typePath(), 0, input.inferenceVariable()); + } + updatedParameterTypes.append(updated); + } + return new PolyNullInferenceContext( + new Type.MethodType( + updatedParameterTypes.toList(), + substitutedMethodType.restype, + substitutedMethodType.thrown, + substitutedMethodType.tsym), + allInputs.build()); + } + + /** Creates a nullable-bounded synthetic type variable for one PolyNull input occurrence. */ + private static Type.TypeVar createPolyNullInferenceVariable( + Symbol.MethodSymbol methodSymbol, int occurrence, VisitorState state) { + Symbol.TypeVariableSymbol symbol = + new Symbol.TypeVariableSymbol( + 0, state.getName("$PolyNull$" + occurrence), Type.noType, methodSymbol); + Type nullableObject = + TypeSubstitutionUtils.typeWithAnnot( + state.getSymtab().objectType, getSyntheticNullableAnnotType(state)); + Type.TypeVar variable = new Type.TypeVar(symbol, nullableObject, state.getSymtab().botType); + symbol.type = variable; + return variable; + } + + /** + * Seeds occurrence variables when a modeled location denotes an explicitly instantiated method + * type variable. + */ + private void seedPolyNullFromExplicitMethodTypeArguments( + Symbol.MethodSymbol methodSymbol, + MethodInvocationTree invocationTree, + ImmutableList inputs, + ConstraintSolver solver) { + if (invocationTree.getTypeArguments().isEmpty()) { + return; + } + Type.MethodType declaredMethodType = methodSymbol.type.asMethodType(); + ImmutableSet methodTypeVariables = + ImmutableSet.copyOf(methodSymbol.getTypeParameters()); + for (PolyNullInput input : inputs) { + int parameterIndex = input.location().parameterIndex(); + if (parameterIndex >= declaredMethodType.argtypes.size()) { + continue; + } + Type declaredLocationType = + typeAtPath( + declaredMethodType.argtypes.get(parameterIndex), input.location().typePath(), 0); + if (declaredLocationType instanceof Type.TypeVar typeVariable + && methodTypeVariables.contains(typeVariable.tsym)) { + solver.addNullabilityEqualityConstraint( + input.substitutedLocationType(), input.inferenceVariable()); + } + } + } + + /** Generates ordinary call constraints using a method type containing PolyNull variables. */ + private void generateConstraintsForCallWithMethodType( + VisitorState state, + @Nullable TreePath path, + ConstraintSolver solver, + MethodInvocationTree invocationTree, + Type.MethodType methodType, + Set allCalls, + boolean calledFromDataflow) { + TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), invocationTree); + new InvocationArguments(invocationTree, methodType) + .forEach( + (argument, argPos, formalParamType, unused) -> { + TreePath pathToArgument = new TreePath(pathToCall, argument); + generateConstraintsForPseudoAssignment( + state.withPath(pathToArgument), + solver, + allCalls, + argument, + formalParamType, + calledFromDataflow); + }); + } + + /** Reports a contradiction between independently inferred PolyNull input occurrences. */ + private void reportPolyNullInferenceFailure( + MethodInvocationTree invocationTree, VisitorState state) { + if (!callsWithReportedInferenceFailures.add(invocationTree)) { + return; + } + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.GENERIC_INFERENCE_FAILURE, + "inference failure: polymorphic nullness constrained to both @NonNull and @Nullable"); + state.reportMatch( + analysis + .getErrorBuilder() + .createErrorDescription( + errorMessage, analysis.buildDescription(invocationTree), state, null)); + } + + /** Returns the nested type at {@code typePath}, or {@code null} if the path is inapplicable. */ + private static @Nullable Type typeAtPath( + Type type, ImmutableList typePath, int pathIndex) { + if (pathIndex == typePath.size()) { + return type; + } + TypePathEntry entry = typePath.get(pathIndex); + return switch (entry.kind()) { + case TYPE_ARGUMENT -> + type instanceof Type.ClassType classType + && entry.index() >= 0 + && entry.index() < classType.getTypeArguments().size() + ? typeAtPath(classType.getTypeArguments().get(entry.index()), typePath, pathIndex + 1) + : null; + case WILDCARD_BOUND -> { + Type bound = wildcardBound(type, entry.index()); + yield bound == null ? null : typeAtPath(bound, typePath, pathIndex + 1); + } + case ARRAY_ELEMENT -> + type instanceof Type.ArrayType arrayType + ? typeAtPath(arrayType.getComponentType(), typePath, pathIndex + 1) + : null; + }; + } + + /** Replaces the nested type at {@code typePath} with {@code replacement}. */ + private static Type replaceTypeAtPath( + Type type, ImmutableList typePath, int pathIndex, Type replacement) { + if (pathIndex == typePath.size()) { + return replacement; + } + TypePathEntry entry = typePath.get(pathIndex); + return switch (entry.kind()) { + case TYPE_ARGUMENT -> { + if (!(type instanceof Type.ClassType classType) + || entry.index() < 0 + || entry.index() >= classType.getTypeArguments().size()) { + yield type; + } + ListBuffer updatedArguments = new ListBuffer<>(); + int argumentIndex = 0; + for (Type argument : classType.getTypeArguments()) { + updatedArguments.add( + argumentIndex++ == entry.index() + ? replaceTypeAtPath(argument, typePath, pathIndex + 1, replacement) + : argument); + } + yield TYPE_METADATA_BUILDER.createClassType( + classType, classType.getEnclosingType(), updatedArguments.toList()); + } + case WILDCARD_BOUND -> { + if (!(type instanceof Type.WildcardType wildcardType)) { + yield type; + } + Type bound = wildcardBound(wildcardType, entry.index()); + if (bound == null) { + yield type; + } + Type updatedBound = replaceTypeAtPath(bound, typePath, pathIndex + 1, replacement); + if (wildcardType.kind == BoundKind.UNBOUND) { + yield TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound( + wildcardType, updatedBound); + } + yield TYPE_METADATA_BUILDER.createWildcardType(wildcardType, updatedBound); + } + case ARRAY_ELEMENT -> + type instanceof Type.ArrayType arrayType + ? TYPE_METADATA_BUILDER.createArrayType( + arrayType, + replaceTypeAtPath( + arrayType.getComponentType(), typePath, pathIndex + 1, replacement)) + : type; + }; + } + + /** Returns the requested bound of a wildcard, or {@code null} if that bound is unavailable. */ + private static @Nullable Type wildcardBound(Type type, int boundIndex) { + if (!(type instanceof Type.WildcardType wildcardType)) { + return null; + } + if (boundIndex == 0) { + if (wildcardType.kind == BoundKind.EXTENDS) { + return wildcardType.type; + } + if (wildcardType.kind == BoundKind.UNBOUND && wildcardType.bound != null) { + return wildcardType.bound.getUpperBound(); + } + } else if (boundIndex == 1 && wildcardType.kind == BoundKind.SUPER) { + return wildcardType.type; + } + return null; + } + + /** Applies one resolved polymorphic-nullness annotation to a method type component. */ + private static Type applyPolyNullAnnotation( + Type type, ImmutableList typePath, Nullness nullness, VisitorState state) { + Type annotationType = + nullness == Nullness.NULLABLE + ? getSyntheticNullableAnnotType(state) + : getSyntheticNonNullAnnotType(state); + return new AddAnnotationToNestedTypeVisitor(typePath, annotationType).apply(type); } /** 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..e4539ba37c 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; @@ -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) { @@ -1205,19 +1221,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 +1339,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 +1399,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 +1431,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 +1522,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 +1553,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 +1640,11 @@ public ImmutableList customStreamNullabilitySpecs() { nestedAnnotationsForMethods() { return nestedAnnotationsForMethods; } + + @Override + public ImmutableSetMultimap polyNullLocations() { + return polyNullLocations; + } } /** @@ -1633,6 +1689,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 +1709,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 +1768,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/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..62ebc10a22 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -0,0 +1,328 @@ +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 with explicit type arguments. + * + *

These tests cover named functional-interface values, lambda and method-reference inference, + * 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 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 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 customLibraryModelRejectsMismatchedExplicitTypeArguments() { + 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 genericMethodRequiresMatchingExplicitTypeArgumentNullability() { + 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) { + PolyNullMethods.twoTypeVariables(nonNull, nonNull); + PolyNullMethods.<@Nullable String, @Nullable String>twoTypeVariables( + nonNull, nonNull); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.twoTypeVariables(nonNull, nonNull); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.<@Nullable String, String>twoTypeVariables(nonNull, nonNull); + } + } + """) + .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..059d5ea920 --- /dev/null +++ b/test-java-lib/src/main/java/com/uber/lib/unannotated/PolyNullMethods.java @@ -0,0 +1,17 @@ +package com.uber.lib.unannotated; + +import java.util.List; + +/* @NullMarked */ +public final class PolyNullMethods { + + private PolyNullMethods() {} + + /** Returns the first element available from either list. */ + public static Object first(List first, List second) { + return first.isEmpty() ? second.get(0) : first.get(0); + } + + /** Accepts two independently typed arguments. */ + public static void twoTypeVariables(T first, U second) {} +} 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..3a471efbea 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,15 @@ 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) + .build(); } @Override @@ -287,6 +295,30 @@ 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())) + .build(); + } + /** Creates the immutable nested-annotation models used by this test provider. */ private static ImmutableMap> createNestedAnnotationsForMethods() { From 8ba649b3b86fb2e389294bb0cfef1bee31d09193 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 08:26:24 -0700 Subject: [PATCH 02/17] simplify --- .../nullaway/generics/GenericsChecks.java | 57 +----- .../handlers/LibraryModelsHandler.java | 11 +- .../AddAnnotationToNestedTypeVisitor.java | 178 ------------------ 3 files changed, 8 insertions(+), 238 deletions(-) delete mode 100644 nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java 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 853b7c3b8b..93df143527 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -63,7 +63,7 @@ import com.uber.nullaway.generics.GenericsUtils.MethodRefTypeRelationKind; import com.uber.nullaway.handlers.Handler; import com.uber.nullaway.libmodel.NestedAnnotationInfo.TypePathEntry; -import com.uber.nullaway.librarymodel.AddAnnotationToNestedTypeVisitor; +import com.uber.nullaway.librarymodel.NestedTypePathUpdater; import java.util.ArrayList; import java.util.HashMap; import java.util.IdentityHashMap; @@ -3508,7 +3508,8 @@ private PolyNullInferenceContext createPolyNullInferenceContext( for (PolyNullInput input : inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { updated = - replaceTypeAtPath(updated, input.location().typePath(), 0, input.inferenceVariable()); + NestedTypePathUpdater.replaceType( + updated, input.location().typePath(), input.inferenceVariable()); } updatedParameterTypes.append(updated); } @@ -3632,56 +3633,6 @@ private void reportPolyNullInferenceFailure( }; } - /** Replaces the nested type at {@code typePath} with {@code replacement}. */ - private static Type replaceTypeAtPath( - Type type, ImmutableList typePath, int pathIndex, Type replacement) { - if (pathIndex == typePath.size()) { - return replacement; - } - TypePathEntry entry = typePath.get(pathIndex); - return switch (entry.kind()) { - case TYPE_ARGUMENT -> { - if (!(type instanceof Type.ClassType classType) - || entry.index() < 0 - || entry.index() >= classType.getTypeArguments().size()) { - yield type; - } - ListBuffer updatedArguments = new ListBuffer<>(); - int argumentIndex = 0; - for (Type argument : classType.getTypeArguments()) { - updatedArguments.add( - argumentIndex++ == entry.index() - ? replaceTypeAtPath(argument, typePath, pathIndex + 1, replacement) - : argument); - } - yield TYPE_METADATA_BUILDER.createClassType( - classType, classType.getEnclosingType(), updatedArguments.toList()); - } - case WILDCARD_BOUND -> { - if (!(type instanceof Type.WildcardType wildcardType)) { - yield type; - } - Type bound = wildcardBound(wildcardType, entry.index()); - if (bound == null) { - yield type; - } - Type updatedBound = replaceTypeAtPath(bound, typePath, pathIndex + 1, replacement); - if (wildcardType.kind == BoundKind.UNBOUND) { - yield TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound( - wildcardType, updatedBound); - } - yield TYPE_METADATA_BUILDER.createWildcardType(wildcardType, updatedBound); - } - case ARRAY_ELEMENT -> - type instanceof Type.ArrayType arrayType - ? TYPE_METADATA_BUILDER.createArrayType( - arrayType, - replaceTypeAtPath( - arrayType.getComponentType(), typePath, pathIndex + 1, replacement)) - : type; - }; - } - /** Returns the requested bound of a wildcard, or {@code null} if that bound is unavailable. */ private static @Nullable Type wildcardBound(Type type, int boundIndex) { if (!(type instanceof Type.WildcardType wildcardType)) { @@ -3707,7 +3658,7 @@ private static Type applyPolyNullAnnotation( nullness == Nullness.NULLABLE ? getSyntheticNullableAnnotType(state) : getSyntheticNonNullAnnotType(state); - return new AddAnnotationToNestedTypeVisitor(typePath, annotationType).apply(type); + return NestedTypePathUpdater.addAnnotation(type, typePath, annotationType); } /** 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 e4539ba37c..7b0a60182e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -64,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; @@ -604,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)); } /** @@ -625,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; } diff --git a/nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java deleted file mode 100644 index ce0c58f881..0000000000 --- a/nullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.java +++ /dev/null @@ -1,178 +0,0 @@ -package com.uber.nullaway.librarymodel; - -import static com.uber.nullaway.generics.TypeMetadataBuilder.TYPE_METADATA_BUILDER; - -import com.google.common.base.Verify; -import com.google.common.collect.ImmutableList; -import com.sun.tools.javac.code.BoundKind; -import com.sun.tools.javac.code.Type; -import com.sun.tools.javac.code.Types; -import com.sun.tools.javac.util.ListBuffer; -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. - */ -@SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks -public final class AddAnnotationToNestedTypeVisitor extends Types.MapVisitor { - private final ImmutableList typePath; - private final Type annotationType; - - /** - * 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) { - this.typePath = typePath; - this.annotationType = annotationType; - } - - /** - * 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) { - return type.accept(this, 0); - } - - @Override - public Type visitClassType(Type.ClassType t, Integer pathIndex) { - if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); - } - NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); - if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.TYPE_ARGUMENT) { - return t; - } - com.sun.tools.javac.util.List typeArgs = t.getTypeArguments(); - int argIndex = entry.index(); - if (argIndex < 0 || argIndex >= typeArgs.size()) { - return t; - } - Type oldTypeArg = typeArgs.get(argIndex); - Type newTypeArg = oldTypeArg.accept(this, pathIndex + 1); - if (newTypeArg == oldTypeArg) { - return t; - } - ListBuffer updatedTypeArgs = new ListBuffer<>(); - int currentIndex = 0; - for (com.sun.tools.javac.util.List l = typeArgs; l.nonEmpty(); l = l.tail) { - updatedTypeArgs.append(currentIndex == argIndex ? newTypeArg : l.head); - currentIndex++; - } - return TYPE_METADATA_BUILDER.createClassType(t, t.getEnclosingType(), updatedTypeArgs.toList()); - } - - @Override - public Type visitArrayType(Type.ArrayType t, Integer pathIndex) { - if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); - } - NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); - if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.ARRAY_ELEMENT) { - return t; - } - Type newElemType = t.elemtype.accept(this, pathIndex + 1); - if (newElemType == t.elemtype) { - return t; - } - return TYPE_METADATA_BUILDER.createArrayType(t, newElemType); - } - - @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 - // 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; - } - NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); - if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.WILDCARD_BOUND) { - return t; - } - int boundIndex = entry.index(); - if (t.kind == BoundKind.UNBOUND) { - if (boundIndex != 0) { - // An unbounded wildcard has an implicit upper bound, but no lower bound. - return t; - } - Type.TypeVar formalTypeVariable = - Verify.verifyNotNull( - t.bound, "unbounded wildcard has no corresponding formal type variable"); - Type upperBound = formalTypeVariable.getUpperBound(); - Type updatedUpperBound = upperBound.accept(this, pathIndex + 1); - return updatedUpperBound == upperBound - ? t - : TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(t, updatedUpperBound); - } - if (boundIndex == 0 && t.kind == BoundKind.EXTENDS) { - Type newBound = t.type.accept(this, pathIndex + 1); - return newBound == t.type ? t : TYPE_METADATA_BUILDER.createWildcardType(t, newBound); - } - if (boundIndex == 1 && t.kind == BoundKind.SUPER) { - Type newBound = t.type.accept(this, pathIndex + 1); - return newBound == t.type ? t : TYPE_METADATA_BUILDER.createWildcardType(t, newBound); - } - 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. - */ - @Override - public Type visitCapturedType(Type.CapturedType t, Integer pathIndex) { - 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 - // bound in that case, on a detached copy so neither path traversal nor annotation updates - // mutate compiler-owned types. - wildcard = - TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(wildcard, t, t.getUpperBound()); - } - Type.WildcardType updatedWildcard; - if (pathIndex < typePath.size()) { - updatedWildcard = (Type.WildcardType) wildcard.accept(this, pathIndex); - } else { - Verify.verify(pathIndex == typePath.size(), "path index out of bounds"); - if (wildcard.kind == BoundKind.UNBOUND) { - Type.TypeVar formalTypeVariable = - Verify.verifyNotNull( - wildcard.bound, "unbounded wildcard has no corresponding formal type variable"); - Type updatedUpperBound = - TypeSubstitutionUtils.typeWithAnnot(formalTypeVariable.getUpperBound(), annotationType); - updatedWildcard = - TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(wildcard, updatedUpperBound); - } else { - Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(wildcard.type, annotationType); - updatedWildcard = TYPE_METADATA_BUILDER.createWildcardType(wildcard, updatedBound); - } - } - if (updatedWildcard == t.wildcard) { - return t; - } - return TypeSubstitutionUtils.replaceCapturedTypeWildcard(t, updatedWildcard); - } - - @Override - public Type visitType(Type t, Integer pathIndex) { - if (pathIndex == typePath.size()) { - return TypeSubstitutionUtils.typeWithAnnot(t, annotationType); - } - return t; - } -} From c009e5394a81a08c7d90720af893ba3d5d138042 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 08:45:30 -0700 Subject: [PATCH 03/17] more --- .../nullaway/generics/GenericsChecks.java | 273 ++++++++++++++---- .../librarymodel/NestedTypePathUpdater.java | 178 ++++++++++++ .../jspecify/PolyNullLibraryModelsTests.java | 80 ++++- .../uber/lib/unannotated/PolyNullMethods.java | 12 + .../testlibrarymodels/TestLibraryModels.java | 42 +++ 5 files changed, 523 insertions(+), 62 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java 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 93df143527..85850e362a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -91,14 +91,24 @@ public record ResolvedMethodReference(Type.MethodType methodType, @Nullable Type /** Marker interface for results of attempting to infer nullability of type variables at a call */ private interface CallInferenceResult {} + /** The result of resolving PolyNull for one invocation in a generic inference session. */ + private record PolyNullInferenceResult(@Nullable Nullness nullness) {} + /** * 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 { @@ -1288,11 +1298,25 @@ 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 = + applyPolyNullToReturnType( + inferredCallType, + handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state), + polyNullResult.nullness(), + state); + } + } + return inferredCallType; } Verify.verify(callTree instanceof NewClassTree); Type constructedTypeAtCallSite = getConstructedTypeAtCallSite((NewClassTree) callTree); @@ -1327,6 +1351,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( @@ -1337,6 +1363,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 @@ -1346,16 +1373,32 @@ private CallInferenceResult runInferenceForCall( typeVarNullability.putIfAbsent(typeVar, ConstraintSolver.InferredNullability.NONNULL); } - InferenceSuccess successResult = new InferenceSuccess(typeVarNullability); + IdentityHashMap polyNullResults = + resolvePolyNullInferenceContexts(polyNullContexts, typeVarNullability, state); + 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) -> { @@ -1488,6 +1531,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 */ @@ -1499,6 +1543,7 @@ private void generateConstraintsForCall( ConstraintSolver solver, ExpressionTree callTree, Set allCalls, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) throws UnsatisfiableConstraintsException { // Register all type variables whose nullability is inferred for this call. @@ -1527,8 +1572,32 @@ private void generateConstraintsForCall( allCalls, argument, formalParamType, + polyNullContexts, calledFromDataflow); }); + if (callTree instanceof MethodInvocationTree invocationTree) { + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(invocationTree); + ImmutableSet locations = + handler.onGetPolyNullLocations(methodSymbol, state); + if (!locations.isEmpty()) { + PolyNullInferenceContext polyNullContext = + polyNullContexts.computeIfAbsent( + invocationTree, + unused -> + createPolyNullInferenceContext(methodSymbol, methodType, locations, state)); + if (!polyNullContext.inputs().isEmpty()) { + generateConstraintsForCallWithMethodType( + state, + path, + solver, + invocationTree, + polyNullContext.inferenceMethodType(), + allCalls, + polyNullContexts, + calledFromDataflow); + } + } + } } /** @@ -1541,6 +1610,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( @@ -1549,6 +1619,7 @@ private void generateConstraintsForPseudoAssignment( Set allCalls, ExpressionTree rhsExpr, Type lhsType, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) { NullabilityUtil.ExprTreeAndState exprTreeAndState = NullabilityUtil.stripParensAndUpdateTreePath(rhsExpr, state); @@ -1559,7 +1630,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 @@ -1571,6 +1650,7 @@ private void generateConstraintsForPseudoAssignment( allCalls, trueExpression, lhsType, + polyNullContexts, calledFromDataflow); ExpressionTree falseExpression = conditionalExpressionTree.getFalseExpression(); TreePath pathToFalseExpression = new TreePath(state.getPath(), falseExpression); @@ -1580,10 +1660,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 @@ -1610,6 +1698,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( @@ -1619,6 +1708,7 @@ private void handleLambdaInGenericMethodInference( Set allCalls, Type lhsType, LambdaExpressionTree lambda, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) { Symbol.MethodSymbol fiMethod = NullabilityUtil.getFunctionalInterfaceMethod(lambda, state.getTypes()); @@ -1643,6 +1733,7 @@ private void handleLambdaInGenericMethodInference( allCalls, returnedExpression, fiReturnType, + polyNullContexts, calledFromDataflow); } else if (body instanceof BlockTree) { // Case 2: Block body, e.g., () -> { return null; } @@ -1658,6 +1749,7 @@ private void handleLambdaInGenericMethodInference( allCalls, returnExpr, fiReturnType, + polyNullContexts, calledFromDataflow); } } @@ -2990,6 +3082,7 @@ public Nullness getGenericReturnNullnessAtInvocation( Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type; Type substitutedReturnType = substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow) + .methodType() .getReturnType(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class @@ -3028,9 +3121,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, @@ -3084,15 +3177,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); } /** @@ -3312,10 +3415,13 @@ 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(); } Type.MethodType modeledMethodType = handler.onOverrideMethodType( @@ -3325,7 +3431,13 @@ private Type.MethodType getInvokedMethodTypeAtCall( tree instanceof MethodInvocationTree invocationTree ? invocationTree : null); return tree instanceof MethodInvocationTree invocationTree ? applyPolyNullModel( - methodSymbol, invocationTree, modeledMethodType, path, state, calledFromDataflow) + methodSymbol, + invocationTree, + modeledMethodType, + jointlyInferredPolyNull, + path, + state, + calledFromDataflow) : modeledMethodType; } @@ -3344,6 +3456,7 @@ private Type.MethodType applyPolyNullModel( Symbol.MethodSymbol methodSymbol, MethodInvocationTree invocationTree, Type.MethodType substitutedMethodType, + @Nullable PolyNullInferenceResult jointlyInferredPolyNull, @Nullable TreePath path, VisitorState state, boolean calledFromDataflow) { @@ -3352,14 +3465,16 @@ private Type.MethodType applyPolyNullModel( return substitutedMethodType; } Nullness polyNullness = - inferPolyNullness( - methodSymbol, - invocationTree, - substitutedMethodType, - locations, - path, - state, - calledFromDataflow); + jointlyInferredPolyNull != null + ? jointlyInferredPolyNull.nullness() + : inferPolyNullness( + methodSymbol, + invocationTree, + substitutedMethodType, + locations, + path, + state, + calledFromDataflow); if (polyNullness == null) { return substitutedMethodType; } @@ -3382,13 +3497,7 @@ private Type.MethodType applyPolyNullModel( changed |= updatedParameterType != parameterType; } Type returnType = substitutedMethodType.restype; - Type updatedReturnType = returnType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == -1) { - updatedReturnType = - applyPolyNullAnnotation(updatedReturnType, location.typePath(), polyNullness, state); - } - } + Type updatedReturnType = applyPolyNullToReturnType(returnType, locations, polyNullness, state); changed |= updatedReturnType != returnType; return changed ? new Type.MethodType( @@ -3399,9 +3508,25 @@ private Type.MethodType applyPolyNullModel( : substitutedMethodType; } + /** Applies a resolved PolyNull value to all modeled locations within a return type. */ + private static Type applyPolyNullToReturnType( + Type returnType, + ImmutableSet locations, + Nullness polyNullness, + VisitorState state) { + Type updatedReturnType = returnType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + updatedReturnType = + applyPolyNullAnnotation(updatedReturnType, location.typePath(), polyNullness, state); + } + } + return updatedReturnType; + } + /** One modeled input occurrence and the synthetic inference variable that represents it. */ private record PolyNullInput( - PolyNullLocation location, Type.TypeVar inferenceVariable, Type substitutedLocationType) {} + PolyNullLocation location, Type.TypeVar inferenceVariable, Type modeledLocationType) {} /** * The method type and occurrence variables used to generate PolyNull constraints for one call. @@ -3446,24 +3571,11 @@ private record PolyNullInferenceContext( invocationTree, inferenceContext.inferenceMethodType(), nestedCalls, + new IdentityHashMap<>(), calledFromDataflow); Map solution = solver.solve(); - Nullness resolved = null; - for (PolyNullInput input : inferenceContext.inputs()) { - ConstraintSolver.InferredNullability inferred = - solution.getOrDefault( - input.inferenceVariable().asElement(), - ConstraintSolver.InferredNullability.NONNULL); - Nullness current = - inferred == ConstraintSolver.InferredNullability.NULLABLE - ? Nullness.NULLABLE - : Nullness.NONNULL; - if (resolved != null && resolved != current) { - reportPolyNullInferenceFailure(invocationTree, state); - return null; - } - resolved = current; - } + Nullness resolved = + resolvePolyNullInferenceContext(invocationTree, inferenceContext, solution, state); if (resolved != null && !calledFromDataflow) { polyNullResolutions.put(invocationTree, resolved); } @@ -3474,10 +3586,51 @@ private record PolyNullInferenceContext( } } + /** Resolves all PolyNull contexts after a shared generic-inference solver run. */ + private IdentityHashMap + resolvePolyNullInferenceContexts( + IdentityHashMap contexts, + Map solution, + VisitorState state) { + IdentityHashMap results = + new IdentityHashMap<>(); + for (Map.Entry entry : contexts.entrySet()) { + results.put( + entry.getKey(), + new PolyNullInferenceResult( + resolvePolyNullInferenceContext(entry.getKey(), entry.getValue(), solution, state))); + } + return results; + } + + /** Resolves one PolyNull context and reports an error if its occurrences disagree. */ + private @Nullable Nullness resolvePolyNullInferenceContext( + MethodInvocationTree invocationTree, + PolyNullInferenceContext inferenceContext, + Map solution, + VisitorState state) { + Nullness resolved = null; + for (PolyNullInput input : inferenceContext.inputs()) { + ConstraintSolver.InferredNullability inferred = + solution.getOrDefault( + input.inferenceVariable().asElement(), ConstraintSolver.InferredNullability.NONNULL); + Nullness current = + inferred == ConstraintSolver.InferredNullability.NULLABLE + ? Nullness.NULLABLE + : Nullness.NONNULL; + if (resolved != null && resolved != current) { + reportPolyNullInferenceFailure(invocationTree, state); + return null; + } + resolved = current; + } + return resolved; + } + /** Creates independent inference variables at all modeled PolyNull input locations. */ private PolyNullInferenceContext createPolyNullInferenceContext( Symbol.MethodSymbol methodSymbol, - Type.MethodType substitutedMethodType, + Type.MethodType methodType, ImmutableSet locations, VisitorState state) { Map> inputsByParameter = new LinkedHashMap<>(); @@ -3485,10 +3638,10 @@ private PolyNullInferenceContext createPolyNullInferenceContext( int occurrence = 0; for (PolyNullLocation location : locations) { int parameterIndex = location.parameterIndex(); - if (parameterIndex < 0 || parameterIndex >= substitutedMethodType.argtypes.size()) { + if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { continue; } - Type parameterType = substitutedMethodType.argtypes.get(parameterIndex); + Type parameterType = methodType.argtypes.get(parameterIndex); Type locationType = typeAtPath(parameterType, location.typePath(), 0); if (locationType == null) { continue; @@ -3501,7 +3654,7 @@ private PolyNullInferenceContext createPolyNullInferenceContext( } ListBuffer updatedParameterTypes = new ListBuffer<>(); int parameterIndex = 0; - for (com.sun.tools.javac.util.List remaining = substitutedMethodType.argtypes; + for (com.sun.tools.javac.util.List remaining = methodType.argtypes; remaining.nonEmpty(); remaining = remaining.tail, parameterIndex++) { Type updated = remaining.head; @@ -3515,10 +3668,7 @@ private PolyNullInferenceContext createPolyNullInferenceContext( } return new PolyNullInferenceContext( new Type.MethodType( - updatedParameterTypes.toList(), - substitutedMethodType.restype, - substitutedMethodType.thrown, - substitutedMethodType.tsym), + updatedParameterTypes.toList(), methodType.restype, methodType.thrown, methodType.tsym), allInputs.build()); } @@ -3562,7 +3712,7 @@ private void seedPolyNullFromExplicitMethodTypeArguments( if (declaredLocationType instanceof Type.TypeVar typeVariable && methodTypeVariables.contains(typeVariable.tsym)) { solver.addNullabilityEqualityConstraint( - input.substitutedLocationType(), input.inferenceVariable()); + input.modeledLocationType(), input.inferenceVariable()); } } } @@ -3575,6 +3725,7 @@ private void generateConstraintsForCallWithMethodType( MethodInvocationTree invocationTree, Type.MethodType methodType, Set allCalls, + IdentityHashMap polyNullContexts, boolean calledFromDataflow) { TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), invocationTree); new InvocationArguments(invocationTree, methodType) @@ -3587,6 +3738,7 @@ private void generateConstraintsForCallWithMethodType( allCalls, argument, formalParamType, + polyNullContexts, calledFromDataflow); }); } @@ -3705,6 +3857,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 diff --git a/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java new file mode 100644 index 0000000000..44bf7ab861 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java @@ -0,0 +1,178 @@ +package com.uber.nullaway.librarymodel; + +import static com.uber.nullaway.generics.TypeMetadataBuilder.TYPE_METADATA_BUILDER; + +import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; +import com.sun.tools.javac.code.BoundKind; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.ListBuffer; +import com.uber.nullaway.generics.TypeSubstitutionUtils; +import com.uber.nullaway.libmodel.NestedAnnotationInfo; + +/** Updates a type at a nested location identified by a library-model type path. */ +@SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks +public final class NestedTypePathUpdater extends Types.MapVisitor { + + private enum UpdateKind { + ADD_ANNOTATION, + REPLACE_TYPE + } + + private final ImmutableList typePath; + private final Type updateType; + private final UpdateKind updateKind; + + private NestedTypePathUpdater( + ImmutableList typePath, + Type updateType, + UpdateKind updateKind) { + this.typePath = typePath; + this.updateType = updateType; + this.updateKind = updateKind; + } + + /** 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 updateLeaf(t); + } + NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); + if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.TYPE_ARGUMENT) { + return t; + } + com.sun.tools.javac.util.List typeArgs = t.getTypeArguments(); + int argIndex = entry.index(); + if (argIndex < 0 || argIndex >= typeArgs.size()) { + return t; + } + Type oldTypeArg = typeArgs.get(argIndex); + Type newTypeArg = oldTypeArg.accept(this, pathIndex + 1); + if (newTypeArg == oldTypeArg) { + return t; + } + ListBuffer updatedTypeArgs = new ListBuffer<>(); + int currentIndex = 0; + for (com.sun.tools.javac.util.List l = typeArgs; l.nonEmpty(); l = l.tail) { + updatedTypeArgs.append(currentIndex == argIndex ? newTypeArg : l.head); + currentIndex++; + } + return TYPE_METADATA_BUILDER.createClassType(t, t.getEnclosingType(), updatedTypeArgs.toList()); + } + + @Override + public Type visitArrayType(Type.ArrayType t, Integer pathIndex) { + if (pathIndex == typePath.size()) { + return updateLeaf(t); + } + NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); + if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.ARRAY_ELEMENT) { + return t; + } + Type newElemType = t.elemtype.accept(this, pathIndex + 1); + if (newElemType == t.elemtype) { + return t; + } + return TYPE_METADATA_BUILDER.createArrayType(t, newElemType); + } + + @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 + // 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 updateKind == UpdateKind.ADD_ANNOTATION ? t : updateLeaf(t); + } + NestedAnnotationInfo.TypePathEntry entry = typePath.get(pathIndex); + if (entry.kind() != NestedAnnotationInfo.TypePathEntry.Kind.WILDCARD_BOUND) { + return t; + } + int boundIndex = entry.index(); + if (t.kind == BoundKind.UNBOUND) { + if (boundIndex != 0) { + // An unbounded wildcard has an implicit upper bound, but no lower bound. + return t; + } + Type.TypeVar formalTypeVariable = + Verify.verifyNotNull( + t.bound, "unbounded wildcard has no corresponding formal type variable"); + Type upperBound = formalTypeVariable.getUpperBound(); + Type updatedUpperBound = upperBound.accept(this, pathIndex + 1); + return updatedUpperBound == upperBound + ? t + : TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(t, updatedUpperBound); + } + if (boundIndex == 0 && t.kind == BoundKind.EXTENDS) { + Type newBound = t.type.accept(this, pathIndex + 1); + return newBound == t.type ? t : TYPE_METADATA_BUILDER.createWildcardType(t, newBound); + } + if (boundIndex == 1 && t.kind == BoundKind.SUPER) { + Type newBound = t.type.accept(this, pathIndex + 1); + return newBound == t.type ? t : TYPE_METADATA_BUILDER.createWildcardType(t, newBound); + } + return t; + } + + /** 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 updatedWildcard; + if (pathIndex < typePath.size()) { + updatedWildcard = (Type.WildcardType) t.wildcard.accept(this, pathIndex); + } else { + Verify.verify(pathIndex == typePath.size(), "path index out of bounds"); + if (t.wildcard.kind == BoundKind.UNBOUND) { + Type.TypeVar formalTypeVariable = + Verify.verifyNotNull( + t.wildcard.bound, "unbounded wildcard has no corresponding formal type variable"); + Type updatedUpperBound = + TypeSubstitutionUtils.typeWithAnnot(formalTypeVariable.getUpperBound(), updateType); + updatedWildcard = + TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(t.wildcard, updatedUpperBound); + } else { + Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(t.wildcard.type, updateType); + updatedWildcard = TYPE_METADATA_BUILDER.createWildcardType(t.wildcard, updatedBound); + } + } + if (updatedWildcard == t.wildcard) { + return t; + } + return TypeSubstitutionUtils.replaceCapturedTypeWildcard(t, updatedWildcard); + } + + @Override + public Type visitType(Type t, Integer pathIndex) { + if (pathIndex == typePath.size()) { + return updateLeaf(t); + } + return t; + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java index 62ebc10a22..c61cb13e01 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -7,10 +7,10 @@ import org.junit.Test; /** - * Specification tests for {@code @PolyNull}-like library models with explicit type arguments. + * Specification tests for {@code @PolyNull}-like library models. * *

These tests cover named functional-interface values, lambda and method-reference inference, - * explicit method type arguments, and propagation through {@code var} locals. + * inferred and explicit method type arguments, and propagation through {@code var} locals. * * @see NullAway issue #1616 */ @@ -321,6 +321,82 @@ void test(String 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(); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.genericFirst(nonNull, nullable); + + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.genericFirst(nullable, nonNull); + } + } + """) + .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: polymorphic nullness constrained to both @NonNull and @Nullable + PolyNullMethods.genericFromSuppliers(() -> "first", () -> null); + } + } + """) + .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 index 059d5ea920..13e38b5fc4 100644 --- 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 @@ -1,6 +1,7 @@ package com.uber.lib.unannotated; import java.util.List; +import java.util.function.Supplier; /* @NullMarked */ public final class PolyNullMethods { @@ -14,4 +15,15 @@ public static Object first(List first, List second) { /** Accepts two independently typed arguments. */ public static void twoTypeVariables(T first, U second) {} + + /** Returns the first of two independently typed arguments. */ + public static T genericFirst(T first, U second) { + return first; + } + + /** Returns a value from the first of two independently typed suppliers. */ + public static 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 3a471efbea..d44a5aea52 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 @@ -286,6 +286,18 @@ public ImmutableSetMultimap methodTypeVariablesWithNullableU 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", + "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(); } @@ -316,6 +328,36 @@ private static ImmutableSetMultimap createPolyNullL .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", + "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(); } From a810f46105c05cdffd1de89db5aa86b4b647c59f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 09:26:55 -0700 Subject: [PATCH 04/17] changes --- .../nullaway/generics/GenericsChecks.java | 138 +++++++++++++++--- .../jspecify/PolyNullLibraryModelsTests.java | 79 ++++++++++ .../uber/lib/unannotated/PolyNullMethods.java | 5 + .../testlibrarymodels/TestLibraryModels.java | 11 ++ 4 files changed, 211 insertions(+), 22 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 85850e362a..11746dc614 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1552,6 +1552,20 @@ private void generateConstraintsForCall( } 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 -> + createPolyNullInferenceContext(methodSymbol, methodType, locations, state)); + } + } // first, handle the call result flow if (typeFromAssignmentContext != null) { Type callResultType = @@ -1559,6 +1573,15 @@ private void generateConstraintsForCall( ? methodType.getReturnType() : getConstructedTypeAtCallSite((NewClassTree) callTree).tsym.type; solver.addSubtypeConstraint(callResultType, typeFromAssignmentContext, assignedToLocal); + if (polyNullContext != null) { + addPolyNullResultConstraints( + solver, + methodType.getReturnType(), + polyNullLocations, + polyNullContext, + typeFromAssignmentContext, + assignedToLocal); + } } // then, handle parameters TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), callTree); @@ -1575,28 +1598,18 @@ private void generateConstraintsForCall( polyNullContexts, calledFromDataflow); }); - if (callTree instanceof MethodInvocationTree invocationTree) { - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(invocationTree); - ImmutableSet locations = - handler.onGetPolyNullLocations(methodSymbol, state); - if (!locations.isEmpty()) { - PolyNullInferenceContext polyNullContext = - polyNullContexts.computeIfAbsent( - invocationTree, - unused -> - createPolyNullInferenceContext(methodSymbol, methodType, locations, state)); - if (!polyNullContext.inputs().isEmpty()) { - generateConstraintsForCallWithMethodType( - state, - path, - solver, - invocationTree, - polyNullContext.inferenceMethodType(), - allCalls, - polyNullContexts, - calledFromDataflow); - } - } + if (callTree instanceof MethodInvocationTree invocationTree + && polyNullContext != null + && !polyNullContext.inputs().isEmpty()) { + generateConstraintsForCallWithMethodType( + state, + path, + solver, + invocationTree, + polyNullContext.inferenceMethodType(), + allCalls, + polyNullContexts, + calledFromDataflow); } } @@ -3564,6 +3577,15 @@ private record PolyNullInferenceContext( try { seedPolyNullFromExplicitMethodTypeArguments( methodSymbol, invocationTree, inferenceContext.inputs(), solver); + addPolyNullResultConstraintsFromDirectAssignmentContext( + invocationTree, + substitutedMethodType, + locations, + inferenceContext, + path, + solver, + state, + calledFromDataflow); generateConstraintsForCallWithMethodType( state, path, @@ -3672,6 +3694,78 @@ private PolyNullInferenceContext createPolyNullInferenceContext( allInputs.build()); } + /** + * 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, + Type.MethodType methodType, + ImmutableSet locations, + 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) { + addPolyNullResultConstraints( + solver, + methodType.getReturnType(), + locations, + inferenceContext, + callAndContext.typeFromAssignmentContext(), + callAndContext.assignedToLocal()); + } + } + + /** Adds call-result subtype constraints for every independently inferred PolyNull occurrence. */ + private static void addPolyNullResultConstraints( + ConstraintSolver solver, + Type returnType, + ImmutableSet locations, + PolyNullInferenceContext inferenceContext, + Type targetType, + boolean assignedToLocal) { + if (locations.stream().noneMatch(location -> location.parameterIndex() == -1)) { + return; + } + for (PolyNullInput input : inferenceContext.inputs()) { + Type inferenceReturnType = returnType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + inferenceReturnType = + NestedTypePathUpdater.replaceType( + inferenceReturnType, location.typePath(), input.inferenceVariable()); + } + } + solver.addSubtypeConstraint(inferenceReturnType, targetType, assignedToLocal); + } + } + /** Creates a nullable-bounded synthetic type variable for one PolyNull input occurrence. */ private static Type.TypeVar createPolyNullInferenceVariable( Symbol.MethodSymbol methodSymbol, int occurrence, VisitorState state) { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java index c61cb13e01..5f00a1d581 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -72,6 +72,35 @@ void test( .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() @@ -205,6 +234,31 @@ void test(Map map) { .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() @@ -357,6 +411,31 @@ void test(String nonNull, @Nullable String nullable) { .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: type variable $PolyNull + field = PolyNullMethods.genericObject(nullable, nullable); + } + } + """) + .doTest(); + } + @Test public void genericMethodInfersPolyNullFromLambdasAndMethodReferences() { makeHelper() 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 index 13e38b5fc4..9fbfc22883 100644 --- 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 @@ -21,6 +21,11 @@ public static T genericFirst(T first, U second) { return first; } + /** Returns the first argument as an object. */ + public static Object genericObject(T first, U second) { + return first; + } + /** Returns a value from the first of two independently typed suppliers. */ public static T genericFromSuppliers( Supplier first, Supplier second) { 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 d44a5aea52..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 @@ -288,6 +288,8 @@ public ImmutableSetMultimap methodTypeVariablesWithNullableU .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", @@ -337,6 +339,15 @@ private static ImmutableSetMultimap createPolyNullL .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", From 596f80d21b58d4eec487808cc1da3474eb5ed01e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 10:03:12 -0700 Subject: [PATCH 05/17] more --- .../nullaway/generics/ConstraintSolver.java | 9 -- .../generics/ConstraintSolverImpl.java | 8 - .../nullaway/generics/GenericsChecks.java | 145 ++++++------------ .../jspecify/PolyNullLibraryModelsTests.java | 29 ++-- 4 files changed, 60 insertions(+), 131 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java index 325a9c4f98..5b74181b70 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolver.java @@ -64,15 +64,6 @@ public boolean isCausedByNonNullUpperBound() { void addSubtypeConstraint(Type subtype, Type supertype, boolean localVariableType) throws UnsatisfiableConstraintsException; - /** - * Constrains {@code first} and {@code second} to have identical nullability. - * - *

This is stronger than Java subtyping and is used when an explicitly written type argument - * fixes the value of a qualifier-inference variable. - */ - void addNullabilityEqualityConstraint(Type first, Type second) - throws UnsatisfiableConstraintsException; - enum InferredNullability { NONNULL, NULLABLE diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java index 8fe393770f..f1f9d3b51d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java @@ -97,14 +97,6 @@ public void addSubtypeConstraint(Type subtype, Type supertype, boolean localVari subtype.accept(new AddSubtypeConstraintsVisitor(localVariableType), supertype); } - @Override - public void addNullabilityEqualityConstraint(Type first, Type second) - throws UnsatisfiableConstraintsException { - AddSubtypeConstraintsVisitor visitor = new AddSubtypeConstraintsVisitor(false); - first.accept(visitor, second); - second.accept(visitor, first); - } - class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Void, Type> { private boolean localVariableType; 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 11746dc614..4065f65e84 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1374,7 +1374,7 @@ private CallInferenceResult runInferenceForCall( } IdentityHashMap polyNullResults = - resolvePolyNullInferenceContexts(polyNullContexts, typeVarNullability, state); + resolvePolyNullInferenceContexts(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 @@ -3462,7 +3462,7 @@ public boolean hasPolyNullModel(Symbol.MethodSymbol methodSymbol, VisitorState s /** * 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 - * input occurrences. + * inputs and any available result target. */ @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) private Type.MethodType applyPolyNullModel( @@ -3537,23 +3537,18 @@ private static Type applyPolyNullToReturnType( return updatedReturnType; } - /** One modeled input occurrence and the synthetic inference variable that represents it. */ - private record PolyNullInput( - PolyNullLocation location, Type.TypeVar inferenceVariable, Type modeledLocationType) {} - - /** - * The method type and occurrence variables used to generate PolyNull constraints for one call. - */ + /** The modeled input overlay and shared PolyNull variable for one call. */ private record PolyNullInferenceContext( - Type.MethodType inferenceMethodType, ImmutableList inputs) {} + Type.MethodType inferenceMethodType, + ImmutableList inputs, + Type.TypeVar inferenceVariable) {} /** * Infers the nullness shared by all PolyNull occurrences using the generic constraint solver. * - *

Each modeled input occurrence receives a separate synthetic type variable while constraints - * are generated. This lets a lambda with several return expressions infer one joined nullness for - * its occurrence. After solving, every occurrence must have the same solution, implementing the - * linked-overload semantics of PolyNull. + *

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, @@ -3575,8 +3570,6 @@ private record PolyNullInferenceContext( ConstraintSolver solver = makeSolver(state, analysis); Set nestedCalls = new LinkedHashSet<>(); try { - seedPolyNullFromExplicitMethodTypeArguments( - methodSymbol, invocationTree, inferenceContext.inputs(), solver); addPolyNullResultConstraintsFromDirectAssignmentContext( invocationTree, substitutedMethodType, @@ -3596,8 +3589,7 @@ private record PolyNullInferenceContext( new IdentityHashMap<>(), calledFromDataflow); Map solution = solver.solve(); - Nullness resolved = - resolvePolyNullInferenceContext(invocationTree, inferenceContext, solution, state); + Nullness resolved = resolvePolyNullInferenceContext(inferenceContext, solution); if (resolved != null && !calledFromDataflow) { polyNullResolutions.put(invocationTree, resolved); } @@ -3612,52 +3604,39 @@ private record PolyNullInferenceContext( private IdentityHashMap resolvePolyNullInferenceContexts( IdentityHashMap contexts, - Map solution, - VisitorState state) { + Map solution) { IdentityHashMap results = new IdentityHashMap<>(); for (Map.Entry entry : contexts.entrySet()) { results.put( entry.getKey(), - new PolyNullInferenceResult( - resolvePolyNullInferenceContext(entry.getKey(), entry.getValue(), solution, state))); + new PolyNullInferenceResult(resolvePolyNullInferenceContext(entry.getValue(), solution))); } return results; } - /** Resolves one PolyNull context and reports an error if its occurrences disagree. */ - private @Nullable Nullness resolvePolyNullInferenceContext( - MethodInvocationTree invocationTree, + /** Resolves the shared PolyNull variable for one invocation. */ + private static Nullness resolvePolyNullInferenceContext( PolyNullInferenceContext inferenceContext, - Map solution, - VisitorState state) { - Nullness resolved = null; - for (PolyNullInput input : inferenceContext.inputs()) { - ConstraintSolver.InferredNullability inferred = - solution.getOrDefault( - input.inferenceVariable().asElement(), ConstraintSolver.InferredNullability.NONNULL); - Nullness current = - inferred == ConstraintSolver.InferredNullability.NULLABLE - ? Nullness.NULLABLE - : Nullness.NONNULL; - if (resolved != null && resolved != current) { - reportPolyNullInferenceFailure(invocationTree, state); - return null; - } - resolved = current; - } - return resolved; + Map solution) { + ConstraintSolver.InferredNullability inferred = + solution.getOrDefault( + inferenceContext.inferenceVariable().asElement(), + ConstraintSolver.InferredNullability.NONNULL); + return inferred == ConstraintSolver.InferredNullability.NULLABLE + ? Nullness.NULLABLE + : Nullness.NONNULL; } - /** Creates independent inference variables at all modeled PolyNull input locations. */ + /** Creates a method-type overlay with one shared variable at every modeled PolyNull input. */ private PolyNullInferenceContext createPolyNullInferenceContext( Symbol.MethodSymbol methodSymbol, Type.MethodType methodType, ImmutableSet locations, VisitorState state) { - Map> inputsByParameter = new LinkedHashMap<>(); - ImmutableList.Builder allInputs = ImmutableList.builder(); - int occurrence = 0; + Map> inputsByParameter = new LinkedHashMap<>(); + ImmutableList.Builder allInputs = ImmutableList.builder(); + Type.TypeVar inferenceVariable = createPolyNullInferenceVariable(methodSymbol, 0, state); for (PolyNullLocation location : locations) { int parameterIndex = location.parameterIndex(); if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { @@ -3668,11 +3647,8 @@ private PolyNullInferenceContext createPolyNullInferenceContext( if (locationType == null) { continue; } - Type.TypeVar inferenceVariable = - createPolyNullInferenceVariable(methodSymbol, occurrence++, state); - PolyNullInput input = new PolyNullInput(location, inferenceVariable, locationType); - inputsByParameter.computeIfAbsent(parameterIndex, unused -> new ArrayList<>()).add(input); - allInputs.add(input); + inputsByParameter.computeIfAbsent(parameterIndex, unused -> new ArrayList<>()).add(location); + allInputs.add(location); } ListBuffer updatedParameterTypes = new ListBuffer<>(); int parameterIndex = 0; @@ -3680,18 +3656,18 @@ private PolyNullInferenceContext createPolyNullInferenceContext( remaining.nonEmpty(); remaining = remaining.tail, parameterIndex++) { Type updated = remaining.head; - for (PolyNullInput input : + for (PolyNullLocation location : inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { updated = - NestedTypePathUpdater.replaceType( - updated, input.location().typePath(), input.inferenceVariable()); + NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); } updatedParameterTypes.append(updated); } return new PolyNullInferenceContext( new Type.MethodType( updatedParameterTypes.toList(), methodType.restype, methodType.thrown, methodType.tsym), - allInputs.build()); + allInputs.build(), + inferenceVariable); } /** @@ -3742,7 +3718,7 @@ private void addPolyNullResultConstraintsFromDirectAssignmentContext( } } - /** Adds call-result subtype constraints for every independently inferred PolyNull occurrence. */ + /** Adds a call-result subtype constraint using the invocation's shared PolyNull variable. */ private static void addPolyNullResultConstraints( ConstraintSolver solver, Type returnType, @@ -3753,25 +3729,23 @@ private static void addPolyNullResultConstraints( if (locations.stream().noneMatch(location -> location.parameterIndex() == -1)) { return; } - for (PolyNullInput input : inferenceContext.inputs()) { - Type inferenceReturnType = returnType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == -1) { - inferenceReturnType = - NestedTypePathUpdater.replaceType( - inferenceReturnType, location.typePath(), input.inferenceVariable()); - } + Type inferenceReturnType = returnType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + inferenceReturnType = + NestedTypePathUpdater.replaceType( + inferenceReturnType, location.typePath(), inferenceContext.inferenceVariable()); } - solver.addSubtypeConstraint(inferenceReturnType, targetType, assignedToLocal); } + solver.addSubtypeConstraint(inferenceReturnType, targetType, assignedToLocal); } - /** Creates a nullable-bounded synthetic type variable for one PolyNull input occurrence. */ + /** Creates the nullable-bounded synthetic type variable for one PolyNull invocation. */ private static Type.TypeVar createPolyNullInferenceVariable( - Symbol.MethodSymbol methodSymbol, int occurrence, VisitorState state) { + Symbol.MethodSymbol methodSymbol, int group, VisitorState state) { Symbol.TypeVariableSymbol symbol = new Symbol.TypeVariableSymbol( - 0, state.getName("$PolyNull$" + occurrence), Type.noType, methodSymbol); + 0, state.getName("$PolyNull$" + group), Type.noType, methodSymbol); Type nullableObject = TypeSubstitutionUtils.typeWithAnnot( state.getSymtab().objectType, getSyntheticNullableAnnotType(state)); @@ -3780,37 +3754,6 @@ private static Type.TypeVar createPolyNullInferenceVariable( return variable; } - /** - * Seeds occurrence variables when a modeled location denotes an explicitly instantiated method - * type variable. - */ - private void seedPolyNullFromExplicitMethodTypeArguments( - Symbol.MethodSymbol methodSymbol, - MethodInvocationTree invocationTree, - ImmutableList inputs, - ConstraintSolver solver) { - if (invocationTree.getTypeArguments().isEmpty()) { - return; - } - Type.MethodType declaredMethodType = methodSymbol.type.asMethodType(); - ImmutableSet methodTypeVariables = - ImmutableSet.copyOf(methodSymbol.getTypeParameters()); - for (PolyNullInput input : inputs) { - int parameterIndex = input.location().parameterIndex(); - if (parameterIndex >= declaredMethodType.argtypes.size()) { - continue; - } - Type declaredLocationType = - typeAtPath( - declaredMethodType.argtypes.get(parameterIndex), input.location().typePath(), 0); - if (declaredLocationType instanceof Type.TypeVar typeVariable - && methodTypeVariables.contains(typeVariable.tsym)) { - solver.addNullabilityEqualityConstraint( - input.modeledLocationType(), input.inferenceVariable()); - } - } - } - /** Generates ordinary call constraints using a method type containing PolyNull variables. */ private void generateConstraintsForCallWithMethodType( VisitorState state, @@ -3837,7 +3780,7 @@ private void generateConstraintsForCallWithMethodType( }); } - /** Reports a contradiction between independently inferred PolyNull input occurrences. */ + /** Reports unsatisfiable constraints on a PolyNull invocation. */ private void reportPolyNullInferenceFailure( MethodInvocationTree invocationTree, VisitorState state) { if (!callsWithReportedInferenceFailures.add(invocationTree)) { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java index 5f00a1d581..8ae8423512 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -326,7 +326,7 @@ void test( } @Test - public void customLibraryModelRejectsMismatchedExplicitTypeArguments() { + public void customLibraryModelRejectsIncompatibleInvariantArguments() { makeHelper() .addSourceLines( "Test.java", @@ -348,7 +348,7 @@ void test(List nonNull, List<@Nullable Object> nullable) { } @Test - public void genericMethodRequiresMatchingExplicitTypeArgumentNullability() { + public void genericMethodAcceptsCompatibleExplicitTypeArguments() { makeHelper() .addSourceLines( "Test.java", @@ -359,16 +359,14 @@ public void genericMethodRequiresMatchingExplicitTypeArgumentNullability() { @NullMarked class Test { - void test(String nonNull) { + void test(String nonNull, @Nullable String nullable) { PolyNullMethods.twoTypeVariables(nonNull, nonNull); PolyNullMethods.<@Nullable String, @Nullable String>twoTypeVariables( nonNull, nonNull); - - // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable PolyNullMethods.twoTypeVariables(nonNull, nonNull); - - // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable PolyNullMethods.<@Nullable String, String>twoTypeVariables(nonNull, nonNull); + PolyNullMethods.twoTypeVariables(nonNull, nullable); + PolyNullMethods.<@Nullable String, String>twoTypeVariables(nullable, nonNull); } } """) @@ -400,11 +398,13 @@ void test(String nonNull, @Nullable String nullable) { // BUG: Diagnostic contains: dereferenced expression 'nullableResult' is @Nullable nullableResult.hashCode(); - // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable - PolyNullMethods.genericFirst(nonNull, nullable); + PolyNullMethods.twoTypeVariables(nonNull, nullable); - // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable - PolyNullMethods.genericFirst(nullable, nonNull); + // 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(); } } """) @@ -468,8 +468,11 @@ void test() { // BUG: Diagnostic contains: dereferenced expression 'PolyNullMethods.genericFromSuppliers(Test::nullableValue, Test::nullableValue)' is @Nullable PolyNullMethods.genericFromSuppliers(Test::nullableValue, Test::nullableValue).hashCode(); - // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable - PolyNullMethods.genericFromSuppliers(() -> "first", () -> null); + // 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(); } } """) From 8282719c7d8d4cdf5b8ea6d5e0ac715ac0ff061f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 10:25:33 -0700 Subject: [PATCH 06/17] improve --- .../com/uber/lib/unannotated/PolyNullMethods.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) 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 index 9fbfc22883..565404e75b 100644 --- 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 @@ -9,26 +9,29 @@ public final class PolyNullMethods { private PolyNullMethods() {} /** Returns the first element available from either list. */ - public static Object first(List first, List second) { + 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(T first, U second) {} + public static void twoTypeVariables(/* @PolyNull */ T first, /* @PolyNull */ U second) {} /** Returns the first of two independently typed arguments. */ - public static T genericFirst(T first, U second) { + public static /* @PolyNull */ T genericFirst( + /* @PolyNull */ T first, /* @PolyNull */ U second) { return first; } /** Returns the first argument as an object. */ - public static Object genericObject(T first, U second) { + 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 T genericFromSuppliers( - Supplier first, Supplier second) { + public static /* @PolyNull */ T genericFromSuppliers( + Supplier first, Supplier second) { return first.get(); } } From 90e61719d1c60530d0242f693da2ce86417f6533 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 10:33:41 -0700 Subject: [PATCH 07/17] better error message --- .../nullaway/generics/GenericsChecks.java | 23 ++++++++++++++----- .../jspecify/PolyNullLibraryModelsTests.java | 2 +- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 4065f65e84..34c0e52bf1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1422,7 +1422,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(); @@ -1445,15 +1445,26 @@ 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(); + String typeVariableDescription = + polyNullContexts.values().stream() + .anyMatch( + context -> + Objects.equals(context.inferenceVariable().asElement(), typeVariable)) + ? "type variable for @PolyNull locations" + : "type variable " + typeVariable; 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()); + "inference failure: %s is constrained to be @Nullable, but its upper bound requires it to be @NonNull", + typeVariableDescription); } return String.format( - "inference failure: type variable %s constrained to be both @NonNull and @Nullable", - e.getTypeVariable()); + "inference failure: %s constrained to be both @NonNull and @Nullable", + typeVariableDescription); } /** Returns the type parameters whose nullability is inferred for {@code callTree}. */ diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java index 8ae8423512..d892104755 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -428,7 +428,7 @@ class Test { void test(Object nonNull, @Nullable Object nullable) { field = PolyNullMethods.genericObject(nonNull, nonNull); - // BUG: Diagnostic contains: type variable $PolyNull + // BUG: Diagnostic contains: type variable for @PolyNull locations field = PolyNullMethods.genericObject(nullable, nullable); } } From afe58e01cd8f2aed0a1b47726231305d22d6b4cb Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 10:39:28 -0700 Subject: [PATCH 08/17] improve --- .../nullaway/generics/GenericsChecks.java | 26 ++++++++++--------- .../jspecify/PolyNullLibraryModelsTests.java | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 34c0e52bf1..1dde9459b1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -88,6 +88,10 @@ public final class GenericsChecks { /** Types resolved for a method reference using its ground target type. */ public record ResolvedMethodReference(Type.MethodType methodType, @Nullable Type qualifierType) {} + /** Diagnostic for incompatible constraints on modeled PolyNull locations. */ + private static final String POLY_NULL_INFERENCE_FAILURE_MESSAGE = + "inference failure: polymorphic nullness constrained to both @NonNull and @Nullable"; + /** Marker interface for results of attempting to infer nullability of type variables at a call */ private interface CallInferenceResult {} @@ -1450,21 +1454,19 @@ private String inferenceFailureMessage( UnsatisfiableConstraintsException e, IdentityHashMap polyNullContexts) { Element typeVariable = e.getTypeVariable(); - String typeVariableDescription = - polyNullContexts.values().stream() - .anyMatch( - context -> - Objects.equals(context.inferenceVariable().asElement(), typeVariable)) - ? "type variable for @PolyNull locations" - : "type variable " + typeVariable; + if (polyNullContexts.values().stream() + .anyMatch( + context -> Objects.equals(context.inferenceVariable().asElement(), typeVariable))) { + return POLY_NULL_INFERENCE_FAILURE_MESSAGE; + } if (e.isCausedByNonNullUpperBound()) { return String.format( - "inference failure: %s is constrained to be @Nullable, but its upper bound requires it to be @NonNull", - typeVariableDescription); + "inference failure: type variable %s is constrained to be @Nullable, but its upper bound requires it to be @NonNull", + typeVariable); } return String.format( - "inference failure: %s constrained to be both @NonNull and @Nullable", - typeVariableDescription); + "inference failure: type variable %s constrained to be both @NonNull and @Nullable", + typeVariable); } /** Returns the type parameters whose nullability is inferred for {@code callTree}. */ @@ -3800,7 +3802,7 @@ private void reportPolyNullInferenceFailure( ErrorMessage errorMessage = new ErrorMessage( ErrorMessage.MessageTypes.GENERIC_INFERENCE_FAILURE, - "inference failure: polymorphic nullness constrained to both @NonNull and @Nullable"); + POLY_NULL_INFERENCE_FAILURE_MESSAGE); state.reportMatch( analysis .getErrorBuilder() diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java index d892104755..c349f39354 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/PolyNullLibraryModelsTests.java @@ -428,7 +428,7 @@ class Test { void test(Object nonNull, @Nullable Object nullable) { field = PolyNullMethods.genericObject(nonNull, nonNull); - // BUG: Diagnostic contains: type variable for @PolyNull locations + // BUG: Diagnostic contains: polymorphic nullness constrained to both @NonNull and @Nullable field = PolyNullMethods.genericObject(nullable, nullable); } } From d9510ff79c60312091eb23f3ba9c262cf1eee406 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 11:48:49 -0700 Subject: [PATCH 09/17] reuse some code --- .../nullaway/generics/GenericsChecks.java | 86 ++++--------------- 1 file changed, 15 insertions(+), 71 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 1dde9459b1..36326fd38f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1597,24 +1597,12 @@ private void generateConstraintsForCall( } } // 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, - polyNullContexts, - calledFromDataflow); - }); + generateArgumentConstraintsForCall( + state, path, solver, callTree, methodType, allCalls, polyNullContexts, calledFromDataflow); if (callTree instanceof MethodInvocationTree invocationTree && polyNullContext != null && !polyNullContext.inputs().isEmpty()) { - generateConstraintsForCallWithMethodType( + generateArgumentConstraintsForCall( state, path, solver, @@ -3592,7 +3580,7 @@ private record PolyNullInferenceContext( solver, state, calledFromDataflow); - generateConstraintsForCallWithMethodType( + generateArgumentConstraintsForCall( state, path, solver, @@ -3642,6 +3630,7 @@ private static Nullness resolvePolyNullInferenceContext( } /** Creates a method-type overlay with one shared variable at every modeled PolyNull input. */ + @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks private PolyNullInferenceContext createPolyNullInferenceContext( Symbol.MethodSymbol methodSymbol, Type.MethodType methodType, @@ -3655,13 +3644,7 @@ private PolyNullInferenceContext createPolyNullInferenceContext( if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { continue; } - Type parameterType = methodType.argtypes.get(parameterIndex); - Type locationType = typeAtPath(parameterType, location.typePath(), 0); - if (locationType == null) { - continue; - } inputsByParameter.computeIfAbsent(parameterIndex, unused -> new ArrayList<>()).add(location); - allInputs.add(location); } ListBuffer updatedParameterTypes = new ListBuffer<>(); int parameterIndex = 0; @@ -3671,8 +3654,12 @@ private PolyNullInferenceContext createPolyNullInferenceContext( Type updated = remaining.head; for (PolyNullLocation location : inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { - updated = + Type replaced = NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); + if (replaced != updated) { + updated = replaced; + allInputs.add(location); + } } updatedParameterTypes.append(updated); } @@ -3767,18 +3754,18 @@ private static Type.TypeVar createPolyNullInferenceVariable( return variable; } - /** Generates ordinary call constraints using a method type containing PolyNull variables. */ - private void generateConstraintsForCallWithMethodType( + /** Generates argument constraints for a call against the supplied method type. */ + private void generateArgumentConstraintsForCall( VisitorState state, @Nullable TreePath path, ConstraintSolver solver, - MethodInvocationTree invocationTree, + ExpressionTree callTree, Type.MethodType methodType, Set allCalls, IdentityHashMap polyNullContexts, boolean calledFromDataflow) { - TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), invocationTree); - new InvocationArguments(invocationTree, methodType) + TreePath pathToCall = path != null ? path : pathWithLeaf(state.getPath(), callTree); + new InvocationArguments(callTree, methodType) .forEach( (argument, argPos, formalParamType, unused) -> { TreePath pathToArgument = new TreePath(pathToCall, argument); @@ -3810,49 +3797,6 @@ private void reportPolyNullInferenceFailure( errorMessage, analysis.buildDescription(invocationTree), state, null)); } - /** Returns the nested type at {@code typePath}, or {@code null} if the path is inapplicable. */ - private static @Nullable Type typeAtPath( - Type type, ImmutableList typePath, int pathIndex) { - if (pathIndex == typePath.size()) { - return type; - } - TypePathEntry entry = typePath.get(pathIndex); - return switch (entry.kind()) { - case TYPE_ARGUMENT -> - type instanceof Type.ClassType classType - && entry.index() >= 0 - && entry.index() < classType.getTypeArguments().size() - ? typeAtPath(classType.getTypeArguments().get(entry.index()), typePath, pathIndex + 1) - : null; - case WILDCARD_BOUND -> { - Type bound = wildcardBound(type, entry.index()); - yield bound == null ? null : typeAtPath(bound, typePath, pathIndex + 1); - } - case ARRAY_ELEMENT -> - type instanceof Type.ArrayType arrayType - ? typeAtPath(arrayType.getComponentType(), typePath, pathIndex + 1) - : null; - }; - } - - /** Returns the requested bound of a wildcard, or {@code null} if that bound is unavailable. */ - private static @Nullable Type wildcardBound(Type type, int boundIndex) { - if (!(type instanceof Type.WildcardType wildcardType)) { - return null; - } - if (boundIndex == 0) { - if (wildcardType.kind == BoundKind.EXTENDS) { - return wildcardType.type; - } - if (wildcardType.kind == BoundKind.UNBOUND && wildcardType.bound != null) { - return wildcardType.bound.getUpperBound(); - } - } else if (boundIndex == 1 && wildcardType.kind == BoundKind.SUPER) { - return wildcardType.type; - } - return null; - } - /** Applies one resolved polymorphic-nullness annotation to a method type component. */ private static Type applyPolyNullAnnotation( Type type, ImmutableList typePath, Nullness nullness, VisitorState state) { From efd49f7725ad7d0795b95daa1dfa1680feff1a4f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 12:02:33 -0700 Subject: [PATCH 10/17] Move some PolyNull code into its own class --- .../nullaway/generics/GenericsChecks.java | 219 +++--------------- .../nullaway/generics/PolyNullInference.java | 199 ++++++++++++++++ 2 files changed, 229 insertions(+), 189 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java 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 36326fd38f..e577535d4e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -8,7 +8,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; import com.google.errorprone.util.ASTHelpers; @@ -44,7 +43,6 @@ import com.sun.tools.javac.code.Types; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.TreeInfo; -import com.sun.tools.javac.util.ListBuffer; import com.sun.tools.javac.util.Name; import com.sun.tools.javac.util.Names; import com.uber.nullaway.CodeAnnotationInfo; @@ -61,9 +59,9 @@ 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 com.uber.nullaway.libmodel.NestedAnnotationInfo.TypePathEntry; -import com.uber.nullaway.librarymodel.NestedTypePathUpdater; import java.util.ArrayList; import java.util.HashMap; import java.util.IdentityHashMap; @@ -88,16 +86,9 @@ public final class GenericsChecks { /** Types resolved for a method reference using its ground target type. */ public record ResolvedMethodReference(Type.MethodType methodType, @Nullable Type qualifierType) {} - /** Diagnostic for incompatible constraints on modeled PolyNull locations. */ - private static final String POLY_NULL_INFERENCE_FAILURE_MESSAGE = - "inference failure: polymorphic nullness constrained to both @NonNull and @Nullable"; - /** Marker interface for results of attempting to infer nullability of type variables at a call */ private interface CallInferenceResult {} - /** The result of resolving PolyNull for one invocation in a generic inference session. */ - private record PolyNullInferenceResult(@Nullable Nullness nullness) {} - /** * Indicates successful inference of nullability of type variables at a call. Stores the inferred * type variable nullability and any PolyNull resolutions computed by the same solver run. @@ -1313,11 +1304,10 @@ private Type inferCallType( successResult.polyNullResults().get(invocationTree); if (polyNullResult != null && polyNullResult.nullness() != null) { inferredCallType = - applyPolyNullToReturnType( + PolyNullInference.applyToReturnType( inferredCallType, handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state), - polyNullResult.nullness(), - state); + polyNullAnnotationType(polyNullResult.nullness(), state)); } } return inferredCallType; @@ -1378,7 +1368,7 @@ private CallInferenceResult runInferenceForCall( } IdentityHashMap polyNullResults = - resolvePolyNullInferenceContexts(polyNullContexts, typeVarNullability); + 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 @@ -1454,10 +1444,8 @@ private String inferenceFailureMessage( UnsatisfiableConstraintsException e, IdentityHashMap polyNullContexts) { Element typeVariable = e.getTypeVariable(); - if (polyNullContexts.values().stream() - .anyMatch( - context -> Objects.equals(context.inferenceVariable().asElement(), typeVariable))) { - return POLY_NULL_INFERENCE_FAILURE_MESSAGE; + if (PolyNullInference.containsInferenceVariable(polyNullContexts, typeVariable)) { + return PolyNullInference.INFERENCE_FAILURE_MESSAGE; } if (e.isCausedByNonNullUpperBound()) { return String.format( @@ -1576,7 +1564,12 @@ private void generateConstraintsForCall( polyNullContexts.computeIfAbsent( invocationTree, unused -> - createPolyNullInferenceContext(methodSymbol, methodType, locations, state)); + PolyNullInference.createContext( + methodSymbol, + methodType, + locations, + getSyntheticNullableAnnotType(state), + state)); } } // first, handle the call result flow @@ -1587,7 +1580,7 @@ private void generateConstraintsForCall( : getConstructedTypeAtCallSite((NewClassTree) callTree).tsym.type; solver.addSubtypeConstraint(callResultType, typeFromAssignmentContext, assignedToLocal); if (polyNullContext != null) { - addPolyNullResultConstraints( + PolyNullInference.addResultConstraints( solver, methodType.getReturnType(), polyNullLocations, @@ -3465,7 +3458,6 @@ public boolean hasPolyNullModel(Symbol.MethodSymbol methodSymbol, VisitorState s * substituted into the invoked method type. The linked nullness is inferred from all modeled * inputs and any available result target. */ - @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) private Type.MethodType applyPolyNullModel( Symbol.MethodSymbol methodSymbol, MethodInvocationTree invocationTree, @@ -3492,58 +3484,10 @@ private Type.MethodType applyPolyNullModel( if (polyNullness == null) { return substitutedMethodType; } - boolean changed = false; - ListBuffer updatedParameterTypes = new ListBuffer<>(); - int parameterIndex = 0; - for (com.sun.tools.javac.util.List remaining = substitutedMethodType.argtypes; - remaining.nonEmpty(); - remaining = remaining.tail, parameterIndex++) { - Type parameterType = remaining.head; - Type updatedParameterType = parameterType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == parameterIndex) { - updatedParameterType = - applyPolyNullAnnotation( - updatedParameterType, location.typePath(), polyNullness, state); - } - } - updatedParameterTypes.append(updatedParameterType); - changed |= updatedParameterType != parameterType; - } - Type returnType = substitutedMethodType.restype; - Type updatedReturnType = applyPolyNullToReturnType(returnType, locations, polyNullness, state); - changed |= updatedReturnType != returnType; - return changed - ? new Type.MethodType( - updatedParameterTypes.toList(), - updatedReturnType, - substitutedMethodType.thrown, - substitutedMethodType.tsym) - : substitutedMethodType; - } - - /** Applies a resolved PolyNull value to all modeled locations within a return type. */ - private static Type applyPolyNullToReturnType( - Type returnType, - ImmutableSet locations, - Nullness polyNullness, - VisitorState state) { - Type updatedReturnType = returnType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == -1) { - updatedReturnType = - applyPolyNullAnnotation(updatedReturnType, location.typePath(), polyNullness, state); - } - } - return updatedReturnType; + return PolyNullInference.applyToMethodType( + substitutedMethodType, locations, polyNullAnnotationType(polyNullness, state)); } - /** The modeled input overlay and shared PolyNull variable for one call. */ - private record PolyNullInferenceContext( - Type.MethodType inferenceMethodType, - ImmutableList inputs, - Type.TypeVar inferenceVariable) {} - /** * Infers the nullness shared by all PolyNull occurrences using the generic constraint solver. * @@ -3564,7 +3508,12 @@ private record PolyNullInferenceContext( return cached; } PolyNullInferenceContext inferenceContext = - createPolyNullInferenceContext(methodSymbol, substitutedMethodType, locations, state); + PolyNullInference.createContext( + methodSymbol, + substitutedMethodType, + locations, + getSyntheticNullableAnnotType(state), + state); if (inferenceContext.inputs().isEmpty()) { return null; } @@ -3590,7 +3539,7 @@ private record PolyNullInferenceContext( new IdentityHashMap<>(), calledFromDataflow); Map solution = solver.solve(); - Nullness resolved = resolvePolyNullInferenceContext(inferenceContext, solution); + Nullness resolved = PolyNullInference.resolveContext(inferenceContext, solution); if (resolved != null && !calledFromDataflow) { polyNullResolutions.put(invocationTree, resolved); } @@ -3601,75 +3550,6 @@ private record PolyNullInferenceContext( } } - /** Resolves all PolyNull contexts after a shared generic-inference solver run. */ - private IdentityHashMap - resolvePolyNullInferenceContexts( - IdentityHashMap contexts, - Map solution) { - IdentityHashMap results = - new IdentityHashMap<>(); - for (Map.Entry entry : contexts.entrySet()) { - results.put( - entry.getKey(), - new PolyNullInferenceResult(resolvePolyNullInferenceContext(entry.getValue(), solution))); - } - return results; - } - - /** Resolves the shared PolyNull variable for one invocation. */ - private static Nullness resolvePolyNullInferenceContext( - 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 input. */ - @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks - private PolyNullInferenceContext createPolyNullInferenceContext( - Symbol.MethodSymbol methodSymbol, - Type.MethodType methodType, - ImmutableSet locations, - VisitorState state) { - Map> inputsByParameter = new LinkedHashMap<>(); - ImmutableList.Builder allInputs = ImmutableList.builder(); - Type.TypeVar inferenceVariable = createPolyNullInferenceVariable(methodSymbol, 0, state); - for (PolyNullLocation location : locations) { - int parameterIndex = location.parameterIndex(); - if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { - continue; - } - inputsByParameter.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 : - inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { - Type replaced = - NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); - if (replaced != updated) { - updated = replaced; - allInputs.add(location); - } - } - updatedParameterTypes.append(updated); - } - return new PolyNullInferenceContext( - new Type.MethodType( - updatedParameterTypes.toList(), methodType.restype, methodType.thrown, methodType.tsym), - allInputs.build(), - inferenceVariable); - } - /** * Constrains a standalone PolyNull call's modeled result using a directly enclosing assignment, * variable initialization, method return, or conditional-expression target. @@ -3708,7 +3588,7 @@ private void addPolyNullResultConstraintsFromDirectAssignmentContext( CallAndContext callAndContext = getDirectCallContextForInference(invocationPath, state, calledFromDataflow); if (callAndContext.typeFromAssignmentContext() != null) { - addPolyNullResultConstraints( + PolyNullInference.addResultConstraints( solver, methodType.getReturnType(), locations, @@ -3718,42 +3598,6 @@ private void addPolyNullResultConstraintsFromDirectAssignmentContext( } } - /** Adds a call-result subtype constraint using the invocation's shared PolyNull variable. */ - private static void addPolyNullResultConstraints( - ConstraintSolver solver, - Type returnType, - ImmutableSet locations, - PolyNullInferenceContext inferenceContext, - Type targetType, - boolean assignedToLocal) { - if (locations.stream().noneMatch(location -> location.parameterIndex() == -1)) { - return; - } - Type inferenceReturnType = returnType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == -1) { - inferenceReturnType = - NestedTypePathUpdater.replaceType( - inferenceReturnType, location.typePath(), inferenceContext.inferenceVariable()); - } - } - solver.addSubtypeConstraint(inferenceReturnType, targetType, assignedToLocal); - } - - /** Creates the nullable-bounded synthetic type variable for one PolyNull invocation. */ - private static Type.TypeVar createPolyNullInferenceVariable( - Symbol.MethodSymbol methodSymbol, int group, VisitorState state) { - Symbol.TypeVariableSymbol symbol = - new Symbol.TypeVariableSymbol( - 0, state.getName("$PolyNull$" + group), Type.noType, methodSymbol); - Type nullableObject = - TypeSubstitutionUtils.typeWithAnnot( - state.getSymtab().objectType, getSyntheticNullableAnnotType(state)); - Type.TypeVar variable = new Type.TypeVar(symbol, nullableObject, state.getSymtab().botType); - symbol.type = variable; - return variable; - } - /** Generates argument constraints for a call against the supplied method type. */ private void generateArgumentConstraintsForCall( VisitorState state, @@ -3789,7 +3633,7 @@ private void reportPolyNullInferenceFailure( ErrorMessage errorMessage = new ErrorMessage( ErrorMessage.MessageTypes.GENERIC_INFERENCE_FAILURE, - POLY_NULL_INFERENCE_FAILURE_MESSAGE); + PolyNullInference.INFERENCE_FAILURE_MESSAGE); state.reportMatch( analysis .getErrorBuilder() @@ -3797,14 +3641,11 @@ private void reportPolyNullInferenceFailure( errorMessage, analysis.buildDescription(invocationTree), state, null)); } - /** Applies one resolved polymorphic-nullness annotation to a method type component. */ - private static Type applyPolyNullAnnotation( - Type type, ImmutableList typePath, Nullness nullness, VisitorState state) { - Type annotationType = - nullness == Nullness.NULLABLE - ? getSyntheticNullableAnnotType(state) - : getSyntheticNonNullAnnotType(state); - return NestedTypePathUpdater.addAnnotation(type, typePath, annotationType); + /** 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); } /** 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..b3f2a13302 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java @@ -0,0 +1,199 @@ +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 input overlay and shared PolyNull variable for one call. */ + record PolyNullInferenceContext( + Type.MethodType inferenceMethodType, + ImmutableList inputs, + Type.TypeVar inferenceVariable) {} + + 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 input. */ + @SuppressWarnings({"ReferenceEquality", "TypeEquals"}) // deliberate reference equality checks + static PolyNullInferenceContext createContext( + Symbol.MethodSymbol methodSymbol, + Type.MethodType methodType, + ImmutableSet locations, + Type nullableAnnotationType, + VisitorState state) { + Map> inputsByParameter = new LinkedHashMap<>(); + ImmutableList.Builder allInputs = 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; + } + inputsByParameter.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 : + inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { + Type replaced = + NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); + if (replaced != updated) { + updated = replaced; + allInputs.add(location); + } + } + updatedParameterTypes.append(updated); + } + return new PolyNullInferenceContext( + new Type.MethodType( + updatedParameterTypes.toList(), methodType.restype, methodType.thrown, methodType.tsym), + allInputs.build(), + inferenceVariable); + } + + /** Adds a call-result subtype constraint using the invocation's shared PolyNull variable. */ + static void addResultConstraints( + ConstraintSolver solver, + Type returnType, + ImmutableSet locations, + PolyNullInferenceContext inferenceContext, + Type targetType, + boolean assignedToLocal) { + if (locations.stream().noneMatch(location -> location.parameterIndex() == -1)) { + return; + } + Type inferenceReturnType = returnType; + for (PolyNullLocation location : locations) { + if (location.parameterIndex() == -1) { + inferenceReturnType = + NestedTypePathUpdater.replaceType( + inferenceReturnType, location.typePath(), inferenceContext.inferenceVariable()); + } + } + solver.addSubtypeConstraint(inferenceReturnType, 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; + } +} From 43327f36a7ca4a8080d28329010aa14dba7b9664 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 20:29:42 -0700 Subject: [PATCH 11/17] cleanup --- .../nullaway/generics/GenericsChecks.java | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index e577535d4e..bbc6069f1d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -3064,8 +3064,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, @@ -3073,40 +3073,26 @@ public Nullness getGenericReturnNullnessAtInvocation( TreePath path, VisitorState state, boolean calledFromDataflow) { - if (hasPolyNullModel(invokedMethodSymbol, state)) { - Type.MethodType invokedMethodType = - getInvokedMethodTypeAtCall(invokedMethodSymbol, tree, path, state, calledFromDataflow); - return getTypeNullnessForRead(invokedMethodType.getReturnType(), state); - } + 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)) { - 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) - .methodType() - .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) { + // 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; - } else { - return getGenericMethodReturnTypeNullness(invokedMethodSymbol, enclosingType, state); } + Type.MethodType invokedMethodType = + getInvokedMethodTypeAtCall(invokedMethodSymbol, tree, path, state, calledFromDataflow); + Type declaredReturnType = invokedMethodSymbol.getReturnType(); + boolean returnsMethodTypeVariable = + declaredReturnType instanceof Type.TypeVar typeVariable + && Objects.equals(typeVariable.tsym.owner, invokedMethodSymbol); + // For an ordinary method type variable, inference supplies a qualifier that can be more + // precise than the effective upper bound of a wildcard capture. PolyNull and receiver type + // variables instead describe a value read from the resolved return type, including captures. + return returnsMethodTypeVariable && !polyNullModeled + ? getTypeNullness(invokedMethodType.getReturnType()) + : getTypeNullnessForRead(invokedMethodType.getReturnType(), state); } private static com.sun.tools.javac.util.List convertTreesToTypes( From cc047b799b3b4ba69ce6bbcd08108bbce7634878 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 20:31:50 -0700 Subject: [PATCH 12/17] more cleanup --- .../uber/nullaway/generics/GenericsChecks.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index bbc6069f1d..bbb35c81c2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -3083,16 +3083,7 @@ public Nullness getGenericReturnNullnessAtInvocation( } Type.MethodType invokedMethodType = getInvokedMethodTypeAtCall(invokedMethodSymbol, tree, path, state, calledFromDataflow); - Type declaredReturnType = invokedMethodSymbol.getReturnType(); - boolean returnsMethodTypeVariable = - declaredReturnType instanceof Type.TypeVar typeVariable - && Objects.equals(typeVariable.tsym.owner, invokedMethodSymbol); - // For an ordinary method type variable, inference supplies a qualifier that can be more - // precise than the effective upper bound of a wildcard capture. PolyNull and receiver type - // variables instead describe a value read from the resolved return type, including captures. - return returnsMethodTypeVariable && !polyNullModeled - ? getTypeNullness(invokedMethodType.getReturnType()) - : getTypeNullnessForRead(invokedMethodType.getReturnType(), state); + return getTypeNullnessForRead(invokedMethodType.getReturnType(), state); } private static com.sun.tools.javac.util.List convertTreesToTypes( @@ -3954,6 +3945,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); From 26f54786b707681fedcd302df46a2167ed3f4e25 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Sep 2026 22:28:48 -0700 Subject: [PATCH 13/17] cleanup --- .../nullaway/generics/GenericsChecks.java | 24 ++----- .../nullaway/generics/PolyNullInference.java | 62 ++++++++++++------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index bbb35c81c2..5f06cb2ace 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1581,12 +1581,7 @@ private void generateConstraintsForCall( solver.addSubtypeConstraint(callResultType, typeFromAssignmentContext, assignedToLocal); if (polyNullContext != null) { PolyNullInference.addResultConstraints( - solver, - methodType.getReturnType(), - polyNullLocations, - polyNullContext, - typeFromAssignmentContext, - assignedToLocal); + solver, polyNullContext, typeFromAssignmentContext, assignedToLocal); } } // then, handle parameters @@ -1594,7 +1589,7 @@ private void generateConstraintsForCall( state, path, solver, callTree, methodType, allCalls, polyNullContexts, calledFromDataflow); if (callTree instanceof MethodInvocationTree invocationTree && polyNullContext != null - && !polyNullContext.inputs().isEmpty()) { + && polyNullContext.hasInputLocations()) { generateArgumentConstraintsForCall( state, path, @@ -3491,21 +3486,14 @@ private Type.MethodType applyPolyNullModel( locations, getSyntheticNullableAnnotType(state), state); - if (inferenceContext.inputs().isEmpty()) { + if (!inferenceContext.hasInputLocations()) { return null; } ConstraintSolver solver = makeSolver(state, analysis); Set nestedCalls = new LinkedHashSet<>(); try { addPolyNullResultConstraintsFromDirectAssignmentContext( - invocationTree, - substitutedMethodType, - locations, - inferenceContext, - path, - solver, - state, - calledFromDataflow); + invocationTree, inferenceContext, path, solver, state, calledFromDataflow); generateArgumentConstraintsForCall( state, path, @@ -3533,8 +3521,6 @@ private Type.MethodType applyPolyNullModel( */ private void addPolyNullResultConstraintsFromDirectAssignmentContext( MethodInvocationTree invocationTree, - Type.MethodType methodType, - ImmutableSet locations, PolyNullInferenceContext inferenceContext, @Nullable TreePath path, ConstraintSolver solver, @@ -3567,8 +3553,6 @@ private void addPolyNullResultConstraintsFromDirectAssignmentContext( if (callAndContext.typeFromAssignmentContext() != null) { PolyNullInference.addResultConstraints( solver, - methodType.getReturnType(), - locations, inferenceContext, callAndContext.typeFromAssignmentContext(), callAndContext.assignedToLocal()); diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java index b3f2a13302..1ea4bddcda 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java @@ -28,11 +28,22 @@ final class PolyNullInference { /** The result of resolving PolyNull for one invocation in a generic inference session. */ record PolyNullInferenceResult(@Nullable Nullness nullness) {} - /** The modeled input overlay and shared PolyNull variable for one call. */ + /** The modeled method-type overlay and shared PolyNull variable for one call. */ record PolyNullInferenceContext( Type.MethodType inferenceMethodType, - ImmutableList inputs, - Type.TypeVar inferenceVariable) {} + 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() {} @@ -93,7 +104,7 @@ static Nullness resolveContext( : Nullness.NONNULL; } - /** Creates a method-type overlay with one shared variable at every modeled PolyNull input. */ + /** 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, @@ -101,8 +112,8 @@ static PolyNullInferenceContext createContext( ImmutableSet locations, Type nullableAnnotationType, VisitorState state) { - Map> inputsByParameter = new LinkedHashMap<>(); - ImmutableList.Builder allInputs = ImmutableList.builder(); + Map> locationsByParameter = new LinkedHashMap<>(); + ImmutableList.Builder appliedLocations = ImmutableList.builder(); Type.TypeVar inferenceVariable = createInferenceVariable(methodSymbol, 0, nullableAnnotationType, state); for (PolyNullLocation location : locations) { @@ -110,7 +121,9 @@ static PolyNullInferenceContext createContext( if (parameterIndex < 0 || parameterIndex >= methodType.argtypes.size()) { continue; } - inputsByParameter.computeIfAbsent(parameterIndex, unused -> new ArrayList<>()).add(location); + locationsByParameter + .computeIfAbsent(parameterIndex, unused -> new ArrayList<>()) + .add(location); } ListBuffer updatedParameterTypes = new ListBuffer<>(); int parameterIndex = 0; @@ -119,43 +132,46 @@ static PolyNullInferenceContext createContext( remaining = remaining.tail, parameterIndex++) { Type updated = remaining.head; for (PolyNullLocation location : - inputsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { + locationsByParameter.getOrDefault(parameterIndex, java.util.List.of())) { Type replaced = NestedTypePathUpdater.replaceType(updated, location.typePath(), inferenceVariable); if (replaced != updated) { updated = replaced; - allInputs.add(location); + 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(), methodType.restype, methodType.thrown, methodType.tsym), - allInputs.build(), + 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, - Type returnType, - ImmutableSet locations, PolyNullInferenceContext inferenceContext, Type targetType, boolean assignedToLocal) { - if (locations.stream().noneMatch(location -> location.parameterIndex() == -1)) { + if (!inferenceContext.hasReturnLocations()) { return; } - Type inferenceReturnType = returnType; - for (PolyNullLocation location : locations) { - if (location.parameterIndex() == -1) { - inferenceReturnType = - NestedTypePathUpdater.replaceType( - inferenceReturnType, location.typePath(), inferenceContext.inferenceVariable()); - } - } - solver.addSubtypeConstraint(inferenceReturnType, targetType, assignedToLocal); + solver.addSubtypeConstraint( + inferenceContext.inferenceMethodType().getReturnType(), targetType, assignedToLocal); } /** Returns whether {@code typeVariable} is a PolyNull variable from one of {@code contexts}. */ From 1f52bb7f35e2e2a5c4eb13c33cade8d94a605ef4 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 7 Sep 2026 17:27:54 -0700 Subject: [PATCH 14/17] Preserve captured wildcard fallback after rebase Retain the upstream fallback to the capture upper bound when javac omits the formal type variable on an unbounded wildcard. Assisted-by: Codex (gpt-6) --- .../librarymodel/NestedTypePathUpdater.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java index 44bf7ab861..dd41e5d238 100644 --- a/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java +++ b/nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedTypePathUpdater.java @@ -144,22 +144,30 @@ 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 + // bound in that case, on a detached copy so neither path traversal nor annotation updates + // mutate compiler-owned types. + wildcard = + TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(wildcard, t, t.getUpperBound()); + } Type.WildcardType updatedWildcard; if (pathIndex < typePath.size()) { - updatedWildcard = (Type.WildcardType) t.wildcard.accept(this, pathIndex); + updatedWildcard = (Type.WildcardType) wildcard.accept(this, pathIndex); } else { Verify.verify(pathIndex == typePath.size(), "path index out of bounds"); - if (t.wildcard.kind == BoundKind.UNBOUND) { + if (wildcard.kind == BoundKind.UNBOUND) { Type.TypeVar formalTypeVariable = Verify.verifyNotNull( - t.wildcard.bound, "unbounded wildcard has no corresponding formal type variable"); + wildcard.bound, "unbounded wildcard has no corresponding formal type variable"); Type updatedUpperBound = TypeSubstitutionUtils.typeWithAnnot(formalTypeVariable.getUpperBound(), updateType); updatedWildcard = - TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(t.wildcard, updatedUpperBound); + TypeSubstitutionUtils.replaceUnboundedWildcardUpperBound(wildcard, updatedUpperBound); } else { - Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(t.wildcard.type, updateType); - updatedWildcard = TYPE_METADATA_BUILDER.createWildcardType(t.wildcard, updatedBound); + Type updatedBound = TypeSubstitutionUtils.typeWithAnnot(wildcard.type, updateType); + updatedWildcard = TYPE_METADATA_BUILDER.createWildcardType(wildcard, updatedBound); } } if (updatedWildcard == t.wildcard) { From c44d002e616dde70b2c64dce2c9c6ee3d8b49468 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 17 Sep 2026 18:24:56 -0700 Subject: [PATCH 15/17] register inference variables --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 2 ++ 1 file changed, 2 insertions(+) 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 5f06cb2ace..284acabb53 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1570,6 +1570,7 @@ private void generateConstraintsForCall( locations, getSyntheticNullableAnnotType(state), state)); + solver.registerInferenceVariable(polyNullContext.inferenceVariable().asElement()); } } // first, handle the call result flow @@ -3490,6 +3491,7 @@ private Type.MethodType applyPolyNullModel( return null; } ConstraintSolver solver = makeSolver(state, analysis); + solver.registerInferenceVariable(inferenceContext.inferenceVariable().asElement()); Set nestedCalls = new LinkedHashSet<>(); try { addPolyNullResultConstraintsFromDirectAssignmentContext( From 153518afa198c191849614e2dd79897f5d04beae Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 17 Sep 2026 20:10:38 -0700 Subject: [PATCH 16/17] remove a wrapper type --- .../nullaway/generics/GenericsChecks.java | 47 ++++++++----------- .../nullaway/generics/PolyNullInference.java | 16 ++----- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 284acabb53..bb6b4ec6c5 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -60,7 +60,6 @@ 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; @@ -95,14 +94,14 @@ private interface CallInferenceResult {} */ private record InferenceSuccess( Map typeVarNullability, - IdentityHashMap polyNullResults) + IdentityHashMap polyNullnessByInvocation) implements CallInferenceResult {} /** * A generic method type after substitution, together with any jointly inferred PolyNull value. */ private record MethodTypeSubstitution( - Type.MethodType methodType, @Nullable PolyNullInferenceResult polyNullResult) {} + Type.MethodType methodType, @Nullable Nullness polyNullness) {} /** Indicates failed inference of nullability of type variables at a call */ private record InferenceFailure(@SuppressWarnings("UnusedVariable") @Nullable String errorMessage) @@ -1300,14 +1299,13 @@ private Type inferCallType( TypeSubstitutionUtils.updateTypeWithInferredNullability( typeAtCallSite, methodReturnType, typeVarNullability, state, config); if (result instanceof InferenceSuccess successResult) { - PolyNullInferenceResult polyNullResult = - successResult.polyNullResults().get(invocationTree); - if (polyNullResult != null && polyNullResult.nullness() != null) { + Nullness polyNullness = successResult.polyNullnessByInvocation().get(invocationTree); + if (polyNullness != null) { inferredCallType = PolyNullInference.applyToReturnType( inferredCallType, handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state), - polyNullAnnotationType(polyNullResult.nullness(), state)); + polyNullAnnotationType(polyNullness, state)); } } return inferredCallType; @@ -1367,17 +1365,16 @@ private CallInferenceResult runInferenceForCall( typeVarNullability.putIfAbsent(typeVar, ConstraintSolver.InferredNullability.NONNULL); } - IdentityHashMap polyNullResults = - PolyNullInference.resolveContexts(polyNullContexts, typeVarNullability); - InferenceSuccess successResult = new InferenceSuccess(typeVarNullability, polyNullResults); + IdentityHashMap polyNullnessByInvocation = + PolyNullInference.resolveNullnessByInvocation(polyNullContexts, typeVarNullability); + InferenceSuccess successResult = + new InferenceSuccess(typeVarNullability, polyNullnessByInvocation); // 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 (Map.Entry entry : + polyNullnessByInvocation.entrySet()) { + polyNullResolutions.put(entry.getKey(), entry.getValue()); } for (Tree inferredCall : allCalls) { inferredTypeVarNullabilityForGenericCalls.put(inferredCall, successResult); @@ -3161,14 +3158,10 @@ private MethodTypeSubstitution substituteTypeArgsInGenericMethodType( TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); return new MethodTypeSubstitution( - substitutedMethodType, successResult.polyNullResults().get(invocationTree)); + substitutedMethodType, successResult.polyNullnessByInvocation().get(invocationTree)); } else { // inference failed; just return the method type at the call site with no substitutions - PolyNullInferenceResult failedPolyNullInference = - handler.onGetPolyNullLocations(ASTHelpers.getSymbol(invocationTree), state).isEmpty() - ? null - : new PolyNullInferenceResult(null); - return new MethodTypeSubstitution(methodTypeAtCallSite, failedPolyNullInference); + return new MethodTypeSubstitution(methodTypeAtCallSite, null); } } return new MethodTypeSubstitution( @@ -3395,13 +3388,13 @@ private Type.MethodType getInvokedMethodTypeAtCall( invokedMethodType = TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, methodSymbol, config); } - PolyNullInferenceResult jointlyInferredPolyNull = null; + Nullness jointlyInferredPolyNullness = null; if (tree instanceof MethodInvocationTree && invokedMethodType instanceof Type.ForAll forAllType) { MethodTypeSubstitution substitution = substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow); invokedMethodType = substitution.methodType(); - jointlyInferredPolyNull = substitution.polyNullResult(); + jointlyInferredPolyNullness = substitution.polyNullness(); } Type.MethodType modeledMethodType = handler.onOverrideMethodType( @@ -3414,7 +3407,7 @@ private Type.MethodType getInvokedMethodTypeAtCall( methodSymbol, invocationTree, modeledMethodType, - jointlyInferredPolyNull, + jointlyInferredPolyNullness, path, state, calledFromDataflow) @@ -3435,7 +3428,7 @@ private Type.MethodType applyPolyNullModel( Symbol.MethodSymbol methodSymbol, MethodInvocationTree invocationTree, Type.MethodType substitutedMethodType, - @Nullable PolyNullInferenceResult jointlyInferredPolyNull, + @Nullable Nullness jointlyInferredPolyNullness, @Nullable TreePath path, VisitorState state, boolean calledFromDataflow) { @@ -3444,8 +3437,8 @@ private Type.MethodType applyPolyNullModel( return substitutedMethodType; } Nullness polyNullness = - jointlyInferredPolyNull != null - ? jointlyInferredPolyNull.nullness() + jointlyInferredPolyNullness != null + ? jointlyInferredPolyNullness : inferPolyNullness( methodSymbol, invocationTree, diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java index 1ea4bddcda..9c65e3e026 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PolyNullInference.java @@ -16,7 +16,6 @@ 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 { @@ -25,9 +24,6 @@ final class PolyNullInference { 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, @@ -78,17 +74,15 @@ static Type applyToReturnType( return applyToType(returnType, -1, locations, annotationType); } - /** Resolves all PolyNull contexts after a shared generic-inference solver run. */ - static IdentityHashMap resolveContexts( + /** Resolves PolyNull nullness by invocation after a shared generic-inference solver run. */ + static IdentityHashMap resolveNullnessByInvocation( IdentityHashMap contexts, Map solution) { - IdentityHashMap results = - new IdentityHashMap<>(); + IdentityHashMap nullnessByInvocation = new IdentityHashMap<>(); for (Map.Entry entry : contexts.entrySet()) { - results.put( - entry.getKey(), new PolyNullInferenceResult(resolveContext(entry.getValue(), solution))); + nullnessByInvocation.put(entry.getKey(), resolveContext(entry.getValue(), solution)); } - return results; + return nullnessByInvocation; } /** Resolves the shared PolyNull variable for one invocation. */ From 2811af1240da363022a116a164d9ba0b5c06d557 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 17 Sep 2026 20:53:41 -0700 Subject: [PATCH 17/17] more --- .../java/com/uber/nullaway/LibraryModels.java | 3 +- .../nullaway/generics/GenericsChecks.java | 69 ++++++++++--------- .../handlers/LibraryModelsHandler.java | 57 +++++++++++++-- 3 files changed, 88 insertions(+), 41 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index 582c73d1c1..7ff67f0455 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -148,7 +148,8 @@ default ImmutableSetMultimap ensuresNonNullIfTrueMethodCal * 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}. + * modeled as {@code @NullMarked}. A PolyNull location takes precedence over any fixed nullable or + * non-null library model for the same location. * * @return map from methods to signature locations with polymorphic nullness */ 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 bb6b4ec6c5..7609ea779b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -90,19 +90,13 @@ private interface CallInferenceResult {} /** * Indicates successful inference of nullability of type variables at a call. Stores the inferred - * type variable nullability and any PolyNull resolutions computed by the same solver run. + * type variable nullability and any PolyNull nullability computed by the same solver run. */ private record InferenceSuccess( Map typeVarNullability, IdentityHashMap polyNullnessByInvocation) implements CallInferenceResult {} - /** - * A generic method type after substitution, together with any jointly inferred PolyNull value. - */ - private record MethodTypeSubstitution( - Type.MethodType methodType, @Nullable Nullness polyNullness) {} - /** Indicates failed inference of nullability of type variables at a call */ private record InferenceFailure(@SuppressWarnings("UnusedVariable") @Nullable String errorMessage) implements CallInferenceResult { @@ -3098,24 +3092,30 @@ 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 and any PolyNull value inferred in the same solver run + * @return the substituted method type with library models applied */ - private MethodTypeSubstitution substituteTypeArgsInGenericMethodType( + private Type.MethodType substituteTypeArgsInGenericMethodType( Tree tree, Type.ForAll forAllType, @Nullable TreePath path, VisitorState state, boolean calledFromDataflow) { Type.MethodType methodType = forAllType.asMethodType(); + MethodInvocationTree invocationTree = + tree instanceof MethodInvocationTree methodInvocationTree ? methodInvocationTree : null; + Symbol.MethodSymbol methodSymbol = + (Symbol.MethodSymbol) castToNonNull(ASTHelpers.getSymbol(tree)); List typeArgumentTrees = - (tree instanceof MethodInvocationTree methodInvocationTree) - ? methodInvocationTree.getTypeArguments() + invocationTree != null + ? invocationTree.getTypeArguments() : ((NewClassTree) tree).getTypeArguments(); com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); + Type.MethodType substitutedMethodType; + Nullness jointlyInferredPolyNullness = null; // There are no explicit type arguments, so use the inferred types - if (explicitTypeArgs.isEmpty() && tree instanceof MethodInvocationTree invocationTree) { + if (explicitTypeArgs.isEmpty() && invocationTree != null) { CallInferenceResult result = inferredTypeVarNullabilityForGenericCalls.get(tree); if (result == null) { // have not yet attempted inference for this call @@ -3154,21 +3154,32 @@ private MethodTypeSubstitution substituteTypeArgsInGenericMethodType( nestedNullabilityRepairInProgress.remove(invocationTree); } } - Type.MethodType substitutedMethodType = + substitutedMethodType = TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); - return new MethodTypeSubstitution( - substitutedMethodType, successResult.polyNullnessByInvocation().get(invocationTree)); + jointlyInferredPolyNullness = successResult.polyNullnessByInvocation().get(invocationTree); } else { // inference failed; just return the method type at the call site with no substitutions - return new MethodTypeSubstitution(methodTypeAtCallSite, null); + substitutedMethodType = methodTypeAtCallSite; } + } else { + substitutedMethodType = + TypeSubstitutionUtils.subst( + state.getTypes(), methodType, forAllType.tvars, explicitTypeArgs, config) + .asMethodType(); } - return new MethodTypeSubstitution( - TypeSubstitutionUtils.subst( - state.getTypes(), methodType, forAllType.tvars, explicitTypeArgs, config) - .asMethodType(), - null); + Type.MethodType modeledMethodType = + handler.onOverrideMethodType(methodSymbol, substitutedMethodType, state, invocationTree); + return invocationTree == null + ? modeledMethodType + : applyPolyNullModel( + methodSymbol, + invocationTree, + modeledMethodType, + jointlyInferredPolyNullness, + path, + state, + calledFromDataflow); } /** @@ -3388,13 +3399,10 @@ private Type.MethodType getInvokedMethodTypeAtCall( invokedMethodType = TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, methodSymbol, config); } - Nullness jointlyInferredPolyNullness = null; if (tree instanceof MethodInvocationTree && invokedMethodType instanceof Type.ForAll forAllType) { - MethodTypeSubstitution substitution = - substituteTypeArgsInGenericMethodType(tree, forAllType, path, state, calledFromDataflow); - invokedMethodType = substitution.methodType(); - jointlyInferredPolyNullness = substitution.polyNullness(); + return substituteTypeArgsInGenericMethodType( + tree, forAllType, path, state, calledFromDataflow); } Type.MethodType modeledMethodType = handler.onOverrideMethodType( @@ -3404,13 +3412,7 @@ private Type.MethodType getInvokedMethodTypeAtCall( tree instanceof MethodInvocationTree invocationTree ? invocationTree : null); return tree instanceof MethodInvocationTree invocationTree ? applyPolyNullModel( - methodSymbol, - invocationTree, - modeledMethodType, - jointlyInferredPolyNullness, - path, - state, - calledFromDataflow) + methodSymbol, invocationTree, modeledMethodType, null, path, state, calledFromDataflow) : modeledMethodType; } @@ -3648,7 +3650,6 @@ 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 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 7b0a60182e..ac3ea397ac 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1527,15 +1527,20 @@ private static class CombinedLibraryModels implements LibraryModels { polyNullLocationsBuilder.put(entry); } } + polyNullLocations = polyNullLocationsBuilder.build(); failIfNullParameters = failIfNullParametersBuilder.build(); - explicitlyNullableParameters = explicitlyNullableParametersBuilder.build(); - nonNullParameters = nonNullParametersBuilder.build(); + explicitlyNullableParameters = + removePolyNullParameterModels( + explicitlyNullableParametersBuilder.build(), polyNullLocations); + nonNullParameters = + removePolyNullParameterModels(nonNullParametersBuilder.build(), polyNullLocations); nullImpliesTrueParameters = nullImpliesTrueParametersBuilder.build(); nullImpliesFalseParameters = nullImpliesFalseParametersBuilder.build(); ensuresNonNullIfTrueMethodCalls = ensuresNonNullIfTrueMethodCallsBuilder.build(); nullImpliesNullParameters = nullImpliesNullParametersBuilder.build(); - nullableReturns = nullableReturnsBuilder.build(); - nonNullReturns = nonNullReturnsBuilder.build(); + nullableReturns = + removePolyNullReturnModels(nullableReturnsBuilder.build(), polyNullLocations); + nonNullReturns = removePolyNullReturnModels(nonNullReturnsBuilder.build(), polyNullLocations); castToNonNullMethods = castToNonNullMethodsBuilder.build(); customStreamNullabilitySpecs = customStreamNullabilitySpecsBuilder.build(); nullableFields = nullableFieldsBuilder.build(); @@ -1547,10 +1552,50 @@ private static class CombinedLibraryModels implements LibraryModels { nestedAnnotationsForMethodsBuilder = new ImmutableMap.Builder<>(); for (Map.Entry> entry : nestedAnnotationsBuilder.entrySet()) { - nestedAnnotationsForMethodsBuilder.put(entry.getKey(), entry.getValue().build()); + ImmutableSetMultimap annotations = entry.getValue().build(); + ImmutableSetMultimap.Builder filteredAnnotations = + ImmutableSetMultimap.builder(); + for (Map.Entry annotation : annotations.entries()) { + if (!polyNullLocations.containsEntry( + entry.getKey(), + new PolyNullLocation(annotation.getKey(), annotation.getValue().typePath()))) { + filteredAnnotations.put(annotation); + } + } + ImmutableSetMultimap filtered = filteredAnnotations.build(); + if (!filtered.isEmpty()) { + nestedAnnotationsForMethodsBuilder.put(entry.getKey(), filtered); + } } nestedAnnotationsForMethods = nestedAnnotationsForMethodsBuilder.build(); - polyNullLocations = polyNullLocationsBuilder.build(); + } + + /** Removes fixed top-level parameter models overridden by PolyNull locations. */ + private static ImmutableSetMultimap removePolyNullParameterModels( + ImmutableSetMultimap fixedModels, + ImmutableSetMultimap polyNullLocations) { + ImmutableSetMultimap.Builder result = ImmutableSetMultimap.builder(); + for (Map.Entry entry : fixedModels.entries()) { + if (!polyNullLocations.containsEntry( + entry.getKey(), new PolyNullLocation(entry.getValue(), ImmutableList.of()))) { + result.put(entry); + } + } + return result.build(); + } + + /** Removes fixed top-level return models overridden by PolyNull locations. */ + private static ImmutableSet removePolyNullReturnModels( + ImmutableSet fixedModels, + ImmutableSetMultimap polyNullLocations) { + ImmutableSet.Builder result = ImmutableSet.builder(); + PolyNullLocation returnLocation = new PolyNullLocation(-1, ImmutableList.of()); + for (MethodRef method : fixedModels) { + if (!polyNullLocations.containsEntry(method, returnLocation)) { + result.add(method); + } + } + return result.build(); } private boolean shouldSkipModel(MethodRef key) {