diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 8905098c83..0cb338df44 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -10,7 +10,6 @@ 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; @@ -151,23 +150,6 @@ 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. * @@ -384,49 +366,4 @@ 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 76bb0d3d61..9bf61ca334 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1493,8 +1493,6 @@ 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 e2291d42d3..d78304c18f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -557,111 +557,6 @@ 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 { }", - " static A testPositive(A a, boolean t) {", - " // BUG: Diagnostic contains: Conditional expression must have type", - " A<@Nullable String> t1 = t ? new A() : new A<@Nullable String>();", - " // BUG: Diagnostic contains: Conditional expression must have type", - " return t ? new A<@Nullable String>() : new A<@Nullable String>();", - " }", - " static void testPositiveTernaryMethodArgument(boolean t) {", - " // BUG: Diagnostic contains: Conditional expression must have type", - " A a = testPositive(t ? new A() : new A<@Nullable String>(), t);", - " }", - " static A<@Nullable String> testNegative(boolean t) {", - " A<@Nullable String> t1 = t ? new A<@Nullable String>() : new A<@Nullable String>();", - " return t ? new A<@Nullable String>() : new A<@Nullable String>();", - " }", - "}") - .doTest(); - } - - @Test - public void ternaryOperatorComplexSubtyping() { - makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "class Test {", - " static class A {}", - " static class B extends A {}", - " static class C extends A {}", - " static void testPositive(boolean t) {", - " // BUG: Diagnostic contains: Conditional expression must have type", - " A<@Nullable String> t1 = t ? new B<@Nullable String>() : new C();", - " // BUG: Diagnostic contains: Conditional expression must have type", - " A<@Nullable String> t2 = t ? new C() : new B<@Nullable String>();", - " // BUG: Diagnostic contains:Conditional expression must have type", - " A<@Nullable String> t3 = t ? new B() : new C<@Nullable String>();", - " // BUG: Diagnostic contains: Conditional expression must have type", - " A t4 = t ? new B<@Nullable String>() : new C<@Nullable String>();", - " }", - " static void testNegative(boolean t) {", - " A<@Nullable String> t1 = t ? new B<@Nullable String>() : new C<@Nullable String>();", - " A<@Nullable String> t2 = t ? new C<@Nullable String>() : new B<@Nullable String>();", - " A t3 = t ? new C() : new B();", - " }", - "}") - .doTest(); - } - - @Test - public void nestedTernary() { - makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "class Test {", - " static class A {}", - " static class B extends A {}", - " static class C extends A {}", - " static void testPositive(boolean t) {", - " A<@Nullable String> t1 = t ? new C<@Nullable String>() :", - " // BUG: Diagnostic contains: Conditional expression must have type", - " (t ? new B<@Nullable String>() : new A());", - " }", - " static void testNegative(boolean t) {", - " A<@Nullable String> t1 = t ? new C<@Nullable String>() :", - " (t ? new B<@Nullable String>() : new A<@Nullable String>());", - " }", - "}") - .doTest(); - } - - @Test - public void ternaryMismatchedAssignmentContext() { - makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "class Test {", - "static class A { }", - " static void testPositive(boolean t) {", - " // we get two errors here, one for each sub-expression; perhaps ideally we would report", - " // just one error (that the ternary operator has type A but the assignment LHS", - " // has type A<@Nullable String>), but implementing that check in general is", - " // a bit tricky", - " A<@Nullable String> t1 = t", - " // BUG: Diagnostic contains: Conditional expression must have type", - " ? new A()", - " // BUG: Diagnostic contains: Conditional expression must have type", - " : new A();", - " }", - "}") - .doTest(); - } - private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList(