Various cleanups suggested by IntelliJ

[msridhar]

Summary by CodeRabbit

Release Notes

• Refactor
  • Code modernization: Updated internal implementation to use modern Java language features and improved patterns for better code clarity and maintainability.

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

[coderabbitai]

Walkthrough

This pull request modernizes two core NullAway files by refactoring code patterns and updating method signatures. In NullAway.java, a public method parameter receives a @Nullable annotation, and various code patterns are updated to use Java pattern matching with inline variable declarations instead of explicit casts. In GenericsChecks.java, similar pattern matching improvements are applied throughout the generics inference logic, replacing traditional instanceof-cast patterns with bound variables. Additionally, minor stylistic updates include replacing size() == 0 checks with isEmpty() calls and adjusting error message formatting. No changes to core algorithmic behavior or public APIs are introduced beyond the nullability annotation.

Possibly related PRs

• PR 1266: Modifies generics/inference logic in GenericsChecks.java for config-aware visitor changes, sharing overlap with the pattern matching refactoring in this PR

Suggested reviewers

• yuxincs
• lazaroclapp

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Various cleanups suggested by IntelliJ' is vague and generic, using non-descriptive terminology that doesn't convey what specific cleanups were made or which files/features were affected.	Provide a more specific title that describes the key changes, such as 'Use Java pattern matching in instanceof checks and add nullable annotations' or 'Modernize code with pattern variables and collection utilities'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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 22aa129 and 7ed36da.

📒 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 (6)

📓 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.

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

📚 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/NullAway.java
• nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

📚 Learning: 2025-12-31T23:59:13.310Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1421
File: nullaway/src/main/java/com/uber/nullaway/NullAway.java:339-340
Timestamp: 2025-12-31T23:59:13.310Z
Learning: In Error Prone's ASTHelpers class, the getSymbol(MethodInvocationTree) overload is guaranteed to return a non-null Symbol.MethodSymbol. This differs from other overloads of getSymbol() that may return null. Therefore, no null check is required after calling ASTHelpers.getSymbol() with a MethodInvocationTree parameter.

Applied to files:

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

📚 Learning: 2025-08-29T18:41:43.584Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

Applied to files:

• 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

📚 Learning: 2025-10-29T23:56:18.236Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Applied to files:

• nullaway/src/main/java/com/uber/nullaway/NullAway.java
• nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.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 and test on macos-latest
• GitHub Check: Build and test on ubuntu-latest
• GitHub Check: Build caffeine with snapshot
• GitHub Check: Build and test on windows-latest
• GitHub Check: Build spring-framework with snapshot

🔇 Additional comments (3)

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

319-323: Public API change: @nullable annotation added to method parameter.

The addition of @Nullable to the invocationNode parameter is a public API change that documents the method already handles null (line 320 checks invocationNode == null). This is good practice for API clarity, though technically it's a breaking change for strict null-safety checkers.

Note: If this is a widely-used public API, consider mentioning this annotation addition in release notes as it affects null-safety contracts for callers.

---

858-859: Cleaner error message formatting.

Removing explicit .toString() calls is fine since String.format() and the + operator automatically invoke toString() on objects. The error messages remain unchanged while the code is more concise.

Also applies to: 900-901, 1111-1112, 1118-1119

---

521-536: LGTM! Pattern matching and idiomatic improvements.

The changes throughout this file demonstrate good code modernization:

• Pattern matching with bound variables (lines 521, 2054) is safer and more readable
• Using isEmpty() instead of size() comparisons is more idiomatic and intention-revealing
• Diamond operator usage reduces verbosity without losing type safety

All changes preserve existing behavior while improving code quality.

Also applies to: 1377-1391, 2237-2243, 2279-2283, 2345-2353

---

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.}

[codecov]

Codecov Report

❌ Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.37%. Comparing base (22aa129) to head (7ed36da).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...away/src/main/java/com/uber/nullaway/NullAway.java	73.33%	0 Missing and 4 partials ⚠️
...ava/com/uber/nullaway/generics/GenericsChecks.java	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1423      +/-   ##
============================================
- Coverage     88.40%   88.37%   -0.03%     
+ Complexity     2669     2667       -2     
============================================
  Files            97       97              
  Lines          8872     8853      -19     
  Branches       1775     1773       -2     
============================================
- Hits           7843     7824      -19     
- Misses          511      512       +1     
+ Partials        518      517       -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.