Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ private boolean typeArgumentContainedBy(Type lhsTypeArgument, Type rhsTypeArgume
private boolean wildcardContains(Type.WildcardType lhsWildcard, Type rhsTypeArgument) {
return switch (lhsWildcard.kind) {
case UNBOUND, EXTENDS ->
extendsBoundContains(wildcardUpperBound(lhsWildcard), rhsTypeArgument);
extendsBoundContains(
GenericsUtils.wildcardUpperBound(lhsWildcard, state), rhsTypeArgument);
case SUPER -> superWildcardContains(lhsWildcard, rhsTypeArgument);
};
}
Expand All @@ -165,26 +166,14 @@ private boolean wildcardContains(Type.WildcardType lhsWildcard, Type rhsTypeArgu
* actuals whose effective upper bound is {@code T}, containment holds when {@code T <: S}.
*/
private boolean extendsBoundContains(Type lhsBound, Type rhsTypeArgument) {
if (rhsTypeArgument instanceof Type.WildcardType rhsWildcard) {
Type rhsUpperBound = wildcardUpperBound(rhsWildcard);
Type.WildcardType rhsWildcard = GenericsUtils.asWildcard(rhsTypeArgument);
if (rhsWildcard != null) {
Type rhsUpperBound = GenericsUtils.wildcardUpperBound(rhsWildcard, state);
return typeArgumentSubtype(lhsBound, rhsUpperBound);
}
return typeArgumentSubtype(lhsBound, rhsTypeArgument);
}

/**
* Returns the effective upper bound of a wildcard, using the corresponding type variable's upper
* bound for unbounded wildcards and {@code super} wildcards.
*/
private Type wildcardUpperBound(Type.WildcardType wildcardType) {
if (wildcardType.kind == BoundKind.EXTENDS) {
return wildcardType.getExtendsBound();
}
// For ? and ? super L, javac stores the wildcard's corresponding type variable in the `bound`
// field. The upper bound of that type variable is the wildcard's effective upper bound.
return wildcardType.bound.getUpperBound();
}

/**
* Returns whether a formal {@code ? super S} contains the actual type argument on the right. For
* concrete actuals {@code T} and wildcard actuals {@code ? super T}, containment holds when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
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;
import com.sun.tools.javac.code.Type.ClassType;
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;
Expand Down Expand Up @@ -90,7 +92,15 @@ class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Vo

@Override
public @Nullable Void visitType(Type subtype, Type supertype) {
// handle flow into a type variable. the check for !(subtype instanceof TypeVar) is a
if (config.handleWildcardGenerics()) {
WildcardType supertypeWildcard = GenericsUtils.asWildcard(supertype);
if (supertypeWildcard != null) {
Verify.verify(!localVariableType, "A local variable should not have a wildcard type");
constrainSubtypeToWildcard(subtype, supertypeWildcard);
return null;
}
}
// handle flow into a type variable. The check for !(subtype instanceof TypeVar) is a
// small optimization, as that case should be handled in visitTypeVar.
if (!localVariableType && (supertype instanceof TypeVar) && !(subtype instanceof TypeVar)) {
directlyConstrainTypePair(subtype, supertype);
Expand All @@ -116,17 +126,13 @@ class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Vo
int numTypeArgs = supertypeTypeArguments.size();
Verify.verify(numTypeArgs == subtypeTypeArguments.size());
for (int i = 0; i < numTypeArgs; i++) {
Type rhsTypeArg = supertypeTypeArguments.get(i);
Type lhsTypeArg = subtypeTypeArguments.get(i);
// constrain in both directions
// TODO should we have a more optimized way to equate two types? this just makes each
// type a subtype of the other
lhsTypeArg.accept(this, rhsTypeArg);
rhsTypeArg.accept(this, lhsTypeArg);
Type supertypeTypeArg = supertypeTypeArguments.get(i);
Type subtypeTypeArg = subtypeTypeArguments.get(i);
constrainTypeArgumentContainment(subtypeTypeArg, supertypeTypeArg);
}
}
// if supertype is not a ClassType, we still call visitType to handle the case where
// supertype is a TypeVar
// supertype is a TypeVar or a wildcard
return visitType(subtype, supertype);
}

Expand All @@ -141,7 +147,7 @@ class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Vo
subtypeComponentType.accept(this, superComponentType);
}
// if supertype is not an ArrayType, we still call visitType to handle the case where
// supertype is a TypeVar
// supertype is a TypeVar or a wildcard
return visitType(subtype, supertype);
}

Expand All @@ -152,6 +158,102 @@ class AddSubtypeConstraintsVisitor extends Types.DefaultTypeVisitor<@Nullable Vo
}
return visitType(subtype, supertype);
}

@Override
public @Nullable Void visitWildcardType(WildcardType subtype, Type supertype) {
if (config.handleWildcardGenerics()) {
Verify.verify(!localVariableType, "A wildcard type cannot be assigned to a local variable");
constrainWildcardToSupertype(subtype, supertype);
}
return null;
}
Comment thread
msridhar marked this conversation as resolved.

/**
* Adds nullability constraints for containment of one type argument by another during generic
* class/interface subtyping. For non-wildcard arguments, NullAway requires identical
* nullability. When either side is a wildcard, containment is reduced to constraints between
* the wildcard bound and the opposing argument.
*/
private void constrainTypeArgumentContainment(Type subtypeTypeArg, Type supertypeTypeArg) {
if (!config.handleWildcardGenerics()) {
equateTypeArguments(subtypeTypeArg, supertypeTypeArg);
return;
}
WildcardType supertypeWildcard = GenericsUtils.asWildcard(supertypeTypeArg);
if (supertypeWildcard != null) {
constrainContainedByWildcard(subtypeTypeArg, supertypeWildcard);
return;
}
WildcardType subtypeWildcard = GenericsUtils.asWildcard(subtypeTypeArg);
if (subtypeWildcard != null) {
constrainWildcardToSupertype(subtypeWildcard, supertypeTypeArg);
return;
}
equateTypeArguments(subtypeTypeArg, supertypeTypeArg);
}

private void equateTypeArguments(Type subtypeTypeArg, Type supertypeTypeArg) {
// constrain in both directions
// TODO should we have a more optimized way to equate two types? this just makes each
// type a subtype of the other
subtypeTypeArg.accept(this, supertypeTypeArg);
supertypeTypeArg.accept(this, subtypeTypeArg);
}

/**
* Adds constraints for type-argument containment where the formal argument is a wildcard. For
* {@code ? extends S} and {@code ?}, containment requires the actual argument's effective upper
* bound to be a subtype of {@code S}. For {@code ? super S}, concrete actual arguments require
* {@code S <: subtypeTypeArg}; {@code ? super T} actual arguments require {@code S <: T}. Other
* actual wildcard forms place no useful nullability constraint.
*/
private void constrainContainedByWildcard(Type subtypeTypeArg, WildcardType supertypeWildcard) {
switch (supertypeWildcard.kind) {
case UNBOUND, EXTENDS -> {
Type subtypeUpperBound = GenericsUtils.effectiveWildcardUpperBound(subtypeTypeArg, state);
subtypeUpperBound.accept(
this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state));
}
case SUPER -> {
Type supertypeLowerBound = castToNonNull(supertypeWildcard.getSuperBound());
WildcardType subtypeWildcard = GenericsUtils.asWildcard(subtypeTypeArg);
if (subtypeWildcard != null) {
if (subtypeWildcard.kind == BoundKind.SUPER) {
supertypeLowerBound.accept(this, castToNonNull(subtypeWildcard.getSuperBound()));
}
// the subtype wildcard could have an extends bound, but as far as I know we do not
// need to generate constraints for this case
// TODO revisit if needed
} else {
supertypeLowerBound.accept(this, subtypeTypeArg);
}
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* Adds constraints for a top-level subtype relation {@code subtype <: supertypeWildcard}. For
* {@code ? extends S} and {@code ?}, this reduces to {@code subtype <: S}. A {@code ? super S}
* supertype places no useful nullability constraint on {@code subtype}.
*/
private void constrainSubtypeToWildcard(Type subtype, WildcardType supertypeWildcard) {
if (supertypeWildcard.kind != BoundKind.SUPER) {
subtype.accept(this, GenericsUtils.wildcardUpperBound(supertypeWildcard, state));
}
}

/**
* Adds constraints for a top-level subtype relation {@code subtypeWildcard <: supertype}. For
* {@code ? extends S} and {@code ?}, this reduces to {@code S <: supertype}. For {@code ? super
* S}, use the lower bound and reduce to {@code S <: supertype}.
*/
private void constrainWildcardToSupertype(WildcardType subtypeWildcard, Type supertype) {
if (subtypeWildcard.kind == BoundKind.SUPER) {
castToNonNull(subtypeWildcard.getSuperBound()).accept(this, supertype);
} else {
GenericsUtils.wildcardUpperBound(subtypeWildcard, state).accept(this, supertype);
}
}
Comment thread
msridhar marked this conversation as resolved.
}

@Override
Expand Down Expand Up @@ -272,10 +374,9 @@ private VarState getState(Element typeVarElement) {

private boolean isTypeVariable(Type t) {
if (t instanceof TypeVar tv) {
// For now ignore capture variables, like "capture#1 of ? extends X". Also, only treat as a
// type variable if it _doesn't_ have an explicit @Nullable or @NonNull annotation.
return !tv.isCaptured()
&& !Nullness.hasNullableAnnotation(tv.getAnnotationMirrors().stream(), config)
// Only treat as a type variable if it _doesn't_ have an explicit @Nullable or @NonNull
// annotation.
return !Nullness.hasNullableAnnotation(tv.getAnnotationMirrors().stream(), config)
&& !Nullness.hasNonNullAnnotation(tv.getAnnotationMirrors().stream(), config);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
return false;
Expand All @@ -298,19 +399,19 @@ private boolean upperBoundIsNullable(Element typeVarElement) {
}
// first, check if library model overrides the upper bound nullability
Element enclosingElement = typeVarElement.getEnclosingElement();
if (enclosingElement instanceof Symbol.MethodSymbol methodSymbol) {
int typeVarIndex =
methodSymbol.getTypeParameters().indexOf((Symbol.TypeVariableSymbol) typeVarElement);
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) {
int typeVarIndex =
classSymbol.getTypeParameters().indexOf((Symbol.TypeVariableSymbol) typeVarElement);
} 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;
Expand All @@ -324,8 +425,11 @@ private boolean upperBoundIsNullable(Element typeVarElement) {
}

private boolean fromUnannotatedMethod(Element typeVarElement) {
Symbol enclosingElement = (Symbol) typeVarElement.getEnclosingElement();
return enclosingElement != null
&& codeAnnotationInfo.isSymbolUnannotated(enclosingElement, config, handler);
Element enclosingElement = typeVarElement.getEnclosingElement();
if (!(enclosingElement instanceof Symbol.MethodSymbol)
&& !(enclosingElement instanceof Symbol.ClassSymbol)) {
return false;
}
return codeAnnotationInfo.isSymbolUnannotated((Symbol) enclosingElement, config, handler);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.tools.javac.code.BoundKind;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.CapturedType;
import com.sun.tools.javac.code.Type.WildcardType;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
import com.uber.nullaway.NullabilityUtil;
import javax.lang.model.type.TypeKind;
import org.jspecify.annotations.Nullable;

/** Utility methods for doing generics-related checking */
public class GenericsUtils {
Expand All @@ -22,6 +27,55 @@ enum MethodRefTypeRelationKind {
PARAMETER
}

/**
* Returns the effective upper bound of {@code typeArg}. For concrete type arguments, returns the
* 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) {
WildcardType wildcardType = asWildcard(typeArg);
return wildcardType == null ? typeArg : wildcardUpperBound(wildcardType, state);
}

/**
* 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) {
Type upperBound;
if (wildcardType.kind == BoundKind.EXTENDS) {
upperBound = wildcardType.getExtendsBound();
} else {
// We have an unbound wildcard or a wildcard with just a lower bound. In such cases, if
// present, we use the upper bound of the formal type variable to which the wildcard is being
// passed (confusingly stored in the `bound` field). E.g., if we have class Foo<T extends
// @Nullable Object>, and then see Foo<? super String>, we use @Nullable Object as the upper
// bound. If not present, default to Object.
Type.TypeVar formalTypeVar = wildcardType.bound;
upperBound =
formalTypeVar == null
? Symtab.instance(state.context).objectType
: formalTypeVar.getUpperBound();
}
if (upperBound instanceof WildcardType nestedWildcard) {
return wildcardUpperBound(nestedWildcard, state);
}
if (upperBound instanceof CapturedType capturedType && capturedType.wildcard != null) {
return wildcardUpperBound(capturedType.wildcard, state);
}
return upperBound;
}

static @Nullable WildcardType asWildcard(Type typeArg) {
if (typeArg instanceof WildcardType wildcardType) {
return wildcardType;
}
if (typeArg instanceof CapturedType capturedType) {
return capturedType.wildcard;
}
return null;
}

/**
* Handler for method reference type relations, used by {{@link
* #processMethodRefTypeRelations(GenericsChecks, Type, MemberReferenceTree, VisitorState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,15 @@ public Type visitMethodType(Type.MethodType t, Type other) {

@Override
public Type visitClassType(Type.ClassType t, Type other) {
if (other instanceof Type.WildcardType wt && wt.kind == BoundKind.EXTENDS) {
// As a temporary measure, we restore nullability annotations from the upper bound of the
// wildcard.
// TODO revisit this decision when we add fuller support for inference and wildcards.
return visit(t, wt.getExtendsBound());
if (other instanceof Type.WildcardType wt) {
// When the other type is a wildcard, restore nullability annotations from the bound that
// determines the functional interface type.
if (wt.kind == BoundKind.EXTENDS) {
return visit(t, wt.getExtendsBound());
} else if (wt.kind == BoundKind.SUPER) {
return visit(t, wt.getSuperBound());
}
}
Comment on lines +296 to 304
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are test cases covering UNBOUND wildcards flowing through this visitor
rg -n --type=java 'visitClassType.*Wildcard|UNBOUND' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java
rg -n --type=java '\<\?\>' nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java | head -20

Repository: uber/NullAway

Length of output: 130


🏁 Script executed:

#!/bin/bash
# Read the TypeSubstitutionUtils.java file to see the full context around lines 296-304
wc -l nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Read the relevant section of TypeSubstitutionUtils.java
sed -n '290,315p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 1129


🏁 Script executed:

#!/bin/bash
# Find GenericsUtils.java and check the wildcardUpperBound method
fd -n GenericsUtils.java nullaway/src/main/java

Repository: uber/NullAway

Length of output: 288


🏁 Script executed:

#!/bin/bash
# Search for UNBOUND keyword in the codebase
rg -n "BoundKind.UNBOUND" nullaway/src/main/java/

Repository: uber/NullAway

Length of output: 187


🏁 Script executed:

#!/bin/bash
# Search for test cases related to wildcards
fd -n "WildcardTests.java" nullaway/src/test/java

Repository: uber/NullAway

Length of output: 288


🏁 Script executed:

#!/bin/bash
# Find GenericsUtils.java
find nullaway/src/main/java -name "GenericsUtils.java"

Repository: uber/NullAway

Length of output: 126


🏁 Script executed:

#!/bin/bash
# Find WildcardTests.java
find nullaway/src/test/java -name "WildcardTests.java"

Repository: uber/NullAway

Length of output: 126


🏁 Script executed:

#!/bin/bash
# Search for wildcardUpperBound method
rg -n "wildcardUpperBound" nullaway/src/main/java/

Repository: uber/NullAway

Length of output: 1483


🏁 Script executed:

#!/bin/bash
# Search for how UNBOUND wildcards are handled in generics code
rg -n "BoundKind\|UNBOUND" nullaway/src/main/java/com/uber/nullaway/generics/ -A 2 -B 2

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Read the GenericsUtils.java file to understand wildcardUpperBound
cat -n nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java | head -80

Repository: uber/NullAway

Length of output: 3711


🏁 Script executed:

#!/bin/bash
# Search for UNBOUND handling patterns
rg -n "UNBOUND" nullaway/src/main/java/com/uber/nullaway/generics/

Repository: uber/NullAway

Length of output: 425


🏁 Script executed:

#!/bin/bash
# Check WildcardTests.java for test coverage of UNBOUND wildcards
cat -n nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java | head -100

Repository: uber/NullAway

Length of output: 4631


🏁 Script executed:

#!/bin/bash
# Search for similar wildcard handling patterns in other visitors
rg -n "instanceof.*WildcardType" nullaway/src/main/java/com/uber/nullaway/generics/ -A 5 -B 1

Repository: uber/NullAway

Length of output: 5547


🏁 Script executed:

#!/bin/bash
# Check if visitClassType is called with UNBOUND wildcards in practice
rg -n "visitClassType" nullaway/src/main/java/com/uber/nullaway/generics/ -B 2 -A 2

Repository: uber/NullAway

Length of output: 3135


🏁 Script executed:

#!/bin/bash
# Read more of TypeSubstitutionUtils.java to understand the context and purpose
sed -n '260,320p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 2698


🏁 Script executed:

#!/bin/bash
# Search for test cases with unbounded wildcards (just "?")
rg -n '\?\>' nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

Repository: uber/NullAway

Length of output: 130


🏁 Script executed:

#!/bin/bash
# Check if there are any tests with super wildcards
rg -n '\? super' nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

Repository: uber/NullAway

Length of output: 2881


🏁 Script executed:

#!/bin/bash
# Search for any existing UNBOUND handling or comments about it
rg -n "UNBOUND" nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check if bound field can be null for WildcardType
rg -n "wildcardType.bound|wt.bound" nullaway/src/main/java/com/uber/nullaway/generics/ -B 2 -A 2

Repository: uber/NullAway

Length of output: 674


🏁 Script executed:

#!/bin/bash
# Look at the visitWildcardType method in TypeSubstitutionUtils to see how it handles wildcards
sed -n '321,345p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 830


🏁 Script executed:

#!/bin/bash
# Check if there are any comments or documentation about why UNBOUND is not handled
sed -n '290,310p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java | cat -n

Repository: uber/NullAway

Length of output: 1046


🏁 Script executed:

#!/bin/bash
# Verify if wt.bound can be null and check the null handling in GenericsUtils pattern
rg -n "bound == null|bound != null" nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check if there are any other visitors that handle UNBOUND explicitly
rg -n "BoundKind.UNBOUND" nullaway/src/main/java/com/uber/nullaway/generics/ -B 2 -A 2

Repository: uber/NullAway

Length of output: 668


🏁 Script executed:

#!/bin/bash
# Check if there are tests for unbounded wildcards by looking for patterns like Foo<?>
rg -n 'Foo<.*>' nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java | grep -v extends | grep -v super | head -20

Repository: uber/NullAway

Length of output: 1458


🏁 Script executed:

#!/bin/bash
# Check if the code actually processes UNBOUND wildcards and what happens
rg -n "RestoreNullnessAnnotationsVisitor\|visitClassType" nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java -B 5 -A 15 | head -60

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Verify what updateDirectNullabilityAnnotationsForType does with WildcardType
rg -n "updateDirectNullabilityAnnotationsForType" nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java -A 5 | head -40

Repository: uber/NullAway

Length of output: 1206


🏁 Script executed:

#!/bin/bash
# Read the updateDirectNullabilityAnnotationsForType method fully
sed -n '362,390p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 1261


🏁 Script executed:

#!/bin/bash
# Check the git history or comments around this code
git log --oneline -20 -- nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 89


🏁 Script executed:

#!/bin/bash
# Look for any test that specifically checks UNBOUND wildcard behavior
rg -n "unboundedFoo\|Foo<\?>" nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java -B 3 -A 8

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check what the actual test expectations are for unbounded wildcards
sed -n '270,320p' nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

Repository: uber/NullAway

Length of output: 2626


🏁 Script executed:

#!/bin/bash
# Look at the context around line 275 test with unbounded foo
rg -n "unboundedFoo" nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java -B 5 -A 15

Repository: uber/NullAway

Length of output: 1671


🏁 Script executed:

#!/bin/bash
# Check if bound field is populated for UNBOUND wildcards
rg -n "\.bound\s*=" nullaway/src/main/java/com/uber/nullaway/generics/ -B 2 -A 2

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check the commit context for the RestoreNullnessAnnotationsVisitor code
git show 272c447 --stat

Repository: uber/NullAway

Length of output: 27203


🏁 Script executed:

#!/bin/bash
# Search for usages of RestoreNullnessAnnotationsVisitor to understand when it's invoked
rg -n "RestoreNullnessAnnotationsVisitor\|new RestoreNullnessAnnotationsVisitor" nullaway/src/main/java/ -B 2 -A 2

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check if there's a way for UNBOUND to be processed through the formal type parameter bound
rg -n "wt.bound\|wildcardType.bound" nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Look at the broader context of how RestoreNullnessAnnotationsVisitor is used
rg -n "new RestoreNullnessAnnotationsVisitor\|RestoreNullnessAnnotationsVisitor(" nullaway/src/main/java/com/uber/nullaway/generics/ -B 3 -A 3

Repository: uber/NullAway

Length of output: 254


🏁 Script executed:

#!/bin/bash
# Look at the broader context of how RestoreNullnessAnnotationsVisitor is used
rg -n "RestoreNullnessAnnotationsVisitor" nullaway/src/main/java/com/uber/nullaway/generics/ -B 3 -A 3

Repository: uber/NullAway

Length of output: 2508


🏁 Script executed:

#!/bin/bash
# Check what happens in the UNBOUND case - does it fall through intentionally or by omission?
# Look for any comments or documentation about wildcard handling
sed -n '290,340p' nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

Repository: uber/NullAway

Length of output: 1782


Handle UNBOUND wildcards by extracting the formal type parameter's upper bound.

The code explicitly extracts bounds for BoundKind.EXTENDS and BoundKind.SUPER, but BoundKind.UNBOUND (i.e., ?) falls through without extraction. This is inconsistent with GenericsUtils.wildcardUpperBound, which uses wt.bound.getUpperBound() for both SUPER and UNBOUND wildcards to access the formal type parameter's upper bound.

For UNBOUND wildcards, consider handling the case explicitly:

Suggested approach
      if (other instanceof Type.WildcardType wt) {
        // When the other type is a wildcard, restore nullability annotations from the bound that
        // determines the functional interface type.
        if (wt.kind == BoundKind.EXTENDS) {
          return visit(t, wt.getExtendsBound());
        } else if (wt.kind == BoundKind.SUPER) {
          return visit(t, wt.getSuperBound());
+       } else if (wt.kind == BoundKind.UNBOUND && wt.bound != null) {
+         return visit(t, wt.bound.getUpperBound());
+       }
      }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java`
around lines 296 - 304, The current handling of Type.WildcardType in
TypeSubstitutionUtils ignores BoundKind.UNBOUND (plain '?'); update the wildcard
branch to explicitly handle BoundKind.UNBOUND by extracting the formal type
parameter's upper bound (as GenericsUtils.wildcardUpperBound(wt) or
wt.bound.getUpperBound()) and then call visit(t, thatUpperBound) just like the
EXTENDS/SUPER cases; modify the code around the Type.WildcardType branch to add
an else-if for wt.kind == BoundKind.UNBOUND that delegates to visit(t,
<upperBound>) so UNBOUND wildcards restore nullability annotations consistently.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There cannot be explicit nullability annotations for the unbound case

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msridhar, thanks for the clarification! That makes sense — an unbound wildcard ? carries no explicit nullability annotation, so there's nothing to restore for that case, and the fall-through to updateDirectNullabilityAnnotationsForType is intentionally correct. I was wrong to suggest handling BoundKind.UNBOUND there.


✏️ Learnings added
Learnt from: msridhar
Repo: uber/NullAway PR: 1549
File: nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java:296-304
Timestamp: 2026-05-03T17:32:46.673Z
Learning: In uber/NullAway, UNBOUND wildcards (`?`) cannot carry explicit nullability annotations (you cannot write `? Nullable`). Therefore, in `RestoreNullnessAnnotationsVisitor.visitClassType` in `TypeSubstitutionUtils.java`, it is correct and intentional for the `BoundKind.UNBOUND` case to fall through without extracting a bound — there is nothing to restore. Only `BoundKind.EXTENDS` and `BoundKind.SUPER` need explicit bound extraction for nullability annotation restoration.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: msridhar
Repo: uber/NullAway PR: 1485
File: nullaway/src/main/java/com/uber/nullaway/NullAway.java:2119-2128
Timestamp: 2026-03-20T15:34:10.657Z
Learning: In this repo (uber/NullAway), don’t annotate local variables with `Nullable` or `NonNull`. NullAway infers nullability for local variables automatically, so adding these annotations to locals is incorrect style for this codebase.


Type updated = updateDirectNullabilityAnnotationsForType(t, other);
if (!(other instanceof Type.ClassType)) {
return updated;
Expand Down
Loading
Loading