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

[Suvrat1629]

• Fixes false negative where a @Nullable boxed value in a ternary expression with primitive result type was unboxed without warning.
• Added check in NullnessVisitor.visitConditionalExpression to detect when javac performs unboxing conversion on one or both operands.
• Added regression test Issue6849.java.

Closes #6849

[coderabbitai]

📝 Walkthrough

Walkthrough

visitConditionalExpression in NullnessVisitor was updated to perform unboxing-aware nullness checks (UNBOXING_OF_NULLABLE) on the true/false operands when the conditional expression has a primitive overall type but one or both operands are reference types. If an unboxing check reports a nullness issue for an operand, the method returns early to avoid cascading errors; otherwise it continues existing flow. A new test Issue6849.java was added to exercise a ternary expression that previously missed an unboxing-of-nullable error.

Possibly related PRs

• Nullness Checker: Manually scan expression and reference type in visitInstanceOf (#7341) #7342: Modifies nullness-checking logic in NullnessVisitor (changes visitInstanceOf scanning) and is related at the class level to the visitConditionalExpression updates.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR successfully detects and reports unboxing.of.nullable errors in conditional expressions where one branch returns a @nullable boxed value and the other a primitive type, addressing the false negative in issue #6849.
Out of Scope Changes check	✅ Passed	All changes are within scope: modifications to NullnessVisitor to detect unboxing conversions in conditional expressions, and a regression test demonstrating the fix for issue #6849.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0994c6a and f60011f.

📒 Files selected for processing (2)

• checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java
• checker/tests/nullness/Issue6849.java

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java

🔇 Additional comments (2)

checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java (1)

775-801: LGTM! The unboxing check for conditional expressions is correctly implemented.

The logic properly identifies when javac will perform implicit unboxing (primitive result type with reference-type operands) and checks the appropriate operands for nullability. The early return on error to avoid cascading diagnostics follows the same pattern used in visitTypeCast (lines 505-508).

checker/tests/nullness/Issue6849.java (1)

1-16: Test case correctly validates the fix for Issue #6849.

The test exercises the exact scenario from the issue: a ternary expression with a primitive int result type where the true branch returns a @Nullable Integer (requiring implicit unboxing). The error annotation is now correctly positioned before the erroneous line.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11643b3 and 0994c6a.

📒 Files selected for processing (2)

• checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java
• checker/tests/nullness/Issue6849.java

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java

🧬 Code graph analysis (1)

checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java (1)

javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java (1)

• TypesUtils (55-1555)

🔇 Additional comments (13)

checker/src/main/java/org/checkerframework/checker/nullness/NullnessVisitor.java (12)

71-72: Formatting-only change — no functional impact.

---

144-146: Formatting-only change — no functional impact.

---

157-163: Formatting-only change — no functional impact.

---

192-196: Formatting-only change — no functional impact.

---

221-226: Formatting-only change — no functional impact.

---

394-400: Formatting-only change — no functional impact.

---

442-444: Formatting-only change — no functional impact.

---

571-571: Formatting-only change — no functional impact.

---

699-701: Formatting-only change — no functional impact.

---

825-836: Formatting-only change — no functional impact.

---

841-842: Formatting-only change — no functional impact.

---

787-816: Logic correctly detects unboxing in conditional expressions, verified by existing tests.

The implementation properly:

1. Checks if the overall conditional result type is primitive
2. Identifies operands that need unboxing (reference types being assigned to primitive)
3. Reports unboxing.of.nullable when nullable references would be unboxed
4. Returns early after reporting an error to avoid cascading errors (consistent with visitTypeCast pattern)

The new logic is exercised by existing tests in Issue6849.java and Issue3614.java, which verify that unboxing errors are correctly reported in conditional expressions (e.g., int y = ((true) ? method() : 10) when method returns @Nullable Integer).

When both operands need unboxing and both are nullable, only the first error is reported due to the early return. This is acceptable—the cascading error prevention is intentional, though users would need to re-run after fixing the first issue to see subsequent errors.

checker/tests/nullness/Issue6849.java (1)

6-8: Consider adding @Nullable annotation to the return type for explicitness.

The method m returns T, which when instantiated as @Nullable Integer correctly returns a nullable value. The current implementation is correct, but you might consider whether adding a test case that explicitly shows the return type could help document the behavior.