Stop matching switch expression kind using strings by msridhar · Pull Request #1404 · uber/NullAway

msridhar · 2025-12-24T17:31:04Z

Summary by CodeRabbit

Bug Fixes
- Improved handling of Java 12+ switch expressions with more consistent null analysis.
- Enhanced error reporting for unsupported expression types.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-24T17:33:30Z

Walkthrough

The changes address the handling of SWITCH_EXPRESSION by replacing string-based detection with direct enum-based matching. In NullAway.java, a dedicated case for SWITCH_EXPRESSION is added to the mayBeNullExpr switch statement, marking switch expressions as potentially null. In GenericsChecks.java, a traditional switch statement in shouldRunDataflowForExpression is converted to a modern switch expression with explicit SWITCH_EXPRESSION support. These modifications eliminate the previous string-name workaround pattern, now that JDK 17 is the minimum requirement.

Suggested reviewers

yuxincs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: replacing string-based matching of switch expression kinds with proper JDK 17 APIs.
Linked Issues check	✅ Passed	The pull request successfully addresses issue #1403 by replacing string-based switch expression matching with dedicated enum handling in both specified files.
Out of Scope Changes check	✅ Passed	All changes are directly related to the objective of removing string-based switch expression matching in NullAway.java and GenericsChecks.java.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch switch-expr-kind

📜 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 07fcb6f and bfe7ceb.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/NullAway.java
nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

📚 Learning: 2025-08-28T04:54:20.953Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
nullaway/src/main/java/com/uber/nullaway/NullAway.java

📚 Learning: 2025-08-14T18:50:06.159Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/NullAway.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build spring-framework with snapshot
GitHub Check: Build and test on windows-latest
GitHub Check: Build caffeine with snapshot
GitHub Check: Build and test on ubuntu-latest
GitHub Check: Build and test on macos-latest

🔇 Additional comments (2)

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (1)

1054-1065: LGTM! Clean refactoring to modern switch expression.

The conversion from a traditional switch statement to a switch expression is well done. The explicit handling of SWITCH_EXPRESSION is correct and removes the need for string-based detection. The logic is preserved: the same expression kinds that previously returned true still do.

nullaway/src/main/java/com/uber/nullaway/NullAway.java (1)

2703-2708: LGTM! Correct handling of SWITCH_EXPRESSION.

The explicit case for SWITCH_EXPRESSION correctly treats it as potentially nullable (line 2704), which is appropriate since the result depends on the branch taken. This aligns with the treatment of CONDITIONAL_EXPRESSION and ASSIGNMENT.

The default case now unconditionally throws RuntimeException (lines 2707-2708), which is the right behavior now that SWITCH_EXPRESSION is explicitly handled above.

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

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

codecov · 2025-12-24T17:37:23Z

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.21%. Comparing base (07fcb6f) to head (bfe7ceb).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...away/src/main/java/com/uber/nullaway/NullAway.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1404      +/-   ##
============================================
+ Coverage     88.19%   88.21%   +0.01%     
+ Complexity     2616     2615       -1     
============================================
  Files            95       95              
  Lines          8675     8671       -4     
  Branches       1743     1741       -2     
============================================
- Hits           7651     7649       -2     
+ Misses          510      509       -1     
+ Partials        514      513       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Stop matching switch expression kind using strings

bfe7ceb

msridhar enabled auto-merge (squash) December 24, 2025 18:19

msridhar requested a review from yuxincs December 24, 2025 18:19

yuxincs approved these changes Dec 24, 2025

View reviewed changes

msridhar merged commit 47e85c5 into master Dec 24, 2025
9 of 11 checks passed

msridhar deleted the switch-expr-kind branch December 24, 2025 18:25

coderabbitai Bot mentioned this pull request Dec 28, 2025

Enable StatementSwitchToExpressionSwitch check and fix all warnings #1413

Merged

coderabbitai Bot mentioned this pull request Jan 13, 2026

Bump minimum Error Prone version to 2.36.0, and add detection of nullable dereference by switch expressions #1427

Merged

Navigation Menu

Repository navigation

Stop matching switch expression kind using strings - #1404

Stop matching switch expression kind using strings#1404
msridhar merged 1 commit into
masterfrom
switch-expr-kind

msridhar commented Dec 24, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 24, 2025

Uh oh!

codecov Bot commented Dec 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

msridhar commented Dec 24, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 24, 2025

Walkthrough

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov Bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

msridhar commented Dec 24, 2025 •

edited by coderabbitai Bot

Loading

codecov Bot commented Dec 24, 2025 •

edited

Loading