diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index 57caffa1f9..db9134e809 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -8,6 +8,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.uber.nullaway.Config; +import com.uber.nullaway.handlers.Handler; import java.util.List; import javax.lang.model.type.NullType; import javax.lang.model.type.TypeKind; @@ -21,12 +22,14 @@ public class CheckIdenticalNullabilityVisitor extends Types.DefaultTypeVisitor extendsBoundContains( - GenericsUtils.wildcardUpperBound(lhsWildcard, state), rhsTypeArgument); + GenericsUtils.wildcardUpperBound(lhsWildcard, state, config, handler), + rhsTypeArgument); case SUPER -> superWildcardContains(lhsWildcard, rhsTypeArgument); }; } @@ -168,7 +172,7 @@ private boolean wildcardContains(Type.WildcardType lhsWildcard, Type rhsTypeArgu private boolean extendsBoundContains(Type lhsBound, Type rhsTypeArgument) { Type.WildcardType rhsWildcard = GenericsUtils.asWildcard(rhsTypeArgument); if (rhsWildcard != null) { - Type rhsUpperBound = GenericsUtils.wildcardUpperBound(rhsWildcard, state); + Type rhsUpperBound = GenericsUtils.wildcardUpperBound(rhsWildcard, state, config, handler); return typeArgumentSubtype(lhsBound, rhsUpperBound); } return typeArgumentSubtype(lhsBound, rhsTypeArgument); 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 ff3e1269c8..f1bb5c382a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java @@ -4,7 +4,6 @@ import com.google.common.base.Verify; import com.google.errorprone.VisitorState; -import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; @@ -12,7 +11,6 @@ import com.sun.tools.javac.code.Type.TypeVar; import com.sun.tools.javac.code.Type.WildcardType; import com.sun.tools.javac.code.Types; -import com.uber.nullaway.CodeAnnotationInfo; import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; @@ -34,13 +32,11 @@ */ public final class ConstraintSolverImpl implements ConstraintSolver { private final Config config; - private final CodeAnnotationInfo codeAnnotationInfo; private final Handler handler; private final VisitorState state; public ConstraintSolverImpl(Config config, VisitorState state, NullAway analysis) { this.config = config; - this.codeAnnotationInfo = CodeAnnotationInfo.instance(state.context); this.handler = analysis.getHandler(); this.state = state; } @@ -210,9 +206,10 @@ private void equateTypeArguments(Type subtypeTypeArg, Type supertypeTypeArg) { private void constrainContainedByWildcard(Type subtypeTypeArg, WildcardType supertypeWildcard) { switch (supertypeWildcard.kind) { case UNBOUND, EXTENDS -> { - Type subtypeUpperBound = GenericsUtils.effectiveWildcardUpperBound(subtypeTypeArg, state); + Type subtypeUpperBound = + GenericsUtils.effectiveWildcardUpperBound(subtypeTypeArg, state, config, handler); subtypeUpperBound.accept( - this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state)); + this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state, config, handler)); } case SUPER -> { Type supertypeLowerBound = castToNonNull(supertypeWildcard.getSuperBound()); @@ -238,7 +235,8 @@ private void constrainContainedByWildcard(Type subtypeTypeArg, WildcardType supe */ private void constrainSubtypeToWildcard(Type subtype, WildcardType supertypeWildcard) { if (supertypeWildcard.kind != BoundKind.SUPER) { - subtype.accept(this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state)); + subtype.accept( + this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state, config, handler)); } } @@ -251,7 +249,8 @@ private void constrainWildcardToSupertype(WildcardType subtypeWildcard, Type sup if (subtypeWildcard.kind == BoundKind.SUPER) { castToNonNull(subtypeWildcard.getSuperBound()).accept(this, supertype); } else { - GenericsUtils.wildcardUpperBound(subtypeWildcard, state).accept(this, supertype); + GenericsUtils.wildcardUpperBound(subtypeWildcard, state, config, handler) + .accept(this, supertype); } } } @@ -391,7 +390,9 @@ private void constrainAsNonNull(Type t, Type knownNonNullType) /* ───────────────────── helpers & stubs ───────────────────── */ private VarState getState(Element typeVarElement) { - return vars.computeIfAbsent(typeVarElement, v -> new VarState(upperBoundIsNullable(v))); + return vars.computeIfAbsent( + typeVarElement, + v -> new VarState(GenericsUtils.upperBoundIsNullable(v, config, handler, state))); } private boolean treatAsTypeVariableForInference(Type t) { @@ -414,44 +415,4 @@ private boolean isKnownNullable(Type t) { private boolean isKnownNonNull(Type t) { return !isKnownNullable(t) && !treatAsTypeVariableForInference(t); } - - private boolean upperBoundIsNullable(Element typeVarElement) { - if (fromUnannotatedMethod(typeVarElement)) { - return true; - } - // first, check if library model overrides the upper bound nullability - Element enclosingElement = typeVarElement.getEnclosingElement(); - if (enclosingElement instanceof Symbol.MethodSymbol methodSymbol - && typeVarElement instanceof Symbol.TypeVariableSymbol typeVariableSymbol) { - int typeVarIndex = methodSymbol.getTypeParameters().indexOf(typeVariableSymbol); - // TODO typeVarIndex is -1 in some cases; see test - // com.uber.nullaway.jspecify.GenericMethodTests.instanceGenericMethodWithMethodRefArgument. - // Investigate further. - if (typeVarIndex >= 0 - && handler.onOverrideMethodTypeVariableUpperBound(methodSymbol, typeVarIndex, state)) { - return true; - } - } else if (enclosingElement instanceof Symbol.ClassSymbol classSymbol - && typeVarElement instanceof Symbol.TypeVariableSymbol typeVariableSymbol) { - int typeVarIndex = classSymbol.getTypeParameters().indexOf(typeVariableSymbol); - if (typeVarIndex >= 0 - && handler.onOverrideClassTypeVariableUpperBound(classSymbol.toString(), typeVarIndex)) { - return true; - } - } - // otherwise, check the actual upper bound annotations - Type upperBound = (Type) ((TypeVariable) typeVarElement.asType()).getUpperBound(); - com.sun.tools.javac.util.List annotationMirrors = - upperBound.getAnnotationMirrors(); - return com.uber.nullaway.Nullness.hasNullableAnnotation(annotationMirrors.stream(), config); - } - - private boolean fromUnannotatedMethod(Element typeVarElement) { - Element enclosingElement = typeVarElement.getEnclosingElement(); - if (!(enclosingElement instanceof Symbol.MethodSymbol) - && !(enclosingElement instanceof Symbol.ClassSymbol)) { - return false; - } - return codeAnnotationInfo.isSymbolUnannotated((Symbol) enclosingElement, config, handler); - } } 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 e5be0d4885..de818c77f7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1082,7 +1082,7 @@ private MethodInferenceResult runInferenceForCall( Type polyExprTreeType = ASTHelpers.getType(argument); if (polyExprTreeType != null) { Type formalParamGroundTargetType = - GenericsUtils.groundTargetType(formalParamType, state, config); + GenericsUtils.groundTargetType(formalParamType, state, config, handler); Type typeWithInferredNullability = TypeSubstitutionUtils.updateTypeWithInferredNullability( polyExprTreeType, @@ -1261,7 +1261,7 @@ private void handleLambdaInGenericMethodInference( Symbol.MethodSymbol fiMethod = NullabilityUtil.getFunctionalInterfaceMethod(lambda, state.getTypes()); - Type groundTargetType = GenericsUtils.groundTargetType(lhsType, state, config); + Type groundTargetType = GenericsUtils.groundTargetType(lhsType, state, config, handler); // get the return type of the functional interface method, viewed as a member of the lhs // type, so the generic method's type variables are substituted in Type.MethodType fiMethodTypeAsMember = @@ -1315,7 +1315,7 @@ private void handleMethodRefInGenericMethodInference( ConstraintSolver solver, Type lhsType, MemberReferenceTree memberReferenceTree) { - Type groundTargetType = GenericsUtils.groundTargetType(lhsType, state, config); + Type groundTargetType = GenericsUtils.groundTargetType(lhsType, state, config, handler); GenericsUtils.processMethodRefTypeRelations( this, groundTargetType, @@ -1645,7 +1645,8 @@ public void checkTypeParameterNullnessForFunctionReturnType( */ private boolean identicalTypeParameterNullability( Type lhsType, Type rhsType, VisitorState state) { - return lhsType.accept(new CheckIdenticalNullabilityVisitor(state, this, config), rhsType); + return lhsType.accept( + new CheckIdenticalNullabilityVisitor(state, this, config, handler), rhsType); } /** @@ -1808,7 +1809,7 @@ public void compareGenericTypeParameterNullabilityForCall( if (currentActualParam instanceof MemberReferenceTree memberReferenceTree) { Type groundFormalParameter = - GenericsUtils.groundTargetType(formalParameter, state, config); + GenericsUtils.groundTargetType(formalParameter, state, config, handler); // the type of the method reference tree provided by javac may not capture // nullability of nested types. So, do explicit type checks based on the return and // parameter types of the referenced method @@ -2613,7 +2614,8 @@ private Nullness getReturnTypeNullness( return Nullness.NULLABLE; } if (config.handleWildcardGenerics() && GenericsUtils.asWildcard(type) != null) { - Type effectiveUpperBound = GenericsUtils.effectiveWildcardUpperBound(type, state); + Type effectiveUpperBound = + GenericsUtils.effectiveWildcardUpperBound(type, state, config, handler); return getReturnTypeNullness(effectiveUpperBound, state, true); } if (followTypeVarUpperBound && type instanceof Type.TypeVar typeVar) { @@ -2702,7 +2704,7 @@ private void maybeStorePolyExpressionTypeFromTarget( if (polyExpressionType == null) { return; } - Type groundTargetType = GenericsUtils.groundTargetType(targetType, state, config); + Type groundTargetType = GenericsUtils.groundTargetType(targetType, state, config, handler); Type polyExpressionTypeWithTargetAnnotations = TypeSubstitutionUtils.restoreExplicitNullabilityAnnotations( groundTargetType, polyExpressionType, config, Collections.emptyMap()); diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java index a10e6a3498..627da3e51b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java @@ -19,9 +19,14 @@ import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.util.List; import com.sun.tools.javac.util.ListBuffer; +import com.uber.nullaway.CodeAnnotationInfo; import com.uber.nullaway.Config; import com.uber.nullaway.NullabilityUtil; +import com.uber.nullaway.Nullness; +import com.uber.nullaway.handlers.Handler; +import javax.lang.model.element.Element; import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeVariable; import org.jspecify.annotations.Nullable; /** Utility methods for doing generics-related checking */ @@ -40,16 +45,20 @@ enum MethodRefTypeRelationKind { * type itself. For wildcards and captured wildcards, returns the wildcard's upper bound, * recursing through nested wildcards and captures produced by javac. */ - static Type effectiveWildcardUpperBound(Type typeArg, VisitorState state) { + static Type effectiveWildcardUpperBound( + Type typeArg, VisitorState state, Config config, Handler handler) { WildcardType wildcardType = asWildcard(typeArg); - return wildcardType == null ? typeArg : wildcardUpperBound(wildcardType, state); + return wildcardType == null + ? typeArg + : wildcardUpperBound(wildcardType, state, config, handler); } /** * Returns the effective upper bound of a wildcard, using the corresponding type variable's upper * bound for unbounded wildcards and {@code super} wildcards. */ - static Type wildcardUpperBound(WildcardType wildcardType, VisitorState state) { + static Type wildcardUpperBound( + WildcardType wildcardType, VisitorState state, Config config, Handler handler) { Type upperBound; if (wildcardType.kind == BoundKind.EXTENDS) { upperBound = wildcardType.getExtendsBound(); @@ -64,16 +73,72 @@ static Type wildcardUpperBound(WildcardType wildcardType, VisitorState state) { formalTypeVar == null ? Symtab.instance(state.context).objectType : formalTypeVar.getUpperBound(); + // check if the upper bound should be treated as @Nullable, e.g., due to a library model or a + // type variable in @NullUnmarked code + if (formalTypeVar != null + && upperBoundIsNullable(formalTypeVar.asElement(), config, handler, state) + && !Nullness.hasNullableAnnotation(upperBound.getAnnotationMirrors().stream(), config)) { + upperBound = + TypeSubstitutionUtils.typeWithAnnot( + upperBound, GenericsChecks.getSyntheticNullableAnnotType(state)); + } } if (upperBound instanceof WildcardType nestedWildcard) { - return wildcardUpperBound(nestedWildcard, state); + return wildcardUpperBound(nestedWildcard, state, config, handler); } if (upperBound instanceof CapturedType capturedType && capturedType.wildcard != null) { - return wildcardUpperBound(capturedType.wildcard, state); + return wildcardUpperBound(capturedType.wildcard, state, config, handler); } return upperBound; } + /** + * Returns true if the upper bound of the given type variable should be treated as nullable. + * + *

A bound is nullable when the enclosing method or class comes from unannotated code, when a + * library model overrides the bound nullability for the type variable, or when the declared upper + * bound has an explicit {@code @Nullable} annotation. + */ + static boolean upperBoundIsNullable( + Element typeVarElement, Config config, Handler handler, VisitorState state) { + if (fromUnannotatedMethodOrClass(typeVarElement, config, handler, state)) { + return true; + } + // First, check if library model overrides the upper bound nullability. + Element enclosingElement = typeVarElement.getEnclosingElement(); + if (enclosingElement instanceof Symbol.MethodSymbol methodSymbol + && typeVarElement instanceof Symbol.TypeVariableSymbol typeVariableSymbol) { + int typeVarIndex = methodSymbol.getTypeParameters().indexOf(typeVariableSymbol); + // TODO typeVarIndex is -1 in some cases; see test + // com.uber.nullaway.jspecify.GenericMethodTests.instanceGenericMethodWithMethodRefArgument. + // Investigate further. + if (typeVarIndex >= 0 + && handler.onOverrideMethodTypeVariableUpperBound(methodSymbol, typeVarIndex, state)) { + return true; + } + } else if (enclosingElement instanceof Symbol.ClassSymbol classSymbol + && typeVarElement instanceof Symbol.TypeVariableSymbol typeVariableSymbol) { + int typeVarIndex = classSymbol.getTypeParameters().indexOf(typeVariableSymbol); + if (typeVarIndex >= 0 + && handler.onOverrideClassTypeVariableUpperBound(classSymbol.toString(), typeVarIndex)) { + return true; + } + } + Type upperBound = (Type) ((TypeVariable) typeVarElement.asType()).getUpperBound(); + return Nullness.hasNullableAnnotation(upperBound.getAnnotationMirrors().stream(), config); + } + + private static boolean fromUnannotatedMethodOrClass( + Element typeVarElement, Config config, Handler handler, VisitorState state) { + Element enclosingElement = typeVarElement.getEnclosingElement(); + if (!(enclosingElement instanceof Symbol.MethodSymbol) + && !(enclosingElement instanceof Symbol.ClassSymbol)) { + return false; + } + return CodeAnnotationInfo.instance(state.context) + .isSymbolUnannotated((Symbol) enclosingElement, config, handler); + } + static @Nullable WildcardType asWildcard(Type typeArg) { if (typeArg instanceof WildcardType wildcardType) { return wildcardType; @@ -98,7 +163,8 @@ static Type wildcardUpperBound(WildcardType wildcardType, VisitorState state) { * href="https://docs.oracle.com/javase/specs/jls/se21/html/jls-9.html#jls-9.9">9.9. */ @SuppressWarnings("ReferenceEquality") - static Type groundTargetType(Type targetType, VisitorState state, Config config) { + static Type groundTargetType( + Type targetType, VisitorState state, Config config, Handler handler) { if (!config.handleWildcardGenerics()) { return targetType; } @@ -112,7 +178,7 @@ static Type groundTargetType(Type targetType, VisitorState state, Config config) ListBuffer groundedTypeArguments = new ListBuffer<>(); boolean changed = false; for (Type typeArgument : typeArguments) { - Type groundedTypeArgument = groundTypeArgument(typeArgument, state); + Type groundedTypeArgument = groundTypeArgument(typeArgument, state, config, handler); groundedTypeArguments.append(groundedTypeArgument); changed |= groundedTypeArgument != typeArgument; } @@ -127,7 +193,8 @@ static Type groundTargetType(Type targetType, VisitorState state, Config config) * rules for functional interface target types in JLS 9.9. */ - private static Type groundTypeArgument(Type typeArgument, VisitorState state) { + private static Type groundTypeArgument( + Type typeArgument, VisitorState state, Config config, Handler handler) { WildcardType wildcardType = asWildcard(typeArgument); if (wildcardType == null) { return typeArgument; @@ -135,7 +202,7 @@ private static Type groundTypeArgument(Type typeArgument, VisitorState state) { if (wildcardType.kind == BoundKind.SUPER) { return castToNonNull(wildcardType.getSuperBound()); } - return wildcardUpperBound(wildcardType, state); + return wildcardUpperBound(wildcardType, state, config, handler); } /** diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java index 6f2b24978e..deccdba575 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java @@ -880,6 +880,29 @@ public Foo or(Foo other) { .doTest(); } + @Test + public void unboundWildcardTypeVarUnmarked() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.NullUnmarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + @NullUnmarked + interface Foo {} + Foo test(Foo<@Nullable Void> foo) { + // legal since Foo is @NullUnmarked, so its V type variable + // is treated as having a @Nullable upper bound + return foo; + } + } + """) + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs(