Fix subtype checking for nested captured types

[msridhar]

The new test wildcardCaptureReturnPreservesNestedNullability is the most direct test for this gap. The key logic fix is in CheckIdenticalNullabilityVisitor#visitClassType, where we use the effective upper bound of rhsType when it is a wildcard or captured type. The change is otherwise a bit larger than expected to ensure checking doesn't break when the config setting to handle wildcards is disabled. We also extract the haveIdenticalNullability function to de-duplicate some logic.

Summary by CodeRabbit

• Bug Fixes

  • Improved nullability checking for wildcard generic types.
  • Prevented incorrect compatibility results and crashes involving wildcard captures, nested nullable types, and method references.
  • Ensured wildcard handling respects the configured settings.

• Tests

  • Added regression coverage for wildcard returns, method-reference parameters, and unbounded wildcard scenarios.

• Documentation

  • Clarified that JUnit test methods do not require Javadoc.

[msridhar]

This change is part of the following stack:

• Fix subtype checking for nested captured types #1663 ◀
  • Handle interaction of captured types and library models #1666
    • Follow-on fix for restoring annotation on captured type wildcards #1667
      • Harden TypeSubstitutionUtils type copies #1669
        • Handle unbound wildcards and interactions with captured types better #1668

_{Change managed by git-spice.}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

CheckIdenticalNullabilityVisitor gates wildcard handling on configuration, resolves captured RHS types to effective wildcard upper bounds, and delegates non-wildcard comparisons to haveIdenticalNullability. WildcardTests adds nested wildcard and method-reference regression cases. AGENTS.md clarifies that JUnit test methods do not require Javadoc.

Possibly related PRs

• uber/NullAway#1520: Modifies the same visitor's wildcard handling and related tests.
• uber/NullAway#1662: Covers related wildcard detection and containment behavior.
• uber/NullAway#1577: Updates overlapping wildcard nullability comparison logic.

Suggested reviewers: lazaroclapp

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing subtype checking for nested captured types.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nested-captures-restored-ananotations

---

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

[codecov]

Codecov Report

❌ Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.78%. Comparing base (da302b4) to head (f54e916).

Files with missing lines	Patch %	Lines
...way/generics/CheckIdenticalNullabilityVisitor.java	79.16%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1663      +/-   ##
============================================
+ Coverage     87.75%   87.78%   +0.02%     
- Complexity     3132     3136       +4     
============================================
  Files           109      109              
  Lines         10580    10586       +6     
  Branches       2136     2140       +4     
============================================
+ Hits           9285     9293       +8     
+ Misses          615      613       -2     
  Partials        680      680              

☔ View full report in Codecov by Harness.
📢 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.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java`:
- Around line 41-60: Add Javadoc to the non-trivial `visitClassType` method in
`CheckIdenticalNullabilityVisitor`, documenting its purpose and the relevant
wildcard/captured-type handling, including the behavior controlled by
`handleWildcardGenerics()`. Keep the implementation unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b24f7c50-1799-4ca5-a9f0-5d04538ce4c2

📥 Commits

Reviewing files that changed from the base of the PR and between 08a25a9 and 272c957.

📒 Files selected for processing (2)

• nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java`:
- Line 124: Add Javadoc to the non-trivial visitArrayType method, documenting
its purpose, parameters, and return value while accurately describing how it
compares array component nullability. Do not alter the existing
haveIdenticalNullability behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aec8c925-8f44-48a2-b812-37d0eb7717c6

📥 Commits

Reviewing files that changed from the base of the PR and between 272c957 and 5157eb1.

📒 Files selected for processing (2)

• nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java