Fix issue with passing poly expression to a function parameter with type from library model

[msridhar]

Consider:

public class LambdaModel {
 public static String apply(Function<String, /* @Nullable */ String> mapper) {
    return mapper.apply("");
  }
}
void test() {
  LambdaModel.apply(unused -> null);
}

Here, the @Nullable annotation within the comment comes from a library model. Accounting for the library model, the code is legal. Before, we did not properly account for the library model, so we assumed the type of the apply parameter was Function<String, String>, and reported a false positive error on the lambda body. This PR properly uses the library models so we don't get errors for cases like the above.

We also do minor fixes for cases where lambdas or method references passed as parameters are enclosed in parentheses.

Summary by CodeRabbit

• New Features

  • Enhanced handling of modeled lambda and method-reference arguments in generic calls, improving propagation of nested nullable annotations.

• Bug Fixes

  • Fixed nullability inference for lambda expressions and method references passed to generic methods, including scenarios where the argument is parenthesized.

• Tests

  • Added new JSpecify and library-model tests covering lambda/method-reference return nullability, explicit type-argument forms, and functional-interface lower-bound mismatch diagnostics.

• Documentation

  • Added new modeled library API test classes to support the updated coverage.

[msridhar]

This change is part of the following stack:

• Fix issue with passing poly expression to a function parameter with type from library model #1614 ◀
  • Fix crash with library model @Nullable being applied to wildcard #1615
    • Add more modeling for Optional #1613

_{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

In JSpecify mode, NullAway now records library-modeled poly-expression types for lambda expressions and method references before later generic nullability checks run. GenericsChecks adds a helper that caches modeled poly-expression types when the invoked generic method’s overridden method type differs, and it normalizes parenthesized arguments during poly-expression cache lookup and member-reference handling. The PR also adds LambdaBox, LambdaConsumer, and LambdaModel test fixtures, registers nested nullable model annotations for LambdaModel, and adds new tests for lambda and method-reference nullability cases.

Possibly related PRs

• uber/NullAway#1407: Introduced the handler.onOverrideMethodType plumbing that this PR uses when storing library-modeled poly-expression types.
• uber/NullAway#1430: Related poly-expression type caching in GenericsChecks for lambda and method-reference handling.
• uber/NullAway#1499: Also updates GenericsChecks poly-expression inference paths for lambdas and method references.

Suggested labels

jspecify

Suggested reviewers

• lazaroclapp
• yuxincs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 summarizes the main fix: handling poly expressions passed to function parameters using library-model types.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch library-model-lambda-issue

---

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 83.07692% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.05%. Comparing base (bb0561c) to head (3929677).

Files with missing lines	Patch %	Lines
...ava/com/uber/nullaway/generics/GenericsChecks.java	76.08%	3 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1614   +/-   ##
=========================================
  Coverage     88.04%   88.05%           
- Complexity     3033     3044   +11     
=========================================
  Files           105      105           
  Lines         10100    10160   +60     
  Branches       2051     2063   +12     
=========================================
+ Hits           8893     8946   +53     
- Misses          571      573    +2     
- Partials        636      641    +5     

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

[lazaroclapp]

Minor, very much optional nits, but otherwise it LGTM!

[lazaroclapp]

Maybe we have equivalent tests elsewhere, but do we want a .mapNonNull and .applyNonNull methods which demonstrate that, in the absence of a model saying otherwise, these still disallow @Nullable in those type argument locations? (Again, if there are already tests for that elsewhere, feel free to skip, just checking)

[msridhar]

I'm quite confident this gets tested elsewhere as part of our JSpecify tests. So I'll skip adding more such tests here.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test-library-models/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java (1)

612-612: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update stale expected diagnostic text

This expectation is missing the single quotes now used in dereference diagnostics, so the test can fail on message mismatch.

Suggested fix

-                  // BUG: Diagnostic contains: dereferenced expression value is `@Nullable`
+                  // BUG: Diagnostic contains: dereferenced expression 'value' is `@Nullable`

🤖 Prompt for 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.

In
`@test-library-models/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java`
at line 612, The expected diagnostic text in the CustomLibraryModelsTests
assertion is stale because dereference messages now wrap the expression in
single quotes. Update the expectation near the existing BUG comment so it
matches the current wording exactly, using the same test case and diagnostic
string pattern already used in CustomLibraryModelsTests.

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

Outside diff comments:
In
`@test-library-models/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java`:
- Line 612: The expected diagnostic text in the CustomLibraryModelsTests
assertion is stale because dereference messages now wrap the expression in
single quotes. Update the expectation near the existing BUG comment so it
matches the current wording exactly, using the same test case and diagnostic
string pattern already used in CustomLibraryModelsTests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6bcbf0c2-24d0-48ae-8461-dd9b81da0cc2

📥 Commits

Reviewing files that changed from the base of the PR and between 6947293 and e24358e.

📒 Files selected for processing (3)

• nullaway/src/main/java/com/uber/nullaway/NullAway.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodLambdaOrMethodRefArgTests.java
• test-library-models/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java