diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 0cb338df44..8905098c83 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -10,6 +10,7 @@ import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; @@ -150,6 +151,23 @@ private static void reportInvalidReturnTypeError( errorMessage, analysis.buildDescription(tree), state, null)); } + private static void reportMismatchedTypeForTernaryOperator( + Tree tree, Type expressionType, Type subPartType, VisitorState state, NullAway analysis) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE, + String.format( + "Conditional expression must have type " + + expressionType + + " but the sub-expression has type " + + subPartType + + ", which has mismatched nullability of type parameters")); + state.reportMatch( + errorBuilder.createErrorDescription( + errorMessage, analysis.buildDescription(tree), state, null)); + } + /** * This method returns the type of the given tree, including any type use annotations. * @@ -366,4 +384,49 @@ private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) type.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgs), type.tsym); return finalType; } + + /** + * For a conditional expression c, check whether the type parameter nullability for each + * sub-expression of c matches the type parameter nullability of c itself. + * + *
Note that the type parameter nullability for c is computed by javac and reflects
+ * what is required of the surrounding context (an assignment, parameter pass, etc.). It is
+ * possible that both sub-expressions of c will have identical type parameter
+ * nullability, but will still not match the type parameter nullability of c itself, due
+ * to requirements from the surrounding context. In such a case, our error messages may be
+ * somewhat confusing; we may want to improve this in the future.
+ *
+ * @param tree A conditional expression tree to check
+ */
+ public void checkTypeParameterNullnessForConditionalExpression(ConditionalExpressionTree tree) {
+ if (!config.isJSpecifyMode()) {
+ return;
+ }
+
+ Tree truePartTree = tree.getTrueExpression();
+ Tree falsePartTree = tree.getFalseExpression();
+
+ Type condExprType = getTreeType(tree);
+ Type truePartType = getTreeType(truePartTree);
+ Type falsePartType = getTreeType(falsePartTree);
+ // 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 instanceof Type.ClassType) {
+ if (truePartType instanceof Type.ClassType) {
+ if (!compareNullabilityAnnotations(
+ (Type.ClassType) condExprType, (Type.ClassType) truePartType)) {
+ reportMismatchedTypeForTernaryOperator(
+ truePartTree, condExprType, truePartType, state, analysis);
+ }
+ }
+ if (falsePartType instanceof Type.ClassType) {
+ if (!compareNullabilityAnnotations(
+ (Type.ClassType) condExprType, (Type.ClassType) falsePartType)) {
+ reportMismatchedTypeForTernaryOperator(
+ falsePartTree, condExprType, falsePartType, state, analysis);
+ }
+ }
+ }
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
index 9bf61ca334..76bb0d3d61 100644
--- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java
+++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -1493,6 +1493,8 @@ public Description matchUnary(UnaryTree tree, VisitorState state) {
public Description matchConditionalExpression(
ConditionalExpressionTree tree, VisitorState state) {
if (withinAnnotatedCode(state)) {
+ new GenericsChecks(state, config, this)
+ .checkTypeParameterNullnessForConditionalExpression(tree);
doUnboxingCheck(state, tree.getCondition());
}
return Description.NO_MATCH;
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
index d78304c18f..e2291d42d3 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -557,6 +557,111 @@ public void genericFunctionReturnTypeMultipleReturnStatementsIfElseBlock() {
.doTest();
}
+ @Test
+ public void genericsChecksForTernaryOperator() {
+ makeHelper()
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.jspecify.annotations.Nullable;",
+ "class Test {",
+ "static class A