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 6 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 all commits
Commits
Show all changes
6 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
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 @@ -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

@coderabbitai coderabbitai bot Mar 3, 2026

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);
}
}
Loading