Skip to content

Fix: Report unboxing.of.nullable in conditional expressions with primitive result #7433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Suvrat1629 wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix: Report unboxing.of.nullable in conditional expressions with primitive result #7433

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0994c6a
ternary false negative fix
Suvrat1629 Jan 3, 2026
f60011f
change
Suvrat1629 Jan 4, 2026
164f5e6
Merge ../checker-framework-branch-master into false-neg-6849
mernst Mar 3, 2026
9ff5e03
Merge ../checker-framework-branch-master into false-neg-6849
mernst Mar 3, 2026
382b23b
Merge ../checker-framework-branch-master into false-neg-6849
mernst Mar 4, 2026
8ba7018
Put error message key in brackets
mernst Mar 4, 2026
8d51e2f
Merge branch 'master' into false-neg-6849
mernst Apr 22, 2026
c885fc1
Add contributor
mernst Apr 23, 2026
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
27 changes: 27 additions & 0 deletions checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,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 +787 to +798

@coderabbitai coderabbitai Bot Apr 22, 2026

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.

⚠️ Potential issue | 🟡 Minor

Early return after true arm error skips checking the false arm.

If the true arm fails the nullability check, the method returns before checking falseExpr, so a second unboxing error on the false arm is suppressed in the same conditional. visitTypeCast (lines 507–513) has only one operand so the early-return pattern is equivalent there, but a conditional expression has two independent operands that can each require unboxing. Consider reporting on both arms before returning, to match how visitBinary (lines 476–479) checks both operands unconditionally.

♻️ Proposed change 
-      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;
-        }
-      }
+      boolean ok = true;
+      if (trueNeedsUnboxing) {
+        ok &= checkForNullability(trueExpr, UNBOXING_OF_NULLABLE);
+      }
+      if (falseNeedsUnboxing) {
+        ok &= checkForNullability(falseExpr, UNBOXING_OF_NULLABLE);
+      }
+      if (!ok) {
+        // Avoid cascading errors from the superclass visit.
+        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 787 - 798, In NullnessVisitor, when checking the conditional arms,
don't early-return after checkForNullability(trueExpr, UNBOXING_OF_NULLABLE);
instead always call checkForNullability on both trueExpr and falseExpr (using
UNBOXING_OF_NULLABLE) and record each result, then return null if either check
failed; mirror the pattern used in visitBinary so both operands can report
unboxing errors while still stopping further processing if any failed. Ensure
you reference the existing trueNeedsUnboxing/falseNeedsUnboxing guards and keep
the existing behavior of returning null when any arm's check fails.

}
Comment on lines +774 to +799

@coderabbitai coderabbitai Bot Mar 3, 2026

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
View file
Open in desktop
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);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading