Fix bug with lambdas assigned to locals with a wildcard in their type

[msridhar]

Previously, we only used the target type of an assignment when inferring the type of a lambda / method reference when a field was being assigned. But this is incorrect; we should also use declared types of local variables. (I think I was confused about local inference before; the top-level nullability of locals does get inferred, but here we need the type arguments, which are not inferred).

Summary by CodeRabbit

• Bug Fixes

  • Improved nullness checking for generic lambdas and method references by expanding how inferred poly-expression target types are cached, including for local and resource variable initializers.
  • Enhanced generic assignability so extends wildcard nullness (including nullable bounds) is reflected more accurately in diagnostics.

• Tests

  • Added coverage for assigning a lambda to a local Supplier<? extends @nullable Object> target and verifying expected nullness behavior.
  • Updated an existing generics compilation test to remove an assertion for a “returning @Nullable” diagnostic that no longer applies in that snippet.

[msridhar]

This change is part of the following stack:

• Fix handling of reference to method with @Nullable parameter from library model #1642
  • Enable JSpecify JDK models (under a flag) #1641
    • Enable JSpecify JDK models for regression tests #1646
      • Fix bug with lambdas assigned to locals with a wildcard in their type #1647 ◀
        • Add JSpecifyExperimental configuration flag #1648
          • Enable inference failure warnings in experimental mode, and fix related bug in library models #1649

_{Change managed by git-spice.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.17%. Comparing base (6943dea) to head (1c4dc6d).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1647   +/-   ##
=========================================
  Coverage     88.16%   88.17%           
+ Complexity     3066     3064    -2     
=========================================
  Files           105      105           
  Lines         10303    10301    -2     
  Branches       2080     2079    -1     
=========================================
- Hits           9084     9083    -1     
  Misses          576      576           
+ Partials        643      642    -1     

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

Walkthrough

GenericsChecks now caches inferred target types for lambda and method-reference assignments without restricting the behavior to field assignments, and removes the obsolete helper. Tests add local lambda coverage for extends-wildcard nullness and remove an outdated diagnostic expectation.

Possibly related PRs

• uber/NullAway#1428: Updates the same assignability logic and poly-expression target caching.
• uber/NullAway#1430: Modifies inferred poly-expression type caching in GenericsChecks.
• uber/NullAway#1499: Changes lambda and method-reference target caching in checkTypeParameterNullnessForAssignability.

Suggested labels: jspecify

Suggested reviewers: yuxincs, lazaroclapp

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: lambda handling for local variables with wildcard types.

✨ 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 supplier-assigned-lambda-returning-null

---

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

[lazaroclapp]

(I think I was confused about local inference before; the top-level nullability of locals does get inferred, but were we need the type arguments, which are not inferred).

I don't think I can parse this...

Edit: never mind, s/were/here/g, right? I thought there was something more complicated going on

[lazaroclapp]

This seems correct, given the tests and just the general notion that we still have to perform "deep" nullness inference on locals, surprised the fix was this straightforward without any side effects 😁

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

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

2010-2010: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove @Nullable from this local variable.

Repository guidance forbids annotating local variables with @Nullable; NullAway infers local nullability automatically. Use Object[] o = ... instead.

As per coding guidelines and retrieved learnings, local nullability annotations should not be added in this codebase.

🤖 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 `@nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java` at line
2010, Remove the `@Nullable` annotation from the local variable declaration in
GenericsTests, changing it to an unannotated Object[] while preserving the
existing conditional assignment and inferred nullability behavior.

Sources: Coding guidelines, Learnings

🤖 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 `@nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java`:
- Line 2010: Remove the `@Nullable` annotation from the local variable declaration
in GenericsTests, changing it to an unannotated Object[] while preserving the
existing conditional assignment and inferred nullability behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 44fb0a68-e00e-49b7-8ba4-692e4de74673

📥 Commits

Reviewing files that changed from the base of the PR and between 974105a and ccd037a.

📒 Files selected for processing (3)

• nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericLambdaTests.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

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

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

Annotate the array reference, not its elements.

@Nullable Object[] permits nullable elements but keeps the array reference non-null. Since the conditional expression can evaluate to null, this test should use Object @nullable [] o; otherwise doTest() should report a nullability error.

Based on learnings, @Nullable Object[] and Object @nullable [] have different nullability targets.

🤖 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 `@nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java` at line
2010, Update the declaration of o in the GenericsTests test so `@Nullable`
annotates the array reference using the Object `@Nullable` [] syntax, not the
element type. Preserve the conditional assignment and ensure doTest() reports no
nullability error for the nullable array reference.

Source: Learnings

🤖 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 `@nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java`:
- Line 2010: Update the declaration of o in the GenericsTests test so `@Nullable`
annotates the array reference using the Object `@Nullable` [] syntax, not the
element type. Preserve the conditional assignment and ensure doTest() reports no
nullability error for the nullable array reference.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9032e39d-8f5c-42ee-a7f6-33f98eda6d9d

📥 Commits

Reviewing files that changed from the base of the PR and between ccd037a and 7aaa8e3.

📒 Files selected for processing (3)

• nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericLambdaTests.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java

[msridhar]

(I think I was confused about local inference before; the top-level nullability of locals does get inferred, but were we need the type arguments, which are not inferred).

I don't think I can parse this...

Edit: never mind, s/were/here/g, right? I thought there was something more complicated going on

Yes, sorry for the typo!