From 46c1a875b14dcb4bacc7c263b7417088fb84ab53 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 9 May 2026 20:04:17 -0700 Subject: [PATCH 01/11] test case --- .../nullaway/jspecify/GenericMethodTests.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 4b53cf1fe1..7cff5575ea 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -1685,6 +1685,33 @@ static class Foo { .doTest(); } + @Test + public void caffeineNestedArgToGenericMethod() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + import org.jspecify.annotations.NonNull; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + import java.util.Map; + import java.util.concurrent.CompletableFuture; + @NullMarked + class Test { + static interface Cache { + Policy policy(); + } + static interface Policy { + Map> refreshes(); + } + static void m(@Nullable Map map) {} + void test(Cache cache) { + m(cache.policy().refreshes()); + } + }""") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From 7b4af618fb988517c4e955a3089eb363ba9eedea Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 10 May 2026 11:18:25 -0700 Subject: [PATCH 02/11] more --- .../nullaway/generics/GenericsChecks.java | 226 ++++++++++++++---- .../nullaway/jspecify/GenericMethodTests.java | 21 ++ 2 files changed, 196 insertions(+), 51 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 de4b8aadf4..c8c401021e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2151,11 +2151,11 @@ private Type substituteTypeArgsInGenericMethodType( } /** - * In narrow cases, javac drops nested type-use nullability annotations on type variables in its - * inferred type for a generic method at a call site. See - * https://github.com/uber/NullAway/issues/1455. This method aims to restore those annotations - * based on the types of actual parameters. It does not attempt to be a very general fix, as we do - * not fully understand the scenarios where this can arise. + * In narrow cases, javac drops or misplaces nested type-use nullability annotations on type + * variables in its inferred type for a generic method at a call site. See + * https://github.com/uber/NullAway/issues/1455. This method repairs those annotations based on + * the types of actual parameters. It does not attempt to be a very general fix, as we do not + * fully understand the scenarios where this can arise. * * @param invocationTree the method invocation tree for the generic method call * @param origMethodType the declared method type for the generic method (to identify formal @@ -2187,57 +2187,30 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( TreePath pathToInvocation = pathWithLeaf(state.getPath(), invocationTree); // use this map to store repaired substitutions for method type variables, to ensure we use the // same repaired substitution for all occurrences of the same type variable - Map repairedTopLevelSubstitutions = new HashMap<>(); + Map repairedSubstitutions = new HashMap<>(); ListBuffer updatedArgTypes = new ListBuffer<>(); boolean changed = false; for (int i = 0; i < genericMethodParamTypes.size(); i++) { Type callSiteParamType = callSiteParamTypes.get(i); Type genericMethodParamType = genericMethodParamTypes.get(i); - // only attempt a repair when the generic method's parameter type is a type variable of the - // method - if (genericMethodParamType instanceof Type.TypeVar typeVar - && typeVar.tsym.owner == methodSymbol) { - Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; - Type repairedSubstitution = repairedTopLevelSubstitutions.get(typeVarSymbol); - if (repairedSubstitution != null) { - // re-use the previous substitution, to ensure consistency - if (repairedSubstitution != callSiteParamType) { - changed = true; - callSiteParamType = repairedSubstitution; - } - } else { // need to compute the substitution - ExpressionTree actualParam = actualParams.get(i); - Type actualArgType = - getTreeType( - actualParam, - state.withPath(pathWithLeaf(pathToInvocation, actualParam)), - calledFromDataflow); - // only handle cases of non-raw actual parameter types that have the same base type as the - // inferred parameter type at the call site - if (actualArgType != null - && !actualArgType.isRaw() - && state - .getTypes() - .isSameType( - state.getTypes().erasure(actualArgType), - state.getTypes().erasure(callSiteParamType))) { - // restore explicit nested annotations from the actual parameter type to the call site - // parameter type (this will only apply to nested type variables within - // callSiteParamType) - Type restoredType = - TypeSubstitutionUtils.restoreExplicitNullabilityAnnotations( - actualArgType, callSiteParamType, config, Collections.emptyMap()); - // remember the substitution so we use it consistently at other parameter positions - repairedTopLevelSubstitutions.put(typeVarSymbol, restoredType); - if (restoredType != callSiteParamType) { - changed = true; - callSiteParamType = restoredType; - } - } else { - // remember that we did _not_ change anything, again for consistency across parameter - // positions - repairedTopLevelSubstitutions.put(typeVarSymbol, callSiteParamType); - } + ExpressionTree actualParam = actualParams.get(i); + Type actualArgType = + getTreeType( + actualParam, + state.withPath(pathWithLeaf(pathToInvocation, actualParam)), + calledFromDataflow); + if (actualArgType != null) { + Type repairedType = + repairNestedTypeVarSubstitutions( + methodSymbol, + genericMethodParamType, + actualArgType, + callSiteParamType, + repairedSubstitutions, + state); + if (repairedType != callSiteParamType) { + changed = true; + callSiteParamType = repairedType; } } updatedArgTypes.append(callSiteParamType); @@ -2252,6 +2225,157 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( methodTypeAtCallSite.tsym); } + /** + * Recursively repairs inferred substitutions for method type variables in {@code callSiteType}. + * + *

The three input types are aligned views of the same parameter: {@code genericMethodType} is + * the declared generic method parameter type, {@code actualArgType} is the type of the expression + * passed to that parameter, and {@code callSiteType} is javac's inferred parameter type at the + * call site. When a method type variable is found in the declared type, we use the corresponding + * actual argument subtree to repair nested annotations in javac's inferred subtree, as long as + * the two have the same erased type. + */ + @SuppressWarnings("ReferenceEquality") + private Type repairNestedTypeVarSubstitutions( + Symbol.MethodSymbol methodSymbol, + Type genericMethodType, + Type actualArgType, + Type callSiteType, + Map repairedSubstitutions, + VisitorState state) { + if (genericMethodType instanceof Type.TypeVar typeVar && typeVar.tsym.owner == methodSymbol) { + return repairTypeVarSubstitution( + typeVar, actualArgType, callSiteType, repairedSubstitutions, state); + } + if (genericMethodType instanceof Type.ClassType + && actualArgType instanceof Type.ClassType + && callSiteType instanceof Type.ClassType callSiteClassType) { + Type actualAsCallSiteType = + TypeSubstitutionUtils.asSuper( + state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); + if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { + return callSiteType; + } + List genericTypeArgs = genericMethodType.getTypeArguments(); + List actualTypeArgs = actualClassType.getTypeArguments(); + List callSiteTypeArgs = callSiteClassType.getTypeArguments(); + if (genericTypeArgs.size() != actualTypeArgs.size() + || genericTypeArgs.size() != callSiteTypeArgs.size()) { + return callSiteType; + } + boolean changed = false; + ListBuffer updatedTypeArgs = new ListBuffer<>(); + for (int i = 0; i < genericTypeArgs.size(); i++) { + Type callSiteTypeArg = callSiteTypeArgs.get(i); + Type repairedTypeArg = + repairNestedTypeVarSubstitutions( + methodSymbol, + genericTypeArgs.get(i), + actualTypeArgs.get(i), + callSiteTypeArg, + repairedSubstitutions, + state); + if (repairedTypeArg != callSiteTypeArg) { + changed = true; + } + updatedTypeArgs.append(repairedTypeArg); + } + Type enclosingType = callSiteClassType.getEnclosingType(); + Type repairedEnclosingType = + repairNestedTypeVarSubstitutions( + methodSymbol, + genericMethodType.getEnclosingType(), + actualClassType.getEnclosingType(), + enclosingType, + repairedSubstitutions, + state); + if (repairedEnclosingType != enclosingType) { + changed = true; + } + return changed + ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( + callSiteClassType, repairedEnclosingType, updatedTypeArgs.toList()) + : callSiteType; + } + if (genericMethodType instanceof Type.ArrayType genericArrayType + && actualArgType instanceof Type.ArrayType actualArrayType + && callSiteType instanceof Type.ArrayType callSiteArrayType) { + Type callSiteElemType = callSiteArrayType.getComponentType(); + Type repairedElemType = + repairNestedTypeVarSubstitutions( + methodSymbol, + genericArrayType.getComponentType(), + actualArrayType.getComponentType(), + callSiteElemType, + repairedSubstitutions, + state); + return repairedElemType != callSiteElemType + ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( + callSiteArrayType, repairedElemType) + : callSiteType; + } + return callSiteType; + } + + private Type repairTypeVarSubstitution( + Type.TypeVar typeVar, + Type actualArgType, + Type callSiteType, + Map repairedSubstitutions, + VisitorState state) { + Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; + if (repairedSubstitutions.containsKey(typeVarSymbol)) { + return castToNonNull(repairedSubstitutions.get(typeVarSymbol)); + } + Type repairedSubstitution = callSiteType; + if (!actualArgType.isRaw() && !callSiteType.isRaw()) { + repairedSubstitution = + repairNestedTypeVarSubstitutionFromActual(actualArgType, callSiteType, state); + } + repairedSubstitutions.put(typeVarSymbol, repairedSubstitution); + return repairedSubstitution; + } + + /** + * Repairs nested annotations in {@code callSiteType} using {@code actualArgType}, while + * preserving direct annotations on {@code callSiteType}. + */ + private Type repairNestedTypeVarSubstitutionFromActual( + Type actualArgType, Type callSiteType, VisitorState state) { + if (!sameErasure(actualArgType, callSiteType, state)) { + return callSiteType; + } + if (actualArgType instanceof Type.ClassType + && callSiteType instanceof Type.ClassType callSiteClassType) { + Type actualAsCallSiteType = + TypeSubstitutionUtils.asSuper( + state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); + if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { + return callSiteType; + } + List actualTypeArgs = actualClassType.getTypeArguments(); + if (actualTypeArgs.isEmpty()) { + return callSiteType; + } + return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( + callSiteClassType, callSiteClassType.getEnclosingType(), actualTypeArgs); + } + if (actualArgType instanceof Type.ArrayType actualArrayType + && callSiteType instanceof Type.ArrayType callSiteArrayType) { + return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( + callSiteArrayType, actualArrayType.getComponentType()); + } + return callSiteType; + } + + private boolean sameErasure(Type type1, Type type2, VisitorState state) { + return !type1.getKind().equals(TypeKind.NULL) + && !type2.getKind().equals(TypeKind.NULL) + && state + .getTypes() + .isSameType(state.getTypes().erasure(type1), state.getTypes().erasure(type2)); + } + /** * An invocation of a generic method, and the corresponding information about its assignment * context, for the purposes of inference. diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 7cff5575ea..2aab256d22 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -1712,6 +1712,27 @@ void test(Cache cache) { .doTest(); } + @Test + public void nestedGenericMethodRepairPreservesTopLevelNullability() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + import java.util.concurrent.CompletableFuture; + @NullMarked + class Test { + static class Box {} + static void accept(Box<@Nullable T> box) {} + void test(Box> box) { + // BUG: Diagnostic contains: inference failure: type variable T constrained to be both @NonNull and @Nullable + accept(box); + } + }""") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From eb8e3a1b417730df6c531a12a8dea29ff40a9e6f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 12 May 2026 08:28:16 -0700 Subject: [PATCH 03/11] refactor --- .../nullaway/generics/GenericsChecks.java | 164 +---------------- ...estedTypeVarSubstitutionRepairVisitor.java | 170 ++++++++++++++++++ 2 files changed, 173 insertions(+), 161 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.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 c8c401021e..3e5d6a8a3a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2185,9 +2185,8 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( methodTypeAtCallSite.getParameterTypes(); List actualParams = invocationTree.getArguments(); TreePath pathToInvocation = pathWithLeaf(state.getPath(), invocationTree); - // use this map to store repaired substitutions for method type variables, to ensure we use the - // same repaired substitution for all occurrences of the same type variable - Map repairedSubstitutions = new HashMap<>(); + NestedTypeVarSubstitutionRepairVisitor repairVisitor = + new NestedTypeVarSubstitutionRepairVisitor(methodSymbol, state, config); ListBuffer updatedArgTypes = new ListBuffer<>(); boolean changed = false; for (int i = 0; i < genericMethodParamTypes.size(); i++) { @@ -2201,13 +2200,7 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( calledFromDataflow); if (actualArgType != null) { Type repairedType = - repairNestedTypeVarSubstitutions( - methodSymbol, - genericMethodParamType, - actualArgType, - callSiteParamType, - repairedSubstitutions, - state); + repairVisitor.repair(genericMethodParamType, actualArgType, callSiteParamType); if (repairedType != callSiteParamType) { changed = true; callSiteParamType = repairedType; @@ -2225,157 +2218,6 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( methodTypeAtCallSite.tsym); } - /** - * Recursively repairs inferred substitutions for method type variables in {@code callSiteType}. - * - *

The three input types are aligned views of the same parameter: {@code genericMethodType} is - * the declared generic method parameter type, {@code actualArgType} is the type of the expression - * passed to that parameter, and {@code callSiteType} is javac's inferred parameter type at the - * call site. When a method type variable is found in the declared type, we use the corresponding - * actual argument subtree to repair nested annotations in javac's inferred subtree, as long as - * the two have the same erased type. - */ - @SuppressWarnings("ReferenceEquality") - private Type repairNestedTypeVarSubstitutions( - Symbol.MethodSymbol methodSymbol, - Type genericMethodType, - Type actualArgType, - Type callSiteType, - Map repairedSubstitutions, - VisitorState state) { - if (genericMethodType instanceof Type.TypeVar typeVar && typeVar.tsym.owner == methodSymbol) { - return repairTypeVarSubstitution( - typeVar, actualArgType, callSiteType, repairedSubstitutions, state); - } - if (genericMethodType instanceof Type.ClassType - && actualArgType instanceof Type.ClassType - && callSiteType instanceof Type.ClassType callSiteClassType) { - Type actualAsCallSiteType = - TypeSubstitutionUtils.asSuper( - state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); - if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { - return callSiteType; - } - List genericTypeArgs = genericMethodType.getTypeArguments(); - List actualTypeArgs = actualClassType.getTypeArguments(); - List callSiteTypeArgs = callSiteClassType.getTypeArguments(); - if (genericTypeArgs.size() != actualTypeArgs.size() - || genericTypeArgs.size() != callSiteTypeArgs.size()) { - return callSiteType; - } - boolean changed = false; - ListBuffer updatedTypeArgs = new ListBuffer<>(); - for (int i = 0; i < genericTypeArgs.size(); i++) { - Type callSiteTypeArg = callSiteTypeArgs.get(i); - Type repairedTypeArg = - repairNestedTypeVarSubstitutions( - methodSymbol, - genericTypeArgs.get(i), - actualTypeArgs.get(i), - callSiteTypeArg, - repairedSubstitutions, - state); - if (repairedTypeArg != callSiteTypeArg) { - changed = true; - } - updatedTypeArgs.append(repairedTypeArg); - } - Type enclosingType = callSiteClassType.getEnclosingType(); - Type repairedEnclosingType = - repairNestedTypeVarSubstitutions( - methodSymbol, - genericMethodType.getEnclosingType(), - actualClassType.getEnclosingType(), - enclosingType, - repairedSubstitutions, - state); - if (repairedEnclosingType != enclosingType) { - changed = true; - } - return changed - ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( - callSiteClassType, repairedEnclosingType, updatedTypeArgs.toList()) - : callSiteType; - } - if (genericMethodType instanceof Type.ArrayType genericArrayType - && actualArgType instanceof Type.ArrayType actualArrayType - && callSiteType instanceof Type.ArrayType callSiteArrayType) { - Type callSiteElemType = callSiteArrayType.getComponentType(); - Type repairedElemType = - repairNestedTypeVarSubstitutions( - methodSymbol, - genericArrayType.getComponentType(), - actualArrayType.getComponentType(), - callSiteElemType, - repairedSubstitutions, - state); - return repairedElemType != callSiteElemType - ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( - callSiteArrayType, repairedElemType) - : callSiteType; - } - return callSiteType; - } - - private Type repairTypeVarSubstitution( - Type.TypeVar typeVar, - Type actualArgType, - Type callSiteType, - Map repairedSubstitutions, - VisitorState state) { - Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; - if (repairedSubstitutions.containsKey(typeVarSymbol)) { - return castToNonNull(repairedSubstitutions.get(typeVarSymbol)); - } - Type repairedSubstitution = callSiteType; - if (!actualArgType.isRaw() && !callSiteType.isRaw()) { - repairedSubstitution = - repairNestedTypeVarSubstitutionFromActual(actualArgType, callSiteType, state); - } - repairedSubstitutions.put(typeVarSymbol, repairedSubstitution); - return repairedSubstitution; - } - - /** - * Repairs nested annotations in {@code callSiteType} using {@code actualArgType}, while - * preserving direct annotations on {@code callSiteType}. - */ - private Type repairNestedTypeVarSubstitutionFromActual( - Type actualArgType, Type callSiteType, VisitorState state) { - if (!sameErasure(actualArgType, callSiteType, state)) { - return callSiteType; - } - if (actualArgType instanceof Type.ClassType - && callSiteType instanceof Type.ClassType callSiteClassType) { - Type actualAsCallSiteType = - TypeSubstitutionUtils.asSuper( - state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); - if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { - return callSiteType; - } - List actualTypeArgs = actualClassType.getTypeArguments(); - if (actualTypeArgs.isEmpty()) { - return callSiteType; - } - return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( - callSiteClassType, callSiteClassType.getEnclosingType(), actualTypeArgs); - } - if (actualArgType instanceof Type.ArrayType actualArrayType - && callSiteType instanceof Type.ArrayType callSiteArrayType) { - return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( - callSiteArrayType, actualArrayType.getComponentType()); - } - return callSiteType; - } - - private boolean sameErasure(Type type1, Type type2, VisitorState state) { - return !type1.getKind().equals(TypeKind.NULL) - && !type2.getKind().equals(TypeKind.NULL) - && state - .getTypes() - .isSameType(state.getTypes().erasure(type1), state.getTypes().erasure(type2)); - } - /** * An invocation of a generic method, and the corresponding information about its assignment * context, for the purposes of inference. diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java new file mode 100644 index 0000000000..c760e5747e --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -0,0 +1,170 @@ +package com.uber.nullaway.generics; + +import static com.uber.nullaway.NullabilityUtil.castToNonNull; + +import com.google.errorprone.VisitorState; +import com.sun.tools.javac.code.Symbol; +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.Config; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.lang.model.type.TypeKind; + +/** + * Repairs inferred substitutions for method type variables in a call-site type using nested + * nullability annotations from the corresponding actual argument type. + */ +@SuppressWarnings("ReferenceEquality") +final class NestedTypeVarSubstitutionRepairVisitor + extends Types.DefaultTypeVisitor { + + private final Symbol.MethodSymbol methodSymbol; + private final VisitorState state; + private final Config config; + private final Map repairedSubstitutions = new HashMap<>(); + + NestedTypeVarSubstitutionRepairVisitor( + Symbol.MethodSymbol methodSymbol, VisitorState state, Config config) { + this.methodSymbol = methodSymbol; + this.state = state; + this.config = config; + } + + Type repair(Type genericMethodType, Type actualArgType, Type callSiteType) { + return genericMethodType.accept(this, new RepairContext(actualArgType, callSiteType)); + } + + @Override + public Type visitTypeVar(Type.TypeVar typeVar, RepairContext context) { + if (typeVar.tsym.owner == methodSymbol) { + return repairTypeVarSubstitution(typeVar, context.actualArgType(), context.callSiteType()); + } + return context.callSiteType(); + } + + @Override + public Type visitClassType(Type.ClassType genericClassType, RepairContext context) { + if (!(context.actualArgType() instanceof Type.ClassType) + || !(context.callSiteType() instanceof Type.ClassType callSiteClassType)) { + return context.callSiteType(); + } + Type actualAsCallSiteType = + TypeSubstitutionUtils.asSuper( + state.getTypes(), + context.actualArgType(), + (Symbol.ClassSymbol) context.callSiteType().tsym, + config); + if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { + return context.callSiteType(); + } + List genericTypeArgs = genericClassType.getTypeArguments(); + List actualTypeArgs = actualClassType.getTypeArguments(); + List callSiteTypeArgs = callSiteClassType.getTypeArguments(); + if (genericTypeArgs.size() != actualTypeArgs.size() + || genericTypeArgs.size() != callSiteTypeArgs.size()) { + return context.callSiteType(); + } + boolean changed = false; + ListBuffer updatedTypeArgs = new ListBuffer<>(); + for (int i = 0; i < genericTypeArgs.size(); i++) { + Type callSiteTypeArg = callSiteTypeArgs.get(i); + Type repairedTypeArg = repair(genericTypeArgs.get(i), actualTypeArgs.get(i), callSiteTypeArg); + if (repairedTypeArg != callSiteTypeArg) { + changed = true; + } + updatedTypeArgs.append(repairedTypeArg); + } + Type enclosingType = callSiteClassType.getEnclosingType(); + Type repairedEnclosingType = + repair( + genericClassType.getEnclosingType(), actualClassType.getEnclosingType(), enclosingType); + if (repairedEnclosingType != enclosingType) { + changed = true; + } + return changed + ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( + callSiteClassType, repairedEnclosingType, updatedTypeArgs.toList()) + : context.callSiteType(); + } + + @Override + public Type visitArrayType(Type.ArrayType genericArrayType, RepairContext context) { + if (!(context.actualArgType() instanceof Type.ArrayType actualArrayType) + || !(context.callSiteType() instanceof Type.ArrayType callSiteArrayType)) { + return context.callSiteType(); + } + Type callSiteElemType = callSiteArrayType.getComponentType(); + Type repairedElemType = + repair( + genericArrayType.getComponentType(), + actualArrayType.getComponentType(), + callSiteElemType); + return repairedElemType != callSiteElemType + ? TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( + callSiteArrayType, repairedElemType) + : context.callSiteType(); + } + + @Override + public Type visitType(Type type, RepairContext context) { + return context.callSiteType(); + } + + private Type repairTypeVarSubstitution( + Type.TypeVar typeVar, Type actualArgType, Type callSiteType) { + Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; + if (repairedSubstitutions.containsKey(typeVarSymbol)) { + return castToNonNull(repairedSubstitutions.get(typeVarSymbol)); + } + Type repairedSubstitution = callSiteType; + if (!actualArgType.isRaw() && !callSiteType.isRaw()) { + repairedSubstitution = repairNestedTypeVarSubstitutionFromActual(actualArgType, callSiteType); + } + repairedSubstitutions.put(typeVarSymbol, repairedSubstitution); + return repairedSubstitution; + } + + /** + * Repairs nested annotations in {@code callSiteType} using {@code actualArgType}, while + * preserving direct annotations on {@code callSiteType}. + */ + private Type repairNestedTypeVarSubstitutionFromActual(Type actualArgType, Type callSiteType) { + if (!sameErasure(actualArgType, callSiteType)) { + return callSiteType; + } + if (actualArgType instanceof Type.ClassType + && callSiteType instanceof Type.ClassType callSiteClassType) { + Type actualAsCallSiteType = + TypeSubstitutionUtils.asSuper( + state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); + if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { + return callSiteType; + } + List actualTypeArgs = actualClassType.getTypeArguments(); + if (actualTypeArgs.isEmpty()) { + return callSiteType; + } + return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( + callSiteClassType, callSiteClassType.getEnclosingType(), actualTypeArgs); + } + if (actualArgType instanceof Type.ArrayType actualArrayType + && callSiteType instanceof Type.ArrayType callSiteArrayType) { + return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( + callSiteArrayType, actualArrayType.getComponentType()); + } + return callSiteType; + } + + private boolean sameErasure(Type type1, Type type2) { + return !type1.getKind().equals(TypeKind.NULL) + && !type2.getKind().equals(TypeKind.NULL) + && state + .getTypes() + .isSameType(state.getTypes().erasure(type1), state.getTypes().erasure(type2)); + } + + record RepairContext(Type actualArgType, Type callSiteType) {} +} From ecb63251afe8fc206649f8d648bc9df94ccaad4e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 12 May 2026 17:50:38 -0700 Subject: [PATCH 04/11] cleanup --- .../com/uber/nullaway/NullabilityUtil.java | 8 ++ .../nullaway/generics/GenericsChecks.java | 65 +++------------- ...estedTypeVarSubstitutionRepairVisitor.java | 75 +++++++++++++++++-- 3 files changed, 89 insertions(+), 59 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 5861f06b02..395a0680e3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -753,6 +753,14 @@ public static ExpressionTree stripParensAndCasts(ExpressionTree expr) { return expr; } + /** + * Returns an updated version of {@code path} with {@code leaf} as the leaf, if needed. If {@code + * leaf} is already the leaf of {@code path}, just return {@code path} unmodified. + */ + public static TreePath pathWithLeaf(TreePath path, Tree leaf) { + return path.getLeaf() == leaf ? path : new TreePath(path, leaf); + } + /** * A pair of an expression tree and a VisitorState, used by {@link #stripParensAndUpdateTreePath} */ 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 3e5d6a8a3a..1a2e339ffc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2,6 +2,7 @@ import static com.google.common.base.Verify.verify; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static com.uber.nullaway.NullabilityUtil.pathWithLeaf; import com.google.common.base.Preconditions; import com.google.common.base.Verify; @@ -38,7 +39,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; @@ -492,7 +492,8 @@ private void reportInvalidOverridingMethodParamTypeError( * @param calledFromDataflow true if the type is being computed as part of dataflow analysis * @return Type of the tree with preserved annotations. */ - private @Nullable Type getTreeType(Tree tree, VisitorState state, boolean calledFromDataflow) { + /* package-private */ @Nullable Type getTreeType( + Tree tree, VisitorState state, boolean calledFromDataflow) { if (tree instanceof ExpressionTree exprTree) { NullabilityUtil.ExprTreeAndState exprTreeAndState = NullabilityUtil.stripParensAndUpdateTreePath(exprTree, state); @@ -1489,14 +1490,6 @@ private Type refineArgumentTypeWithDataflow( return updateTypeWithNullness(state, exprType, refinedNullness); } - /** - * Returns an updated version of {@code path} with {@code leaf} as the leaf, if needed. If {@code - * leaf} is already the leaf of {@code path}, just return {@code path} unmodified. - */ - private static TreePath pathWithLeaf(TreePath path, Tree leaf) { - return path.getLeaf() == leaf ? path : new TreePath(path, leaf); - } - /** * Sets up the environment mapping for a lambda expression so that dataflow analysis can be run * within the lambda body, handling the case where dataflow analysis is already running on the @@ -2167,55 +2160,21 @@ private Type substituteTypeArgsInGenericMethodType( * annotations on type variables restored to match those on actual parameters passed at the * call site */ - @SuppressWarnings("ReferenceEquality") private Type.MethodType restoreNestedNullabilityForTypeVarArguments( MethodInvocationTree invocationTree, Type.MethodType origMethodType, Type.MethodType methodTypeAtCallSite, VisitorState state, boolean calledFromDataflow) { - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(invocationTree); - if (methodSymbol.isVarArgs()) { - // skip handling of varargs for now - return methodTypeAtCallSite; - } - com.sun.tools.javac.util.List genericMethodParamTypes = - origMethodType.getParameterTypes(); - com.sun.tools.javac.util.List callSiteParamTypes = - methodTypeAtCallSite.getParameterTypes(); - List actualParams = invocationTree.getArguments(); - TreePath pathToInvocation = pathWithLeaf(state.getPath(), invocationTree); - NestedTypeVarSubstitutionRepairVisitor repairVisitor = - new NestedTypeVarSubstitutionRepairVisitor(methodSymbol, state, config); - ListBuffer updatedArgTypes = new ListBuffer<>(); - boolean changed = false; - for (int i = 0; i < genericMethodParamTypes.size(); i++) { - Type callSiteParamType = callSiteParamTypes.get(i); - Type genericMethodParamType = genericMethodParamTypes.get(i); - ExpressionTree actualParam = actualParams.get(i); - Type actualArgType = - getTreeType( - actualParam, - state.withPath(pathWithLeaf(pathToInvocation, actualParam)), - calledFromDataflow); - if (actualArgType != null) { - Type repairedType = - repairVisitor.repair(genericMethodParamType, actualArgType, callSiteParamType); - if (repairedType != callSiteParamType) { - changed = true; - callSiteParamType = repairedType; - } - } - updatedArgTypes.append(callSiteParamType); - } - if (!changed) { - return methodTypeAtCallSite; - } - return new Type.MethodType( - updatedArgTypes.toList(), - methodTypeAtCallSite.getReturnType(), - methodTypeAtCallSite.getThrownTypes(), - methodTypeAtCallSite.tsym); + return new NestedTypeVarSubstitutionRepairVisitor( + this, + invocationTree, + origMethodType, + methodTypeAtCallSite, + state, + config, + calledFromDataflow) + .repairMethodType(); } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index c760e5747e..ef0b0cc0f7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -1,8 +1,13 @@ package com.uber.nullaway.generics; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static com.uber.nullaway.NullabilityUtil.pathWithLeaf; import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; @@ -21,19 +26,76 @@ final class NestedTypeVarSubstitutionRepairVisitor extends Types.DefaultTypeVisitor { + private final GenericsChecks genericsChecks; + private final MethodInvocationTree invocationTree; + private final Type.MethodType origMethodType; + private final Type.MethodType methodTypeAtCallSite; private final Symbol.MethodSymbol methodSymbol; private final VisitorState state; private final Config config; + private final boolean calledFromDataflow; private final Map repairedSubstitutions = new HashMap<>(); NestedTypeVarSubstitutionRepairVisitor( - Symbol.MethodSymbol methodSymbol, VisitorState state, Config config) { - this.methodSymbol = methodSymbol; + GenericsChecks genericsChecks, + MethodInvocationTree invocationTree, + Type.MethodType origMethodType, + Type.MethodType methodTypeAtCallSite, + VisitorState state, + Config config, + boolean calledFromDataflow) { + this.genericsChecks = genericsChecks; + this.invocationTree = invocationTree; + this.origMethodType = origMethodType; + this.methodTypeAtCallSite = methodTypeAtCallSite; + this.methodSymbol = ASTHelpers.getSymbol(invocationTree); this.state = state; this.config = config; + this.calledFromDataflow = calledFromDataflow; } - Type repair(Type genericMethodType, Type actualArgType, Type callSiteType) { + Type.MethodType repairMethodType() { + if (methodSymbol.isVarArgs()) { + // skip handling of varargs for now + return methodTypeAtCallSite; + } + com.sun.tools.javac.util.List genericMethodParamTypes = + origMethodType.getParameterTypes(); + com.sun.tools.javac.util.List callSiteParamTypes = + methodTypeAtCallSite.getParameterTypes(); + List actualParams = invocationTree.getArguments(); + TreePath pathToInvocation = pathWithLeaf(state.getPath(), invocationTree); + ListBuffer updatedArgTypes = new ListBuffer<>(); + boolean changed = false; + for (int i = 0; i < genericMethodParamTypes.size(); i++) { + Type callSiteParamType = callSiteParamTypes.get(i); + Type genericMethodParamType = genericMethodParamTypes.get(i); + ExpressionTree actualParam = actualParams.get(i); + Type actualArgType = + genericsChecks.getTreeType( + actualParam, + state.withPath(pathWithLeaf(pathToInvocation, actualParam)), + calledFromDataflow); + if (actualArgType != null) { + Type repairedType = repairType(genericMethodParamType, actualArgType, callSiteParamType); + if (repairedType != callSiteParamType) { + changed = true; + callSiteParamType = repairedType; + } + } + updatedArgTypes.append(callSiteParamType); + } + if (!changed) { + return methodTypeAtCallSite; + } + return new Type.MethodType( + updatedArgTypes.toList(), + methodTypeAtCallSite.getReturnType(), + methodTypeAtCallSite.getThrownTypes(), + methodTypeAtCallSite.tsym); + } + + private Type repairType(Type genericMethodType, Type actualArgType, Type callSiteType) { return genericMethodType.accept(this, new RepairContext(actualArgType, callSiteType)); } @@ -71,7 +133,8 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex ListBuffer updatedTypeArgs = new ListBuffer<>(); for (int i = 0; i < genericTypeArgs.size(); i++) { Type callSiteTypeArg = callSiteTypeArgs.get(i); - Type repairedTypeArg = repair(genericTypeArgs.get(i), actualTypeArgs.get(i), callSiteTypeArg); + Type repairedTypeArg = + repairType(genericTypeArgs.get(i), actualTypeArgs.get(i), callSiteTypeArg); if (repairedTypeArg != callSiteTypeArg) { changed = true; } @@ -79,7 +142,7 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex } Type enclosingType = callSiteClassType.getEnclosingType(); Type repairedEnclosingType = - repair( + repairType( genericClassType.getEnclosingType(), actualClassType.getEnclosingType(), enclosingType); if (repairedEnclosingType != enclosingType) { changed = true; @@ -98,7 +161,7 @@ public Type visitArrayType(Type.ArrayType genericArrayType, RepairContext contex } Type callSiteElemType = callSiteArrayType.getComponentType(); Type repairedElemType = - repair( + repairType( genericArrayType.getComponentType(), actualArrayType.getComponentType(), callSiteElemType); From c534555924080e68906774405052f9f0e94bbdae Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 12 May 2026 17:57:01 -0700 Subject: [PATCH 05/11] make it a static method --- .../nullaway/generics/GenericsChecks.java | 17 ++++---- ...estedTypeVarSubstitutionRepairVisitor.java | 40 ++++++++++++++++++- 2 files changed, 46 insertions(+), 11 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 1a2e339ffc..b3adb1d612 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2166,15 +2166,14 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( Type.MethodType methodTypeAtCallSite, VisitorState state, boolean calledFromDataflow) { - return new NestedTypeVarSubstitutionRepairVisitor( - this, - invocationTree, - origMethodType, - methodTypeAtCallSite, - state, - config, - calledFromDataflow) - .repairMethodType(); + return NestedTypeVarSubstitutionRepairVisitor.repairMethodType( + this, + invocationTree, + origMethodType, + methodTypeAtCallSite, + state, + config, + calledFromDataflow); } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index ef0b0cc0f7..19d5435642 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -36,7 +36,43 @@ final class NestedTypeVarSubstitutionRepairVisitor private final boolean calledFromDataflow; private final Map repairedSubstitutions = new HashMap<>(); - NestedTypeVarSubstitutionRepairVisitor( + /** + * Repairs nested nullability annotations in the inferred call-site method type for a generic + * method invocation. + * + *

This method creates one visitor for the entire invocation so repaired substitutions for + * method type variables are shared across all parameters of the call. + * + * @param genericsChecks the owning generics checker, used to compute actual argument types + * @param invocationTree the method invocation tree for the generic method call + * @param origMethodType the declared method type for the generic method + * @param methodTypeAtCallSite the method type inferred by javac at the call site + * @param state the visitor state + * @param config the NullAway configuration + * @param calledFromDataflow true if the repair is being computed as part of dataflow analysis + * @return a method type based on {@code methodTypeAtCallSite}, with nested nullability + * annotations on method type-variable substitutions restored where possible + */ + static Type.MethodType repairMethodType( + GenericsChecks genericsChecks, + MethodInvocationTree invocationTree, + Type.MethodType origMethodType, + Type.MethodType methodTypeAtCallSite, + VisitorState state, + Config config, + boolean calledFromDataflow) { + return new NestedTypeVarSubstitutionRepairVisitor( + genericsChecks, + invocationTree, + origMethodType, + methodTypeAtCallSite, + state, + config, + calledFromDataflow) + .repairMethodTypeInternal(); + } + + private NestedTypeVarSubstitutionRepairVisitor( GenericsChecks genericsChecks, MethodInvocationTree invocationTree, Type.MethodType origMethodType, @@ -54,7 +90,7 @@ final class NestedTypeVarSubstitutionRepairVisitor this.calledFromDataflow = calledFromDataflow; } - Type.MethodType repairMethodType() { + private Type.MethodType repairMethodTypeInternal() { if (methodSymbol.isVarArgs()) { // skip handling of varargs for now return methodTypeAtCallSite; From 76d1782a5449d488d5d2e5c9f718df466bd6467e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 12 May 2026 18:02:30 -0700 Subject: [PATCH 06/11] cleanup --- .../nullaway/generics/GenericsChecks.java | 8 ++++---- ...estedTypeVarSubstitutionRepairVisitor.java | 19 +++++++++++++++---- 2 files changed, 19 insertions(+), 8 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 b3adb1d612..9d42adf40d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -2145,10 +2145,10 @@ private Type substituteTypeArgsInGenericMethodType( /** * In narrow cases, javac drops or misplaces nested type-use nullability annotations on type - * variables in its inferred type for a generic method at a call site. See - * https://github.com/uber/NullAway/issues/1455. This method repairs those annotations based on - * the types of actual parameters. It does not attempt to be a very general fix, as we do not - * fully understand the scenarios where this can arise. + * variables in its inferred type for a generic method at a call site. See issue 1455. This method repairs those + * annotations based on the types of actual parameters. It does not attempt to be a very general + * fix, as we do not fully understand the scenarios where this can arise. * * @param invocationTree the method invocation tree for the generic method call * @param origMethodType the declared method type for the generic method (to identify formal diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index 19d5435642..6fa9f80285 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -38,10 +38,11 @@ final class NestedTypeVarSubstitutionRepairVisitor /** * Repairs nested nullability annotations in the inferred call-site method type for a generic - * method invocation. - * - *

This method creates one visitor for the entire invocation so repaired substitutions for - * method type variables are shared across all parameters of the call. + * method invocation. In narrow cases, javac drops or misplaces nested type-use nullability + * annotations on type variables in its inferred type for a generic method at a call site. See issue 1455. This method repairs those + * annotations based on the types of actual parameters. It does not attempt to be a very general + * fix, as we do not fully understand the scenarios where this can arise. * * @param genericsChecks the owning generics checker, used to compute actual argument types * @param invocationTree the method invocation tree for the generic method call @@ -265,5 +266,15 @@ private boolean sameErasure(Type type1, Type type2) { .isSameType(state.getTypes().erasure(type1), state.getTypes().erasure(type2)); } + /** + * The two types being compared while recursively walking the declared generic method parameter + * type. At each recursive step, the visitor uses {@code actualArgType} as the source of nested + * nullability annotations and applies any repair to the corresponding subtree of {@code + * callSiteType}. + * + * @param actualArgType the subtree of the actual argument type aligned with the current declared + * generic method parameter subtree + * @param callSiteType the subtree of javac's inferred call-site parameter type to repair + */ record RepairContext(Type actualArgType, Type callSiteType) {} } From 2a8271e7dadc6e23c208fc3d87874c58e540a2a3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 23 May 2026 14:23:55 -0700 Subject: [PATCH 07/11] narrow suppressions --- .../NestedTypeVarSubstitutionRepairVisitor.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index 6fa9f80285..c24c8d8177 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -16,13 +16,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import javax.lang.model.type.TypeKind; /** * Repairs inferred substitutions for method type variables in a call-site type using nested * nullability annotations from the corresponding actual argument type. */ -@SuppressWarnings("ReferenceEquality") final class NestedTypeVarSubstitutionRepairVisitor extends Types.DefaultTypeVisitor { @@ -91,6 +91,8 @@ private NestedTypeVarSubstitutionRepairVisitor( this.calledFromDataflow = calledFromDataflow; } + // suppress since we want to check for a specific identical Type object to check for changes + @SuppressWarnings("ReferenceEquality") private Type.MethodType repairMethodTypeInternal() { if (methodSymbol.isVarArgs()) { // skip handling of varargs for now @@ -138,12 +140,14 @@ private Type repairType(Type genericMethodType, Type actualArgType, Type callSit @Override public Type visitTypeVar(Type.TypeVar typeVar, RepairContext context) { - if (typeVar.tsym.owner == methodSymbol) { + if (Objects.equals(typeVar.tsym.owner, methodSymbol)) { return repairTypeVarSubstitution(typeVar, context.actualArgType(), context.callSiteType()); } return context.callSiteType(); } + // suppress since we want to check for a specific identical Type object to check for changes + @SuppressWarnings("ReferenceEquality") @Override public Type visitClassType(Type.ClassType genericClassType, RepairContext context) { if (!(context.actualArgType() instanceof Type.ClassType) @@ -190,6 +194,8 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex : context.callSiteType(); } + // suppress since we want to check for a specific identical Type object to check for changes + @SuppressWarnings("ReferenceEquality") @Override public Type visitArrayType(Type.ArrayType genericArrayType, RepairContext context) { if (!(context.actualArgType() instanceof Type.ArrayType actualArrayType) From e9a6d1355a81a1c1a59bb7d8ec96f83d06f7c012 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 23 May 2026 15:10:22 -0700 Subject: [PATCH 08/11] improve docs --- .../nullaway/generics/GenericsChecks.java | 3 +- ...estedTypeVarSubstitutionRepairVisitor.java | 77 ++++++++++++------- 2 files changed, 51 insertions(+), 29 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 9d42adf40d..c4bbd37351 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -490,7 +490,8 @@ private void reportInvalidOverridingMethodParamTypeError( * @param tree A tree for which we need the type with preserved annotations. * @param state the visitor state * @param calledFromDataflow true if the type is being computed as part of dataflow analysis - * @return Type of the tree with preserved annotations. + * @return Type of the tree with preserved annotations. Returns {@code null} for raw types and + * other unhandled cases. */ /* package-private */ @Nullable Type getTreeType( Tree tree, VisitorState state, boolean calledFromDataflow) { diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index c24c8d8177..7b90048692 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -17,7 +17,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import javax.lang.model.type.TypeKind; /** * Repairs inferred substitutions for method type variables in a call-site type using nested @@ -28,12 +27,24 @@ final class NestedTypeVarSubstitutionRepairVisitor private final GenericsChecks genericsChecks; private final MethodInvocationTree invocationTree; + + /** declared method type for generic method */ private final Type.MethodType origMethodType; + + /** method type inferred by javac at the call site */ private final Type.MethodType methodTypeAtCallSite; + + /** symbol of the invoked generic method */ private final Symbol.MethodSymbol methodSymbol; + private final VisitorState state; private final Config config; private final boolean calledFromDataflow; + + /** + * use this map to store repaired substitutions for method type variables, to ensure we use the + * same repaired substitution for all occurrences of the same type variable + */ private final Map repairedSubstitutions = new HashMap<>(); /** @@ -91,6 +102,10 @@ private NestedTypeVarSubstitutionRepairVisitor( this.calledFromDataflow = calledFromDataflow; } + /** + * repairs all parameter types at the call site, and then returns a new method type if any + * parameter type was actually repaired. otherwise, returns {@link #methodTypeAtCallSite}. + */ // suppress since we want to check for a specific identical Type object to check for changes @SuppressWarnings("ReferenceEquality") private Type.MethodType repairMethodTypeInternal() { @@ -140,6 +155,7 @@ private Type repairType(Type genericMethodType, Type actualArgType, Type callSit @Override public Type visitTypeVar(Type.TypeVar typeVar, RepairContext context) { + // only repair type variables on the invoked method if (Objects.equals(typeVar.tsym.owner, methodSymbol)) { return repairTypeVarSubstitution(typeVar, context.actualArgType(), context.callSiteType()); } @@ -154,13 +170,14 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex || !(context.callSiteType() instanceof Type.ClassType callSiteClassType)) { return context.callSiteType(); } - Type actualAsCallSiteType = - TypeSubstitutionUtils.asSuper( - state.getTypes(), - context.actualArgType(), - (Symbol.ClassSymbol) context.callSiteType().tsym, - config); - if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { + Type.ClassType actualClassType = + (Type.ClassType) + TypeSubstitutionUtils.asSuper( + state.getTypes(), + context.actualArgType(), + (Symbol.ClassSymbol) callSiteClassType.tsym, + config); + if (actualClassType == null) { return context.callSiteType(); } List genericTypeArgs = genericClassType.getTypeArguments(); @@ -219,6 +236,13 @@ public Type visitType(Type type, RepairContext context) { return context.callSiteType(); } + /** + * @param typeVar the type variable from the generic method + * @param actualArgType the actual parameter type passed in the type variable's position at the + * call site + * @param callSiteType the type javac determined is passed in the type variable's position at the + * call site + */ private Type repairTypeVarSubstitution( Type.TypeVar typeVar, Type actualArgType, Type callSiteType) { Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; @@ -234,48 +258,45 @@ private Type repairTypeVarSubstitution( } /** - * Repairs nested annotations in {@code callSiteType} using {@code actualArgType}, while - * preserving direct annotations on {@code callSiteType}. + * Repairs nested annotations in {@code callSiteType} using the nested types from {@code + * actualArgType}, while preserving any direct annotations on {@code callSiteType}. + * + *

So, for class types, if {@code actualArgType} is {@code Foo<@Nullable Bar>} and {@code + * callSiteType} is {@code @Nullable Foo}, we return {@code @Nullable Foo<@Nullable Bar>}, + * using the top-level type from {@code callSiteType} and the type argument from {@code + * actualArgType}. + * + *

Similarly, for array types, if {@code actualArgType} is {@code @Nullable Foo []} and {@code + * callSiteType} is {@code Foo @Nullable []}, we return {@code @Nullable Foo @Nullable []}. */ private Type repairNestedTypeVarSubstitutionFromActual(Type actualArgType, Type callSiteType) { - if (!sameErasure(actualArgType, callSiteType)) { + // only handle cases where base types are identical for now + if (!ASTHelpers.isSameType(actualArgType, callSiteType, state)) { return callSiteType; } - if (actualArgType instanceof Type.ClassType + if (actualArgType instanceof Type.ClassType actualClassType && callSiteType instanceof Type.ClassType callSiteClassType) { - Type actualAsCallSiteType = - TypeSubstitutionUtils.asSuper( - state.getTypes(), actualArgType, (Symbol.ClassSymbol) callSiteType.tsym, config); - if (!(actualAsCallSiteType instanceof Type.ClassType actualClassType)) { - return callSiteType; - } List actualTypeArgs = actualClassType.getTypeArguments(); if (actualTypeArgs.isEmpty()) { return callSiteType; } + // use call site type with type arguments from actual return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createClassType( callSiteClassType, callSiteClassType.getEnclosingType(), actualTypeArgs); } if (actualArgType instanceof Type.ArrayType actualArrayType && callSiteType instanceof Type.ArrayType callSiteArrayType) { + // use call site type with component type from actual return TypeMetadataBuilder.TYPE_METADATA_BUILDER.createArrayType( callSiteArrayType, actualArrayType.getComponentType()); } return callSiteType; } - private boolean sameErasure(Type type1, Type type2) { - return !type1.getKind().equals(TypeKind.NULL) - && !type2.getKind().equals(TypeKind.NULL) - && state - .getTypes() - .isSameType(state.getTypes().erasure(type1), state.getTypes().erasure(type2)); - } - /** * The two types being compared while recursively walking the declared generic method parameter - * type. At each recursive step, the visitor uses {@code actualArgType} as the source of nested - * nullability annotations and applies any repair to the corresponding subtree of {@code + * type. At each recursive step, the visitor uses {@code actualArgType} as the "ground truth" of + * nested nullability annotations and applies any repair to the corresponding subtree of {@code * callSiteType}. * * @param actualArgType the subtree of the actual argument type aligned with the current declared From 79bebdc3e8d54850e5e7fb28c6065aaae0612510 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 23 May 2026 15:28:30 -0700 Subject: [PATCH 09/11] more docs --- ...estedTypeVarSubstitutionRepairVisitor.java | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index 7b90048692..ac48c09a0e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -125,6 +125,11 @@ private Type.MethodType repairMethodTypeInternal() { Type callSiteParamType = callSiteParamTypes.get(i); Type genericMethodParamType = genericMethodParamTypes.get(i); ExpressionTree actualParam = actualParams.get(i); + // IMPORTANT: actualArgType is the result of getTreeType(), which will apply NullAway's own + // reasoning about nullability of nested types, e.g., by running generic method inference at + // nested levels of the expression. This is how actualArgType ends up having the "ground + // truth" information about nullability of nested types, which is used to repair the + // javac-determined call site type. Type actualArgType = genericsChecks.getTreeType( actualParam, @@ -162,7 +167,18 @@ public Type visitTypeVar(Type.TypeVar typeVar, RepairContext context) { return context.callSiteType(); } - // suppress since we want to check for a specific identical Type object to check for changes + /** + * when this method is called, {@code genericClassType} appears within some level a parameter type + * for the generic method, {@code context.actualArgType()} is the (NullAway-determined) type of + * the actual parameter at the same nesting level, and {@code context.callSiteType()} is the + * javac-determined type for the parameter at the same nesting level. + * + *

This method recurses through the type arguments of {@code genericClassType}, invoking {@link + * #repairType(Type, Type, Type)} passing the corresponding type arguments from the actual + * parameter type and javac-determined call site type. If any repair occurs, returns the repaired + * type as the new type to be used at this level. (The actual repair logic only kicks in when + * visiting a nested type variable.) + */ @SuppressWarnings("ReferenceEquality") @Override public Type visitClassType(Type.ClassType genericClassType, RepairContext context) { @@ -170,6 +186,8 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex || !(context.callSiteType() instanceof Type.ClassType callSiteClassType)) { return context.callSiteType(); } + // the actual type can be a subtype of the javac-inferred call-site type, so convert to the + // supertype Type.ClassType actualClassType = (Type.ClassType) TypeSubstitutionUtils.asSuper( @@ -211,6 +229,18 @@ public Type visitClassType(Type.ClassType genericClassType, RepairContext contex : context.callSiteType(); } + /** + * when this method is called, {@code genericArrayType} appears within some level a parameter type + * for the generic method, {@code context.actualArgType()} is the (NullAway-determined) type of + * the actual parameter at the same nesting level, and {@code context.callSiteType()} is the + * javac-determined type for the parameter at the same nesting level. + * + *

This method recurses to the component type of {@code genericArrayType}, invoking {@link + * #repairType(Type, Type, Type)} passing the corresponding component type from the actual + * parameter type and javac-determined call site type. If any repair occurs, returns the repaired + * type as the new type to be used at this level. (The actual repair logic only kicks in when + * visiting a nested type variable.) + */ // suppress since we want to check for a specific identical Type object to check for changes @SuppressWarnings("ReferenceEquality") @Override @@ -237,11 +267,17 @@ public Type visitType(Type type, RepairContext context) { } /** + * For a javac-determined call site type passed in the position of a type variable from the + * generic method, update nested types in the call site type based on the corresponding nested + * types from the actual parameter. + * * @param typeVar the type variable from the generic method * @param actualArgType the actual parameter type passed in the type variable's position at the * call site * @param callSiteType the type javac determined is passed in the type variable's position at the * call site + * @return updated type to use at the position in the call site, or {@code callSiteType} if no + * repair is needed */ private Type repairTypeVarSubstitution( Type.TypeVar typeVar, Type actualArgType, Type callSiteType) { From df5baa3271fd9d89f34997febb3bc474e895a6cc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 23 May 2026 16:53:10 -0700 Subject: [PATCH 10/11] more comments in tests --- .../com/uber/nullaway/jspecify/GenericMethodTests.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 2aab256d22..8cb3c5ecb5 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -1572,6 +1572,9 @@ > T acceptSup(T supplier) { return supplier; } void test() { + // Here, javac computes the formal parameter type as Supplier. + // Our repair updates the type to Supplier<@Nullable OuterT>, matching + // the actual parameter, so we get no error. acceptSup(sup); } > void acceptTwoSup(T supplier1, T supplier2) { @@ -1580,6 +1583,7 @@ > void acceptTwoSup(T supplier1, T supplier2) { Supplier make2() { throw new RuntimeException(); } + // tests that our repair computes a consistent substitution for the type variables void test2() { // BUG: Diagnostic contains: incompatible types: Supplier cannot be converted to Supplier<@Nullable OuterT> acceptTwoSup(sup, sup2); @@ -1706,6 +1710,12 @@ static interface Policy { } static void m(@Nullable Map map) {} void test(Cache cache) { + // javac computes the formal parameter type as @Nullable Map>, + // presumably based on the @Nullable Object type argument for cache. + // NullAway determines the type of the actual parameter correctly as + // Map> (due to the @NonNull annotation on V in the signature for policy). + // The type repair in NestedTypeVarSubstitutionRepairVisitor fixes the javac type so we don't report + // an error here. m(cache.policy().refreshes()); } }""") From 9fe80434484d25634338df8324fbd4898f87ec3d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 25 May 2026 10:05:32 -0700 Subject: [PATCH 11/11] use computeIfAbsent --- ...estedTypeVarSubstitutionRepairVisitor.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java index ac48c09a0e..de0e46bfb8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java @@ -1,6 +1,5 @@ package com.uber.nullaway.generics; -import static com.uber.nullaway.NullabilityUtil.castToNonNull; import static com.uber.nullaway.NullabilityUtil.pathWithLeaf; import com.google.errorprone.VisitorState; @@ -282,15 +281,16 @@ public Type visitType(Type type, RepairContext context) { private Type repairTypeVarSubstitution( Type.TypeVar typeVar, Type actualArgType, Type callSiteType) { Symbol.TypeVariableSymbol typeVarSymbol = (Symbol.TypeVariableSymbol) typeVar.tsym; - if (repairedSubstitutions.containsKey(typeVarSymbol)) { - return castToNonNull(repairedSubstitutions.get(typeVarSymbol)); - } - Type repairedSubstitution = callSiteType; - if (!actualArgType.isRaw() && !callSiteType.isRaw()) { - repairedSubstitution = repairNestedTypeVarSubstitutionFromActual(actualArgType, callSiteType); - } - repairedSubstitutions.put(typeVarSymbol, repairedSubstitution); - return repairedSubstitution; + return repairedSubstitutions.computeIfAbsent( + typeVarSymbol, + (unused) -> { + Type repairedSubstitution = callSiteType; + if (!actualArgType.isRaw() && !callSiteType.isRaw()) { + repairedSubstitution = + repairNestedTypeVarSubstitutionFromActual(actualArgType, callSiteType); + } + return repairedSubstitution; + }); } /**