Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,33 @@ public Void visitDoWhileLoop(DoWhileLoopTree tree, Void p) {
@Override
public Void visitConditionalExpression(ConditionalExpressionTree tree, Void p) {
checkForNullability(tree.getCondition(), CONDITION_NULLABLE);
// If the overall conditional expression has a primitive Java type but one or both
// operand expressions are reference types, then unboxing will occur as part of
// evaluating the conditional. In that case, check the operand(s) for possible
// nullness (unboxing.of.nullable).
AnnotatedTypeMirror type = atypeFactory.getAnnotatedType(tree);
// Only check for unboxing when javac has chosen a primitive underlying type
// for the conditional expression, but one or both operands are non-primitive.
if (type.getKind().isPrimitive()) {
ExpressionTree trueExpr = tree.getTrueExpression();
ExpressionTree falseExpr = tree.getFalseExpression();
boolean trueNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(trueExpr));
boolean falseNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(falseExpr));

if (trueNeedsUnboxing) {
if (!checkForNullability(trueExpr, UNBOXING_OF_NULLABLE)) {
// If we reported an unboxing.of.nullable error for the true arm, stop
// further checking to avoid cascading errors.
return null;
}
}
if (falseNeedsUnboxing) {
if (!checkForNullability(falseExpr, UNBOXING_OF_NULLABLE)) {
return null;
}
}
}
Comment on lines +780 to +805
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Logic is correct; consider using the existing helper method for consistency.

The fix correctly identifies when javac implicitly unboxes a reference-typed operand in a conditional expression with a primitive result type and reports unboxing.of.nullable when appropriate. The early return pattern is consistent with visitTypeCast (lines 509-513).

Minor suggestion: The existing isPrimitive(ExpressionTree) helper method (lines 714-716) could be reused for slightly more readable and consistent code.

♻️ Suggested refactor to use existing helper
     if (type.getKind().isPrimitive()) {
       ExpressionTree trueExpr = tree.getTrueExpression();
       ExpressionTree falseExpr = tree.getFalseExpression();
-      boolean trueNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(trueExpr));
-      boolean falseNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(falseExpr));
+      boolean trueNeedsUnboxing = !isPrimitive(trueExpr);
+      boolean falseNeedsUnboxing = !isPrimitive(falseExpr);
 
       if (trueNeedsUnboxing) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// If the overall conditional expression has a primitive Java type but one or both
// operand expressions are reference types, then unboxing will occur as part of
// evaluating the conditional. In that case, check the operand(s) for possible
// nullness (unboxing.of.nullable).
AnnotatedTypeMirror type = atypeFactory.getAnnotatedType(tree);
// Only check for unboxing when javac has chosen a primitive underlying type
// for the conditional expression, but one or both operands are non-primitive.
if (type.getKind().isPrimitive()) {
ExpressionTree trueExpr = tree.getTrueExpression();
ExpressionTree falseExpr = tree.getFalseExpression();
boolean trueNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(trueExpr));
boolean falseNeedsUnboxing = !TypesUtils.isPrimitive(TreeUtils.typeOf(falseExpr));
if (trueNeedsUnboxing) {
if (!checkForNullability(trueExpr, UNBOXING_OF_NULLABLE)) {
// If we reported an unboxing.of.nullable error for the true arm, stop
// further checking to avoid cascading errors.
return null;
}
}
if (falseNeedsUnboxing) {
if (!checkForNullability(falseExpr, UNBOXING_OF_NULLABLE)) {
return null;
}
}
}
// If the overall conditional expression has a primitive Java type but one or both
// operand expressions are reference types, then unboxing will occur as part of
// evaluating the conditional. In that case, check the operand(s) for possible
// nullness (unboxing.of.nullable).
AnnotatedTypeMirror type = atypeFactory.getAnnotatedType(tree);
// Only check for unboxing when javac has chosen a primitive underlying type
// for the conditional expression, but one or both operands are non-primitive.
if (type.getKind().isPrimitive()) {
ExpressionTree trueExpr = tree.getTrueExpression();
ExpressionTree falseExpr = tree.getFalseExpression();
boolean trueNeedsUnboxing = !isPrimitive(trueExpr);
boolean falseNeedsUnboxing = !isPrimitive(falseExpr);
if (trueNeedsUnboxing) {
if (!checkForNullability(trueExpr, UNBOXING_OF_NULLABLE)) {
// If we reported an unboxing.of.nullable error for the true arm, stop
// further checking to avoid cascading errors.
return null;
}
}
if (falseNeedsUnboxing) {
if (!checkForNullability(falseExpr, UNBOXING_OF_NULLABLE)) {
return null;
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java`
around lines 780 - 805, Replace the explicit primitive checks that call
TypesUtils.isPrimitive(TreeUtils.typeOf(...)) with the class's existing helper
isPrimitive(ExpressionTree) for consistency and readability inside
NullnessVisitor (the conditional-expression handling that computes
trueNeedsUnboxing/falseNeedsUnboxing); keep the rest of the logic intact (still
call checkForNullability on trueExpr/falseExpr with UNBOXING_OF_NULLABLE and
preserve the early-return behavior when checkForNullability returns false) so
only the primitive-test expressions are swapped to use
isPrimitive(ExpressionTree).


return super.visitConditionalExpression(tree, p);
}

Expand Down
16 changes: 16 additions & 0 deletions checker/tests/nullness/Issue6849.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import java.util.*;
import org.checkerframework.checker.nullness.qual.*;

public class Issue6849 {

public static <T> T m(List<T> lst) {
return lst.get(0);
}

public static void main(String[] args) {
List<@Nullable Integer> lst = new LinkedList<>();
lst.add(null);
// :: error: (unboxing.of.nullable)
int y = ((true) ? Issue6849.<@Nullable Integer>m(lst) : 10);
}
}
Loading