Support marking method type variable upper bounds as @Nullable in library models

[msridhar]

We need this support to address #1157 (which I'll do in a follow-up PR). Now LibraryModels can mark the upper bound of some method type variable as @Nullable. Rename method in LibraryModels for getting upper bounds of class type variables for clarity.

Summary by CodeRabbit

• New Features

  • Enhanced null-checking capabilities to support nullable upper bounds in generic type variables for library methods.

• Tests

  • Added test coverage for validating nullable upper bounds in generic method type parameters.

[coderabbitai]

Walkthrough

Adds per-method type-variable nullable-upper-bound support to library models and wires it through nullability checks. Introduces LibraryModels.methodTypeVariablesWithNullableUpperBounds(), implements it in DefaultLibraryModels and CombinedLibraryModels, and aggregates the data in the combined model. Renames the class-level hook onOverrideTypeParameterUpperBound → onOverrideClassTypeVariableUpperBound and adds onOverrideMethodTypeVariableUpperBound to Handler/handlers/LibraryModelsHandler/CompositeHandler/BaseNoOpHandler. ConstraintSolverImpl and GenericsChecks now consult the new method-level and renamed class-level override hooks. Tests and a library helper method were added to exercise the new behavior.

Possibly related PRs

• Improved inference for generic method calls #1244: Adds per-method nullable-upper-bound library models and updates constraint-solving to consult handler-level overrides (directly touches generics/constraint code).
• Add more instance fields to GenericsChecks #1256: Modifies GenericsChecks and related handler APIs for upper-bound override checks (affects the same API surface and checks).

Suggested reviewers

• yuxincs
• lazaroclapp

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding support for marking method type variable upper bounds as @nullable in library models.

✨ 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 library-model-method-nullable-upper-bound

---

📜 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 57619e3 and 1d6ef8e.

📒 Files selected for processing (2)

• nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java (1 hunks)
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java (1 hunks)

🧰 Additional context used

🧠 Learnings (4)

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

📚 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/ConstraintSolverImpl.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.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/generics/ConstraintSolverImpl.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/generics/ConstraintSolverImpl.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.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 and test on macos-latest
• GitHub Check: Build and test on ubuntu-latest
• GitHub Check: Build caffeine with snapshot

🔇 Additional comments (3)

nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java (2)

302-323: LGTM! Override checking logic is correct and well-structured.

The implementation correctly checks for library model overrides before falling back to actual upper bound annotations:

• Method-level type variables are checked via handler.onOverrideMethodTypeVariableUpperBound
• Class-level type variables are checked via handler.onOverrideClassTypeVariableUpperBound
• Both paths defensively guard against invalid type variable indices (typeVarIndex >= 0)

The early-return behavior when an override applies aligns with the PR's goal of supporting nullable upper bounds in library models.

---

324-328: Good addition of clarifying comment.

The comment on line 324 clearly documents that this is the fallback path when no library model overrides apply, improving code readability.

nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java (1)

1451-1467: Test is sound and properly integrated.

The test validates an important edge case: method reference type inference with generic methods. The type scenario (calling visitEnum with l::add where l : List<Object> satisfies Consumer<E> with E extends Enum<E>) is non-trivial and correctly expects compilation to succeed without errors.

Recent commits show extensive improvements to generic method inference (including handling lambda and void-returning lambda arguments). This test likely prevents regression on the corner case mentioned in the latest commit "don't crash on corner case observed with spring." The test does not directly exercise nullable bounds (no @Nullable in the bound), making it a complementary regression test rather than a feature test.

The test structure, code, and integration are all correct.

---

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 85.36585% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.40%. Comparing base (fdec09a) to head (1d6ef8e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...m/uber/nullaway/generics/ConstraintSolverImpl.java	73.33%	1 Missing and 3 partials ⚠️
...ava/com/uber/nullaway/generics/GenericsChecks.java	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1345      +/-   ##
============================================
- Coverage     88.41%   88.40%   -0.01%     
- Complexity     2582     2592      +10     
============================================
  Files            97       97              
  Lines          8664     8701      +37     
  Branches       1722     1731       +9     
============================================
+ Hits           7660     7692      +32     
- Misses          504      505       +1     
- Partials        500      504       +4     

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