From 5fa18a76fad9b3b12a0eb18c7888b9934651a569 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 8 Jun 2026 13:04:43 -0700 Subject: [PATCH 01/13] test case --- .../uber/nullaway/jspecify/WildcardTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java index ff7334f8bd..59cc9c4255 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java @@ -933,6 +933,30 @@ class Test { .doTest(); } + @Test + public void conditionalExpr() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + interface ClassLike { + String name(); + } + static String guardedNullableWildcard(@Nullable ClassLike maybeClass) { + return (maybeClass != null ? maybeClass : fallback()).name(); + } + static ClassLike fallback() { + throw new RuntimeException(); + } + } + """) + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From 1ccab8a345381d6c5fe10fc734e337f1df28c7d7 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 14 Jun 2026 19:11:11 -0700 Subject: [PATCH 02/13] move test --- .../jspecify/ConditionalExprTests.java | 42 +++++++++++++++++++ .../uber/nullaway/jspecify/WildcardTests.java | 24 ----------- 2 files changed, 42 insertions(+), 24 deletions(-) create mode 100644 nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java new file mode 100644 index 0000000000..5839dc9afb --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -0,0 +1,42 @@ +package com.uber.nullaway.jspecify; + +import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; +import com.uber.nullaway.generics.JSpecifyJavacConfig; +import java.util.List; +import org.junit.Test; + +public class ConditionalExprTests extends NullAwayTestsBase { + + @Test + public void wildcard() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + interface ClassLike { + String name(); + } + static String guardedNullableWildcard(@Nullable ClassLike maybeClass) { + return (maybeClass != null ? maybeClass : fallback()).name(); + } + static ClassLike fallback() { + throw new RuntimeException(); + } + } + """) + .doTest(); + } + + private CompilationTestHelper makeHelperWithInferenceFailureWarning() { + return makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + List.of( + "-XepOpt:NullAway:OnlyNullMarked=true", + "-XepOpt:NullAway:WarnOnGenericInferenceFailure=true"))); + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java index 59cc9c4255..ff7334f8bd 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java @@ -933,30 +933,6 @@ class Test { .doTest(); } - @Test - public void conditionalExpr() { - makeHelperWithInferenceFailureWarning() - .addSourceLines( - "Test.java", - """ - package com.example; - import org.jspecify.annotations.*; - @NullMarked - final class Test { - interface ClassLike { - String name(); - } - static String guardedNullableWildcard(@Nullable ClassLike maybeClass) { - return (maybeClass != null ? maybeClass : fallback()).name(); - } - static ClassLike fallback() { - throw new RuntimeException(); - } - } - """) - .doTest(); - } - private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From 01b4a3576b86743178ec0fb4ae09d05575b8f88b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 14 Jun 2026 20:03:56 -0700 Subject: [PATCH 03/13] WIP --- .../nullaway/generics/GenericsChecks.java | 231 ++++++++++++++++-- .../jspecify/ConditionalExprTests.java | 217 ++++++++++++++++ 2 files changed, 434 insertions(+), 14 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 b8b716c765..f4e2c09750 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -597,10 +597,15 @@ private void reportInvalidOverridingMethodParamTypeError( tree = exprTreeAndState.expr(); state = exprTreeAndState.state(); } - if (tree instanceof LambdaExpressionTree || tree instanceof MemberReferenceTree) { + if (tree instanceof LambdaExpressionTree + || tree instanceof MemberReferenceTree + || tree instanceof ConditionalExpressionTree) { Type result = inferredPolyExpressionTypes.get(tree); if (result == null) { - result = ASTHelpers.getType(tree); + result = + tree instanceof ConditionalExpressionTree conditionalExpressionTree + ? getConditionalExpressionType(conditionalExpressionTree, state, calledFromDataflow) + : ASTHelpers.getType(tree); } return typeOrNullIfRaw(result); } @@ -803,9 +808,13 @@ private void reportInvalidOverridingMethodParamTypeError( } } if (parent instanceof ConditionalExpressionTree) { - // TODO infer diamond type from the overall conditional expression type - // tracked in https://github.com/uber/NullAway/issues/1477 - return null; + TreePath conditionalPath = getOutermostConditionalExpressionPath(parentPath); + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForConditionalExpression( + (ConditionalExpressionTree) conditionalPath.getLeaf(), + state.withPath(conditionalPath), + calledFromDataflow); + return targetTypeAndAssignmentKind.typeFromAssignmentContext(); } return null; } @@ -1044,6 +1053,13 @@ public void registerVarLocalDeclaration(VariableTree tree) { typeFromAssignmentContext, assignedToLocal, calledFromDataflow); + } else if (rhsTree instanceof ConditionalExpressionTree conditionalExpressionTree) { + return inferConditionalExpressionType( + state.withPath(pathToRhs), + conditionalExpressionTree, + pathToRhs, + typeFromAssignmentContext, + calledFromDataflow); } else { return getTreeType(rhsTree, state.withPath(pathToRhs), calledFromDataflow); } @@ -1318,6 +1334,25 @@ private void generateConstraintsForPseudoAssignment( invTree, allInvocations, calledFromDataflow); + } else if (rhsExpr instanceof ConditionalExpressionTree conditionalExpressionTree) { + ExpressionTree trueExpression = conditionalExpressionTree.getTrueExpression(); + TreePath pathToTrueExpression = new TreePath(state.getPath(), trueExpression); + generateConstraintsForPseudoAssignment( + state.withPath(pathToTrueExpression), + solver, + allInvocations, + trueExpression, + lhsType, + calledFromDataflow); + ExpressionTree falseExpression = conditionalExpressionTree.getFalseExpression(); + TreePath pathToFalseExpression = new TreePath(state.getPath(), falseExpression); + generateConstraintsForPseudoAssignment( + state.withPath(pathToFalseExpression), + solver, + allInvocations, + falseExpression, + lhsType, + calledFromDataflow); } else if (rhsExpr instanceof LambdaExpressionTree lambda) { handleLambdaInGenericMethodInference( state, state.getPath(), solver, allInvocations, lhsType, lambda, calledFromDataflow); @@ -1722,6 +1757,14 @@ public void checkTypeParameterNullnessForFunctionReturnType( formalReturnType, false, false); + } else if (retExpr instanceof ConditionalExpressionTree conditionalExpressionTree) { + returnExpressionType = + inferConditionalExpressionType( + state.withPath(pathToRetExpr), + conditionalExpressionTree, + pathToRetExpr, + formalReturnType, + false); } boolean isReturnTypeValid = subtypeParameterNullability(formalReturnType, returnExpressionType, state); @@ -1816,11 +1859,33 @@ public void checkTypeParameterNullnessForConditionalExpression( Tree truePartTree = tree.getTrueExpression(); Tree falsePartTree = tree.getFalseExpression(); - Type condExprType = getConditionalExpressionType(tree, state); + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForConditionalExpression(tree, state, false); + boolean hasTargetType = + targetTypeAndAssignmentKind.typeFromAssignmentContext() != null + || inferredPolyExpressionTypes.containsKey(tree); + Type condExprType = + inferConditionalExpressionType( + state, + tree, + state.getPath(), + targetTypeAndAssignmentKind.typeFromAssignmentContext(), + false); + if (!hasTargetType) { + return; + } + TreePath pathToTruePart = pathWithLeaf(state.getPath(), truePartTree); + TreePath pathToFalsePart = pathWithLeaf(state.getPath(), falsePartTree); Type truePartType = - getTreeType(truePartTree, state.withPath(pathWithLeaf(state.getPath(), truePartTree))); + condExprType == null + ? getTreeType(truePartTree, state.withPath(pathToTruePart)) + : getTypeForRhsOfAssignment( + tree.getTrueExpression(), pathToTruePart, condExprType, false, state, false); Type falsePartType = - getTreeType(falsePartTree, state.withPath(pathWithLeaf(state.getPath(), falsePartTree))); + condExprType == null + ? getTreeType(falsePartTree, state.withPath(pathToFalsePart)) + : getTypeForRhsOfAssignment( + tree.getFalseExpression(), pathToFalsePart, condExprType, false, state, false); // The condExpr type should be the least-upper bound of the true and false part types. To check // the nullability annotations, we check that the true and false parts are assignable to the // type of the whole expression @@ -1839,19 +1904,137 @@ public void checkTypeParameterNullnessForConditionalExpression( } private @Nullable Type getConditionalExpressionType( - ConditionalExpressionTree tree, VisitorState state) { - // hack: sometimes array nullability doesn't get computed correctly for a conditional expression - // on the RHS of an assignment. So, look at the type of the assignment tree. - TreePath parentPath = state.getPath().getParentPath(); + ConditionalExpressionTree tree, VisitorState state, boolean calledFromDataflow) { + Type cachedType = inferredPolyExpressionTypes.get(tree); + if (cachedType != null) { + return cachedType; + } + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForConditionalExpression(tree, state, calledFromDataflow); + Type typeFromAssignmentContext = targetTypeAndAssignmentKind.typeFromAssignmentContext(); + return typeFromAssignmentContext != null + ? typeFromAssignmentContext + : typeOrNullIfRaw(ASTHelpers.getType(tree)); + } + + private @Nullable Type inferConditionalExpressionType( + VisitorState state, + ConditionalExpressionTree tree, + TreePath path, + @Nullable Type typeFromAssignmentContext, + boolean calledFromDataflow) { + Type cachedType = inferredPolyExpressionTypes.get(tree); + if (cachedType != null) { + return cachedType; + } + boolean hasTargetType = typeFromAssignmentContext != null; + Type condExprType = typeFromAssignmentContext; + if (condExprType == null) { + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForConditionalExpression(tree, state.withPath(path), calledFromDataflow); + condExprType = targetTypeAndAssignmentKind.typeFromAssignmentContext(); + hasTargetType = condExprType != null; + } + if (condExprType == null) { + condExprType = typeOrNullIfRaw(ASTHelpers.getType(tree)); + } + if (condExprType == null || condExprType.isRaw()) { + return null; + } + if (hasTargetType && !calledFromDataflow) { + inferredPolyExpressionTypes.put(tree, condExprType); + } + return condExprType; + } + + private record TargetTypeAndAssignmentKind( + @Nullable Type typeFromAssignmentContext, boolean assignedToLocal) {} + + private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( + ConditionalExpressionTree tree, VisitorState state, boolean calledFromDataflow) { + TreePath conditionalPath = + Objects.equals(state.getPath().getLeaf(), tree) + ? state.getPath() + : pathWithLeaf(state.getPath(), tree); + conditionalPath = getOutermostConditionalExpressionPath(conditionalPath); + TreePath parentPath = conditionalPath.getParentPath(); + if (parentPath == null) { + return new TargetTypeAndAssignmentKind(null, false); + } Tree parent = parentPath.getLeaf(); while (parent instanceof ParenthesizedTree) { parentPath = parentPath.getParentPath(); + if (parentPath == null) { + return new TargetTypeAndAssignmentKind(null, false); + } parent = parentPath.getLeaf(); } + VisitorState parentState = state.withPath(parentPath); if (parent instanceof AssignmentTree || parent instanceof VariableTree) { - return getTreeType(parent, state.withPath(parentPath)); + return new TargetTypeAndAssignmentKind( + getTreeType(parent, parentState, calledFromDataflow), + isAssignmentToLocalVariable(parent)); + } + if (parent instanceof ReturnTree) { + TreePath enclosingMethodOrLambda = + NullabilityUtil.findEnclosingMethodOrLambdaOrInitializer(parentPath); + if (enclosingMethodOrLambda != null + && enclosingMethodOrLambda.getLeaf() instanceof MethodTree enclosingMethod) { + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(enclosingMethod); + if (methodSymbol != null) { + return new TargetTypeAndAssignmentKind(methodSymbol.getReturnType(), false); + } + } + return new TargetTypeAndAssignmentKind(null, false); + } + if (parent instanceof MethodInvocationTree parentInvocation) { + if (isGenericCallNeedingInference(parentInvocation)) { + return new TargetTypeAndAssignmentKind(null, false); + } + Type methodType = ASTHelpers.getType(parentInvocation.getMethodSelect()); + if (methodType == null) { + return new TargetTypeAndAssignmentKind(null, false); + } + return new TargetTypeAndAssignmentKind( + getFormalParameterTypeForArgument( + parentInvocation, methodType.asMethodType(), conditionalPath.getLeaf()), + false); + } + if (parent instanceof NewClassTree parentConstructorCall) { + Type parentClassType = getTreeType(parentConstructorCall, parentState, calledFromDataflow); + if (parentClassType != null) { + Symbol parentCtorSymbol = ASTHelpers.getSymbol(parentConstructorCall); + Type parentCtorType = + TypeSubstitutionUtils.memberType( + state.getTypes(), parentClassType, parentCtorSymbol, config); + return new TargetTypeAndAssignmentKind( + getFormalParameterTypeForArgument( + parentConstructorCall, parentCtorType.asMethodType(), conditionalPath.getLeaf()), + false); + } + } + return new TargetTypeAndAssignmentKind(null, false); + } + + private TreePath getOutermostConditionalExpressionPath(TreePath path) { + TreePath conditionalPath = path; + TreePath parentPath = conditionalPath.getParentPath(); + while (parentPath != null) { + Tree parent = parentPath.getLeaf(); + while (parent instanceof ParenthesizedTree) { + parentPath = parentPath.getParentPath(); + if (parentPath == null) { + return conditionalPath; + } + parent = parentPath.getLeaf(); + } + if (!(parent instanceof ConditionalExpressionTree)) { + return conditionalPath; + } + conditionalPath = parentPath; + parentPath = conditionalPath.getParentPath(); } - return getTreeType(tree, state); + return conditionalPath; } /** @@ -1968,6 +2151,15 @@ public void compareGenericTypeParameterNullabilityForCall( formalParameter, false, false); + } else if (currentActualParam + instanceof ConditionalExpressionTree conditionalExpressionTree) { + actualParameterType = + inferConditionalExpressionType( + state.withPath(pathToParam), + conditionalExpressionTree, + pathToParam, + formalParameter, + false); } if (!subtypeParameterNullability(formalParameter, actualParameterType, state)) { reportInvalidParametersNullabilityError( @@ -2399,6 +2591,17 @@ private InvocationAndContext getInvocationAndContextForInference( } } return new InvocationAndContext(invocation, formalParamType, false); + } else if (exprParent instanceof ConditionalExpressionTree) { + TreePath conditionalPath = getOutermostConditionalExpressionPath(parentPath); + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForConditionalExpression( + (ConditionalExpressionTree) conditionalPath.getLeaf(), + state.withPath(conditionalPath), + calledFromDataflow); + return new InvocationAndContext( + invocation, + targetTypeAndAssignmentKind.typeFromAssignmentContext(), + targetTypeAndAssignmentKind.assignedToLocal()); } } // an unhandled case; for now, give up and return no assignment context diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java index 5839dc9afb..790af829b7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -32,6 +32,223 @@ static ClassLike fallback() { .doTest(); } + @Test + public void genericMethodConditionalArmWithFieldAssignmentContext() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + String nonNullField = "hello"; + @Nullable String nullableField; + static T id(T t) { + return t; + } + void test(boolean flag) { + nullableField = flag ? id(null) : "fallback"; + // BUG: Diagnostic contains: passing @Nullable parameter + nonNullField = flag ? id(null) : "fallback"; + nonNullField = flag ? id("value") : "fallback"; + } + } + """) + .doTest(); + } + + @Test + public void genericMethodConditionalBothArmsWithFieldAssignmentContext() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + String nonNullField = "hello"; + @Nullable String nullableField; + static T id(T t) { + return t; + } + void test(boolean flag) { + nullableField = flag ? id(null) : id("fallback"); + // BUG: Diagnostic contains: passing @Nullable parameter + nonNullField = flag ? id(null) : id("fallback"); + nonNullField = flag ? id("value") : id("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void nestedGenericMethodConditionalWithFieldAssignmentContext() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + String nonNullField = "hello"; + @Nullable String nullableField; + static T id(T t) { + return t; + } + void test(boolean first, boolean second) { + nullableField = first ? id(null) : second ? id("value") : "fallback"; + // BUG: Diagnostic contains: passing @Nullable parameter + nonNullField = first ? id(null) : second ? id("value") : "fallback"; + nonNullField = first ? id("first") : second ? id("second") : "fallback"; + } + } + """) + .doTest(); + } + + @Test + public void genericMethodConditionalAsMethodArgument() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static T id(T t) { + return t; + } + static void takesNonNull(String s) {} + static void takesNullable(@Nullable String s) {} + void test(boolean flag) { + takesNullable(flag ? id(null) : "fallback"); + // BUG: Diagnostic contains: passing @Nullable parameter + takesNonNull(flag ? id(null) : "fallback"); + takesNonNull(flag ? id("value") : "fallback"); + } + } + """) + .doTest(); + } + + @Test + public void conditionalInfersGenericTypeArguments() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box {} + static Box box(T t) { + return new Box(); + } + void test(boolean flag) { + Box<@Nullable String> nullableBox = flag ? box(null) : box("fallback"); + // BUG: Diagnostic contains: passing @Nullable parameter + Box nonNullBox = flag ? box(null) : box("fallback"); + Box nonNullBoxOk = flag ? box("value") : box("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void nestedConditionalInfersGenericTypeArguments() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box {} + static Box box(T t) { + return new Box(); + } + void test(boolean first, boolean second) { + Box<@Nullable String> nullableBox = + first ? box(null) : second ? box("value") : box("fallback"); + Box nonNullBox = + // BUG: Diagnostic contains: passing @Nullable parameter + first ? box(null) : second ? box("value") : box("fallback"); + Box nonNullBoxOk = + first ? box("first") : second ? box("second") : box("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void conditionalInfersDiamondTypeArguments() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box { + Box(T t) {} + } + void test(boolean flag) { + Box<@Nullable String> nullableBox = + flag ? new Box<>(null) : new Box<>("fallback"); + Box nonNullBox = + // BUG: Diagnostic contains: passing @Nullable parameter + flag ? new Box<>(null) : new Box<>("fallback"); + Box nonNullBoxOk = + flag ? new Box<>("value") : new Box<>("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void nestedConditionalInfersDiamondTypeArguments() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box { + Box(T t) {} + } + void test(boolean first, boolean second) { + Box<@Nullable String> nullableBox = + first + ? new Box<>(null) + : second ? new Box<>("value") : new Box<>("fallback"); + Box nonNullBox = + first + // BUG: Diagnostic contains: passing @Nullable parameter + ? new Box<>(null) + : second ? new Box<>("value") : new Box<>("fallback"); + Box nonNullBoxOk = + first + ? new Box<>("first") + : second ? new Box<>("second") : new Box<>("fallback"); + } + } + """) + .doTest(); + } + private CompilationTestHelper makeHelperWithInferenceFailureWarning() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From 8390665367fd86d804bbff2526b6f5b537040bb2 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 15 Jun 2026 10:07:40 -0700 Subject: [PATCH 04/13] more --- .../nullaway/generics/GenericsChecks.java | 14 ++- .../jspecify/ConditionalExprTests.java | 104 ++++++++++++++++++ 2 files changed, 116 insertions(+), 2 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 f4e2c09750..947f9db42d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1880,12 +1880,22 @@ public void checkTypeParameterNullnessForConditionalExpression( condExprType == null ? getTreeType(truePartTree, state.withPath(pathToTruePart)) : getTypeForRhsOfAssignment( - tree.getTrueExpression(), pathToTruePart, condExprType, false, state, false); + tree.getTrueExpression(), + pathToTruePart, + condExprType, + targetTypeAndAssignmentKind.assignedToLocal(), + state, + false); Type falsePartType = condExprType == null ? getTreeType(falsePartTree, state.withPath(pathToFalsePart)) : getTypeForRhsOfAssignment( - tree.getFalseExpression(), pathToFalsePart, condExprType, false, state, false); + tree.getFalseExpression(), + pathToFalsePart, + condExprType, + targetTypeAndAssignmentKind.assignedToLocal(), + state, + false); // The condExpr type should be the least-upper bound of the true and false part types. To check // the nullability annotations, we check that the true and false parts are assignable to the // type of the whole expression diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java index 790af829b7..5344a6f904 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -249,6 +249,110 @@ void test(boolean first, boolean second) { .doTest(); } + @Test + public void conditionalGenericMethodInferenceWithDataflow() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + String nonNullField = "hello"; + @Nullable String nullableField; + static T id(T t) { + return t; + } + void test(boolean flag, @Nullable String s) { + if (s != null) { + nonNullField = flag ? id(s) : id("fallback"); + } + nullableField = flag ? id(s) : id("fallback"); + // BUG: Diagnostic contains: passing @Nullable parameter + nonNullField = flag ? id(s) : id("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void conditionalGenericTypeArgumentInferenceWithDataflow() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box {} + static Box box(T t) { + return new Box(); + } + void test(boolean flag, @Nullable String s) { + if (s != null) { + Box nonNullBox = flag ? box(s) : box("fallback"); + } + Box<@Nullable String> nullableBox = flag ? box(s) : box("fallback"); + // BUG: Diagnostic contains: passing @Nullable parameter + Box bad = flag ? box(s) : box("fallback"); + } + } + """) + .doTest(); + } + + @Test + public void conditionalGenericMethodInferenceDataflowAndLoops() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static T id(T t) { + return t; + } + void testLoop1(boolean flag) { + String s = "hello"; + while (true) { + String t = flag ? id(s) : id("fallback"); + // BUG: Diagnostic contains: dereferenced expression t is @Nullable + t.hashCode(); + s = null; + } + } + void testLoop2(boolean flag) { + String t = "hello"; + while (true) { + // BUG: Diagnostic contains: dereferenced expression t is @Nullable + t.hashCode(); + String s = null; + t = flag ? id(s) : id("fallback"); + } + } + void testLoop3(boolean flag) { + String t = "hello"; + String s = "hello"; + t.hashCode(); + int i = 2; + while (i > 0) { + t = flag ? id(s) : id("fallback"); + s = null; + i--; + } + // BUG: Diagnostic contains: dereferenced expression t is @Nullable + t.hashCode(); + } + } + """) + .doTest(); + } + private CompilationTestHelper makeHelperWithInferenceFailureWarning() { return makeTestHelperWithArgs( JSpecifyJavacConfig.withJSpecifyModeArgs( From 14ba0f3fbfb046eb381cb9194296803c4d9a0875 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 15 Jun 2026 10:20:17 -0700 Subject: [PATCH 05/13] fix nullaway warnings --- .../java/com/uber/nullaway/generics/GenericsChecks.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 947f9db42d..997f1503f2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1766,6 +1766,9 @@ public void checkTypeParameterNullnessForFunctionReturnType( formalReturnType, false); } + if (returnExpressionType == null) { + return; + } boolean isReturnTypeValid = subtypeParameterNullability(formalReturnType, returnExpressionType, state); if (!isReturnTypeValid) { @@ -2171,6 +2174,9 @@ public void compareGenericTypeParameterNullabilityForCall( formalParameter, false); } + if (actualParameterType == null) { + return; + } if (!subtypeParameterNullability(formalParameter, actualParameterType, state)) { reportInvalidParametersNullabilityError( formalParameter, actualParameterType, currentActualParam, state); From 70795e5df263539bf10833eab680757e9174f713 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 14:41:40 -0700 Subject: [PATCH 06/13] fix test failure --- .../com/uber/nullaway/jspecify/ConditionalExprTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java index 5344a6f904..98cc44bf90 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -321,7 +321,7 @@ void testLoop1(boolean flag) { String s = "hello"; while (true) { String t = flag ? id(s) : id("fallback"); - // BUG: Diagnostic contains: dereferenced expression t is @Nullable + // BUG: Diagnostic contains: dereferenced expression 't' is @Nullable t.hashCode(); s = null; } @@ -329,7 +329,7 @@ void testLoop1(boolean flag) { void testLoop2(boolean flag) { String t = "hello"; while (true) { - // BUG: Diagnostic contains: dereferenced expression t is @Nullable + // BUG: Diagnostic contains: dereferenced expression 't' is @Nullable t.hashCode(); String s = null; t = flag ? id(s) : id("fallback"); @@ -345,7 +345,7 @@ void testLoop3(boolean flag) { s = null; i--; } - // BUG: Diagnostic contains: dereferenced expression t is @Nullable + // BUG: Diagnostic contains: dereferenced expression 't' is @Nullable t.hashCode(); } } From 8c23a1540fc31e53a35220d9742aae65d5ffbfed Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 14:56:31 -0700 Subject: [PATCH 07/13] cleanup and remove redundancy --- .../nullaway/generics/GenericsChecks.java | 141 +++++++----------- .../jspecify/ConditionalExprTests.java | 62 ++++++++ .../jspecify/GenericDiamondTests.java | 30 ++++ .../jspecify/VarDeclaredLocalTests.java | 30 ++++ 4 files changed, 172 insertions(+), 91 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 997f1503f2..488d60c2d8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -756,67 +756,9 @@ private void reportInvalidOverridingMethodParamTypeError( */ private @Nullable Type getDiamondTypeFromParentContext( NewClassTree tree, VisitorState state, TreePath parentPath, boolean calledFromDataflow) { - Tree parent = parentPath.getLeaf(); - while (parent instanceof ParenthesizedTree) { - parentPath = parentPath.getParentPath(); - if (parentPath == null) { - return null; - } - parent = parentPath.getLeaf(); - } - if (parent instanceof VariableTree || parent instanceof AssignmentTree) { - return getTreeType(parent, state.withPath(parentPath), calledFromDataflow); - } - if (parent instanceof ReturnTree) { - TreePath enclosingMethodOrLambda = - NullabilityUtil.findEnclosingMethodOrLambdaOrInitializer(parentPath); - if (enclosingMethodOrLambda != null - && enclosingMethodOrLambda.getLeaf() instanceof MethodTree enclosingMethod) { - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(enclosingMethod); - if (methodSymbol != null) { - return methodSymbol.getReturnType(); - } - } - return null; - } - if (parent instanceof MethodInvocationTree parentInvocation) { - if (isGenericCallNeedingInference(parentInvocation)) { - // TODO support full integration of diamond constructor calls with generic method inference - // https://github.com/uber/NullAway/issues/1470 - // for now, just give up and return null - return null; - } - Type methodType = ASTHelpers.getType(parentInvocation.getMethodSelect()); - if (methodType == null) { - return null; - } - return getFormalParameterTypeForArgument(parentInvocation, methodType.asMethodType(), tree); - } - if (parent instanceof NewClassTree parentConstructorCall) { - // get the type returned by the parent constructor call - Type parentClassType = - getTreeType(parentConstructorCall, state.withPath(parentPath), calledFromDataflow); - if (parentClassType != null) { - Symbol parentCtorSymbol = ASTHelpers.getSymbol(parentConstructorCall); - // get the proper type for the constructor, as a member of the type returned by the - // constructor - Type parentCtorType = - TypeSubstitutionUtils.memberType( - state.getTypes(), parentClassType, parentCtorSymbol, config); - return getFormalParameterTypeForArgument( - parentConstructorCall, parentCtorType.asMethodType(), tree); - } - } - if (parent instanceof ConditionalExpressionTree) { - TreePath conditionalPath = getOutermostConditionalExpressionPath(parentPath); - TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = - getTargetTypeForConditionalExpression( - (ConditionalExpressionTree) conditionalPath.getLeaf(), - state.withPath(conditionalPath), - calledFromDataflow); - return targetTypeAndAssignmentKind.typeFromAssignmentContext(); - } - return null; + return getTargetTypeFromParentContext( + tree, new TreePath(parentPath, tree), state, calledFromDataflow) + .typeFromAssignmentContext(); } /** @@ -1969,8 +1911,14 @@ private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( Objects.equals(state.getPath().getLeaf(), tree) ? state.getPath() : pathWithLeaf(state.getPath(), tree); - conditionalPath = getOutermostConditionalExpressionPath(conditionalPath); - TreePath parentPath = conditionalPath.getParentPath(); + return getTargetTypeFromParentContext(tree, conditionalPath, state, calledFromDataflow); + } + + private TargetTypeAndAssignmentKind getTargetTypeFromParentContext( + Tree tree, TreePath path, VisitorState state, boolean calledFromDataflow) { + Tree expressionTree = tree; + TreePath expressionPath = path; + TreePath parentPath = expressionPath.getParentPath(); if (parentPath == null) { return new TargetTypeAndAssignmentKind(null, false); } @@ -1982,11 +1930,25 @@ private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( } parent = parentPath.getLeaf(); } + if (parent instanceof ConditionalExpressionTree) { + expressionPath = getOutermostConditionalExpressionPath(parentPath); + expressionTree = expressionPath.getLeaf(); + parentPath = expressionPath.getParentPath(); + if (parentPath == null) { + return new TargetTypeAndAssignmentKind(null, false); + } + parent = parentPath.getLeaf(); + while (parent instanceof ParenthesizedTree) { + parentPath = parentPath.getParentPath(); + if (parentPath == null) { + return new TargetTypeAndAssignmentKind(null, false); + } + parent = parentPath.getLeaf(); + } + } VisitorState parentState = state.withPath(parentPath); if (parent instanceof AssignmentTree || parent instanceof VariableTree) { - return new TargetTypeAndAssignmentKind( - getTreeType(parent, parentState, calledFromDataflow), - isAssignmentToLocalVariable(parent)); + return getTargetTypeForAssignmentContext(parent, parentState, calledFromDataflow); } if (parent instanceof ReturnTree) { TreePath enclosingMethodOrLambda = @@ -2010,7 +1972,7 @@ private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( } return new TargetTypeAndAssignmentKind( getFormalParameterTypeForArgument( - parentInvocation, methodType.asMethodType(), conditionalPath.getLeaf()), + parentInvocation, methodType.asMethodType(), expressionTree), false); } if (parent instanceof NewClassTree parentConstructorCall) { @@ -2022,13 +1984,27 @@ private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( state.getTypes(), parentClassType, parentCtorSymbol, config); return new TargetTypeAndAssignmentKind( getFormalParameterTypeForArgument( - parentConstructorCall, parentCtorType.asMethodType(), conditionalPath.getLeaf()), + parentConstructorCall, parentCtorType.asMethodType(), expressionTree), false); } } return new TargetTypeAndAssignmentKind(null, false); } + private TargetTypeAndAssignmentKind getTargetTypeForAssignmentContext( + Tree assignmentOrVariable, VisitorState state, boolean calledFromDataflow) { + Preconditions.checkArgument( + assignmentOrVariable instanceof AssignmentTree + || assignmentOrVariable instanceof VariableTree); + Type targetType = + assignmentOrVariable instanceof VariableTree variableTree + && isVarLocalVariableDeclaration(variableTree) + ? null + : getTreeType(assignmentOrVariable, state, calledFromDataflow); + return new TargetTypeAndAssignmentKind( + targetType, isAssignmentToLocalVariable(assignmentOrVariable)); + } + private TreePath getOutermostConditionalExpressionPath(TreePath path) { TreePath conditionalPath = path; TreePath parentPath = conditionalPath.getParentPath(); @@ -2545,8 +2521,12 @@ private InvocationAndContext getInvocationAndContextForInference( parent = parentPath.getLeaf(); } if (parent instanceof AssignmentTree || parent instanceof VariableTree) { - return getInvocationInferenceInfoForAssignment( - parent, invocation, state.withPath(parentPath), calledFromDataflow); + TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = + getTargetTypeForAssignmentContext(parent, state.withPath(parentPath), calledFromDataflow); + return new InvocationAndContext( + invocation, + targetTypeAndAssignmentKind.typeFromAssignmentContext(), + targetTypeAndAssignmentKind.assignedToLocal()); } else if (parent instanceof ReturnTree) { // find the enclosing method and return its return type TreePath enclosingMethodOrLambda = @@ -2624,27 +2604,6 @@ private InvocationAndContext getInvocationAndContextForInference( return new InvocationAndContext(invocation, null, false); } - private InvocationAndContext getInvocationInferenceInfoForAssignment( - Tree assignment, - MethodInvocationTree invocation, - VisitorState state, - boolean calledFromDataflow) { - Preconditions.checkArgument( - assignment instanceof AssignmentTree || assignment instanceof VariableTree); - TreePath path = state.getPath(); - @SuppressWarnings("ReferenceEquality") // deliberate reference equality check - boolean leafIsAssignment = path.getLeaf() != assignment; - if (leafIsAssignment) { - state = state.withPath(pathWithLeaf(path, assignment)); - } - Type treeType = - assignment instanceof VariableTree variableTree - && isVarLocalVariableDeclaration(variableTree) - ? null // no info from assignment context if declared with `var` - : getTreeType(assignment, state, calledFromDataflow); - return new InvocationAndContext(invocation, treeType, isAssignmentToLocalVariable(assignment)); - } - /** * Computes the nullness of a formal parameter of a generic method at an invocation, in the * context of the declared type of its receiver argument. If the formal parameter's type is a type diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java index 98cc44bf90..2771244df1 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -161,6 +161,35 @@ void test(boolean flag) { .doTest(); } + @Test + public void varConditionalDoesNotProvideTargetTypeForGenericMethodInference() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + interface Box { + T get(); + } + static Box box(T t) { + throw new RuntimeException(); + } + void test(boolean flag) { + var inferredFromInitializer = flag ? box(null) : box("fallback"); + Box<@Nullable String> explicitNullableTarget = + flag ? box(null) : box("fallback"); + Box explicitNonNullTarget = + // BUG: Diagnostic contains: passing @Nullable parameter + flag ? box(null) : box("fallback"); + } + } + """) + .doTest(); + } + @Test public void nestedConditionalInfersGenericTypeArguments() { makeHelperWithInferenceFailureWarning() @@ -216,6 +245,39 @@ void test(boolean flag) { .doTest(); } + @Test + public void varConditionalDoesNotProvideTargetTypeForDiamondInference() { + makeHelperWithInferenceFailureWarning() + .addSourceLines( + "Test.java", + """ + package com.example; + import org.jspecify.annotations.*; + @NullMarked + final class Test { + static final class Box { + private final T value; + Box(T value) { + this.value = value; + } + T get() { + return value; + } + } + void test(boolean flag) { + // BUG: Diagnostic contains: passing @Nullable parameter + var inferredFromInitializer = flag ? new Box<>(null) : new Box<>("fallback"); + Box<@Nullable String> explicitNullableTarget = + flag ? new Box<>(null) : new Box<>("fallback"); + Box explicitNonNullTarget = + // BUG: Diagnostic contains: passing @Nullable parameter + flag ? new Box<>(null) : new Box<>("fallback"); + } + } + """) + .doTest(); + } + @Test public void nestedConditionalInfersDiamondTypeArguments() { makeHelperWithInferenceFailureWarning() diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericDiamondTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericDiamondTests.java index 07c4ac7ed5..21d6240888 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericDiamondTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericDiamondTests.java @@ -44,6 +44,36 @@ void testPositive() { .doTest(); } + @Test + public void varLocalDoesNotProvideTargetTypeForDiamondInference() { + makeHelper() + .addSourceLines( + "Test.java", + """ + import org.jspecify.annotations.*; + @NullMarked + public class Test { + static class Box { + private final T value; + Box(T value) { + this.value = value; + } + T get() { + return value; + } + } + void test() { + // BUG: Diagnostic contains: passing @Nullable parameter + var inferredFromInitializer = new Box<>(null); + // BUG: Diagnostic contains: passing @Nullable parameter + Box explicitNonNullTarget = new Box<>(null); + Box<@Nullable String> explicitNullableTarget = new Box<>(null); + } + } + """) + .doTest(); + } + @Test public void returnDiamond() { makeHelper() diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java index 845ce123a2..b31b56e9b3 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java @@ -40,6 +40,36 @@ void test() { .doTest(); } + @Test + public void varLocalDoesNotProvideTargetTypeForGenericMethodInference() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Box { + T get(); + } + static Box box(U u) { + throw new RuntimeException(); + } + void test() { + var inferredFromInitializer = box(null); + // BUG: Diagnostic contains: dereferenced expression 'inferredFromInitializer.get()' is @Nullable + inferredFromInitializer.get().hashCode(); + // BUG: Diagnostic contains: inference failure + Box explicitNonNullTarget = box(null); + Box<@Nullable String> explicitNullableTarget = box(null); + } + } + """) + .doTest(); + } + @Test public void varLocalReassigned() { makeHelper() From ea3068139ebb9239d554f42f79b57fff9ccb89ed Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 15:07:36 -0700 Subject: [PATCH 08/13] improve caching --- .../com/uber/nullaway/generics/GenericsChecks.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 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 488d60c2d8..90cd0ec15e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -111,8 +111,8 @@ private static final class InferenceFailure implements MethodInferenceResult { inferredTypeVarNullabilityForGenericCalls = new LinkedHashMap<>(); /** - * Maps each poly expression ({@code LambdaExpressionTree} or {@code MemberReferenceTree}) passed - * as a parameter to a generic method to its inferred type, if inference succeeded. + * Maps poly expressions for which we have computed a context-derived type to that type, if + * inference succeeded. */ private final Map inferredPolyExpressionTypes = new LinkedHashMap<>(); @@ -1867,9 +1867,13 @@ public void checkTypeParameterNullnessForConditionalExpression( TargetTypeAndAssignmentKind targetTypeAndAssignmentKind = getTargetTypeForConditionalExpression(tree, state, calledFromDataflow); Type typeFromAssignmentContext = targetTypeAndAssignmentKind.typeFromAssignmentContext(); - return typeFromAssignmentContext != null - ? typeFromAssignmentContext - : typeOrNullIfRaw(ASTHelpers.getType(tree)); + if (typeFromAssignmentContext != null) { + if (!calledFromDataflow) { + inferredPolyExpressionTypes.put(tree, typeFromAssignmentContext); + } + return typeFromAssignmentContext; + } + return typeOrNullIfRaw(ASTHelpers.getType(tree)); } private @Nullable Type inferConditionalExpressionType( From 232347efc8531e6223222281f5dde86dbc5d3f30 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 18:06:57 -0700 Subject: [PATCH 09/13] add a comment --- .../main/java/com/uber/nullaway/generics/GenericsChecks.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 90cd0ec15e..73d4e36e95 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -602,6 +602,10 @@ private void reportInvalidOverridingMethodParamTypeError( || tree instanceof ConditionalExpressionTree) { Type result = inferredPolyExpressionTypes.get(tree); if (result == null) { + // For lambda and method ref poly expressions, their nullness-enhanced type should have + // already been cached via an inference call from a parent tree. In contrast, the + // nullness-enhanced type of a conditional expression tree may not have been computed, so we + // invoke getConditionalExpressionType here to do so result = tree instanceof ConditionalExpressionTree conditionalExpressionTree ? getConditionalExpressionType(conditionalExpressionTree, state, calledFromDataflow) From 478cf64cb3859afa6165ee4c7b3148b5594ca472 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 18:13:17 -0700 Subject: [PATCH 10/13] more docs --- .../nullaway/generics/GenericsChecks.java | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 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 73d4e36e95..e2e9fd18f6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1922,6 +1922,17 @@ private TargetTypeAndAssignmentKind getTargetTypeForConditionalExpression( return getTargetTypeFromParentContext(tree, conditionalPath, state, calledFromDataflow); } + /** + * Returns the target type supplied by the parent context for an expression, if one is available. + * + *

Handles assignment and variable declaration contexts, method and constructor arguments, + * return expressions, and expressions nested inside conditional expressions. If {@code path} is + * inside a nested conditional expression, this method first climbs to the outermost conditional + * expression, since the surrounding target type applies to that expression as a whole. + * + *

A {@code var}-declared local variable does not provide a target type; its type is inferred + * from the initializer. + */ private TargetTypeAndAssignmentKind getTargetTypeFromParentContext( Tree tree, TreePath path, VisitorState state, boolean calledFromDataflow) { Tree expressionTree = tree; @@ -1933,24 +1944,15 @@ private TargetTypeAndAssignmentKind getTargetTypeFromParentContext( Tree parent = parentPath.getLeaf(); while (parent instanceof ParenthesizedTree) { parentPath = parentPath.getParentPath(); - if (parentPath == null) { - return new TargetTypeAndAssignmentKind(null, false); - } parent = parentPath.getLeaf(); } if (parent instanceof ConditionalExpressionTree) { expressionPath = getOutermostConditionalExpressionPath(parentPath); expressionTree = expressionPath.getLeaf(); parentPath = expressionPath.getParentPath(); - if (parentPath == null) { - return new TargetTypeAndAssignmentKind(null, false); - } parent = parentPath.getLeaf(); while (parent instanceof ParenthesizedTree) { parentPath = parentPath.getParentPath(); - if (parentPath == null) { - return new TargetTypeAndAssignmentKind(null, false); - } parent = parentPath.getLeaf(); } } @@ -1972,6 +1974,10 @@ private TargetTypeAndAssignmentKind getTargetTypeFromParentContext( } if (parent instanceof MethodInvocationTree parentInvocation) { if (isGenericCallNeedingInference(parentInvocation)) { + // The parent invocation's formal parameter type is still part of the inference problem, not + // a solved target type. Generic method inference will handle this expression from the + // parent + // call side. return new TargetTypeAndAssignmentKind(null, false); } Type methodType = ASTHelpers.getType(parentInvocation.getMethodSelect()); @@ -1999,6 +2005,13 @@ private TargetTypeAndAssignmentKind getTargetTypeFromParentContext( return new TargetTypeAndAssignmentKind(null, false); } + /** + * Returns target-type information for an assignment or variable declaration context. + * + *

For explicit variable declarations and assignments, the assigned location's type is the + * target type. For {@code var}-declared locals, no target type is returned because the local's + * type is inferred from the initializer. + */ private TargetTypeAndAssignmentKind getTargetTypeForAssignmentContext( Tree assignmentOrVariable, VisitorState state, boolean calledFromDataflow) { Preconditions.checkArgument( @@ -2013,6 +2026,13 @@ && isVarLocalVariableDeclaration(variableTree) targetType, isAssignmentToLocalVariable(assignmentOrVariable)); } + /** + * Returns the outermost conditional expression enclosing {@code path}, ignoring parentheses + * between nested conditionals. + * + *

If {@code path} is not enclosed by another conditional expression, returns {@code path} + * itself. + */ private TreePath getOutermostConditionalExpressionPath(TreePath path) { TreePath conditionalPath = path; TreePath parentPath = conditionalPath.getParentPath(); @@ -2020,9 +2040,6 @@ private TreePath getOutermostConditionalExpressionPath(TreePath path) { Tree parent = parentPath.getLeaf(); while (parent instanceof ParenthesizedTree) { parentPath = parentPath.getParentPath(); - if (parentPath == null) { - return conditionalPath; - } parent = parentPath.getLeaf(); } if (!(parent instanceof ConditionalExpressionTree)) { From c3313a927fd4fa3d647fc9a4c8b2da7f52a2bc5f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 18:20:55 -0700 Subject: [PATCH 11/13] various cleanups --- .../nullaway/generics/GenericsChecks.java | 70 ++++++++++++------- 1 file changed, 43 insertions(+), 27 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 e2e9fd18f6..e4eb77c433 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -1281,6 +1281,8 @@ private void generateConstraintsForPseudoAssignment( allInvocations, calledFromDataflow); } else if (rhsExpr instanceof ConditionalExpressionTree conditionalExpressionTree) { + // generate constraints for both the true and false sub-expressions of the conditional + // expression ExpressionTree trueExpression = conditionalExpressionTree.getTrueExpression(); TreePath pathToTrueExpression = new TreePath(state.getPath(), trueExpression); generateConstraintsForPseudoAssignment( @@ -1823,45 +1825,50 @@ public void checkTypeParameterNullnessForConditionalExpression( if (!hasTargetType) { return; } + if (condExprType == null) { + return; + } TreePath pathToTruePart = pathWithLeaf(state.getPath(), truePartTree); TreePath pathToFalsePart = pathWithLeaf(state.getPath(), falsePartTree); Type truePartType = - condExprType == null - ? getTreeType(truePartTree, state.withPath(pathToTruePart)) - : getTypeForRhsOfAssignment( - tree.getTrueExpression(), - pathToTruePart, - condExprType, - targetTypeAndAssignmentKind.assignedToLocal(), - state, - false); + getTypeForRhsOfAssignment( + tree.getTrueExpression(), + pathToTruePart, + condExprType, + targetTypeAndAssignmentKind.assignedToLocal(), + state, + false); Type falsePartType = - condExprType == null - ? getTreeType(falsePartTree, state.withPath(pathToFalsePart)) - : getTypeForRhsOfAssignment( - tree.getFalseExpression(), - pathToFalsePart, - condExprType, - targetTypeAndAssignmentKind.assignedToLocal(), - state, - false); + getTypeForRhsOfAssignment( + tree.getFalseExpression(), + pathToFalsePart, + condExprType, + targetTypeAndAssignmentKind.assignedToLocal(), + state, + false); // The condExpr type should be the least-upper bound of the true and false part types. To check // the nullability annotations, we check that the true and false parts are assignable to the // type of the whole expression - if (condExprType != null) { - if (truePartType != null) { - if (!subtypeParameterNullability(condExprType, truePartType, state)) { - reportMismatchedTypeForTernaryOperator(truePartTree, condExprType, truePartType, state); - } + if (truePartType != null) { + if (!subtypeParameterNullability(condExprType, truePartType, state)) { + reportMismatchedTypeForTernaryOperator(truePartTree, condExprType, truePartType, state); } - if (falsePartType != null) { - if (!subtypeParameterNullability(condExprType, falsePartType, state)) { - reportMismatchedTypeForTernaryOperator(falsePartTree, condExprType, falsePartType, state); - } + } + if (falsePartType != null) { + if (!subtypeParameterNullability(condExprType, falsePartType, state)) { + reportMismatchedTypeForTernaryOperator(falsePartTree, condExprType, falsePartType, state); } } } + /** + * Returns the type to use for a conditional expression. + * + *

If a target/contextual type for the conditional expression has already been cached, returns + * it. Otherwise, this method tries to recover a target type from the conditional expression's + * parent context and caches it, unless called from dataflow. If no target type is available, + * falls back to javac's type for the conditional expression. + */ private @Nullable Type getConditionalExpressionType( ConditionalExpressionTree tree, VisitorState state, boolean calledFromDataflow) { Type cachedType = inferredPolyExpressionTypes.get(tree); @@ -1880,6 +1887,15 @@ public void checkTypeParameterNullnessForConditionalExpression( return typeOrNullIfRaw(ASTHelpers.getType(tree)); } + /** + * Infers the type to use for a conditional expression in a pseudo-assignment context. + * + *

If {@code typeFromAssignmentContext} is non-null, it is used as the conditional expression's + * target type. Otherwise, this method tries to recover a target type from the parent context. + * When a target type is found, it is cached unless called from dataflow. If no target type is + * available, this method falls back to javac's type for the conditional expression. Returns + * {@code null} for raw or otherwise unavailable types. + */ private @Nullable Type inferConditionalExpressionType( VisitorState state, ConditionalExpressionTree tree, From 894398a7b42cb4e6bff3a5d7bb3c74b0f1d69b39 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 18:27:10 -0700 Subject: [PATCH 12/13] tweak test names --- .../com/uber/nullaway/generics/JSpecifyJavacConfig.java | 2 +- .../com/uber/nullaway/jspecify/ConditionalExprTests.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java b/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java index a3a2d1094b..62a3740e38 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java @@ -34,7 +34,7 @@ private JSpecifyJavacConfig() {} public static List withJSpecifyModeArgs(List args) { List result = new ArrayList<>(args.size() + JSPECIFY_MODE_ARGS.size()); result.addAll(args); - result.addAll(JSPECIFY_MODE_ARGS); + result.addAll(JSPECIFY_MODE_AGS); return Collections.unmodifiableList(result); } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java index 2771244df1..b735b72dd4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ConditionalExprTests.java @@ -33,7 +33,7 @@ static ClassLike fallback() { } @Test - public void genericMethodConditionalArmWithFieldAssignmentContext() { + public void genericMethodCallInOneArm() { makeHelperWithInferenceFailureWarning() .addSourceLines( "Test.java", @@ -59,7 +59,7 @@ void test(boolean flag) { } @Test - public void genericMethodConditionalBothArmsWithFieldAssignmentContext() { + public void genericMethodCallInBothArms() { makeHelperWithInferenceFailureWarning() .addSourceLines( "Test.java", @@ -85,7 +85,7 @@ void test(boolean flag) { } @Test - public void nestedGenericMethodConditionalWithFieldAssignmentContext() { + public void nestedConditionalExpressions() { makeHelperWithInferenceFailureWarning() .addSourceLines( "Test.java", @@ -111,7 +111,7 @@ void test(boolean first, boolean second) { } @Test - public void genericMethodConditionalAsMethodArgument() { + public void conditionalAsMethodArgument() { makeHelperWithInferenceFailureWarning() .addSourceLines( "Test.java", From 7e0c16b88183add21a9f7f853f94e5805ff90cf6 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 3 Jul 2026 18:27:35 -0700 Subject: [PATCH 13/13] whoops --- .../java/com/uber/nullaway/generics/JSpecifyJavacConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java b/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java index 62a3740e38..a3a2d1094b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/JSpecifyJavacConfig.java @@ -34,7 +34,7 @@ private JSpecifyJavacConfig() {} public static List withJSpecifyModeArgs(List args) { List result = new ArrayList<>(args.size() + JSPECIFY_MODE_ARGS.size()); result.addAll(args); - result.addAll(JSPECIFY_MODE_AGS); + result.addAll(JSPECIFY_MODE_ARGS); return Collections.unmodifiableList(result); }