Fix wildcard inference bug with method references

[msridhar]

Fixes #1552

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved accuracy of null-checking for generic types and method references by ensuring proper type resolution.

• Tests

  • Enhanced test coverage for generic type nullability scenarios.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.45%. Comparing base (29a896a) to head (f379612).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1553   +/-   ##
=========================================
  Coverage     88.45%   88.45%           
  Complexity     2869     2869           
=========================================
  Files           103      103           
  Lines          9586     9587    +1     
  Branches       1930     1930           
=========================================
+ Hits           8479     8480    +1     
  Misses          532      532           
  Partials        575      575           

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

[msridhar]

This change is part of the following stack:

• Initial inference for wildcards #1549
  • Fix wildcard inference bug with method references #1553 ◀
    • Fix nullability for return and parameter wildcards #1558
      • Enable test for issue 1500 and add comment #1560

_{Change managed by git-spice.}

[lazaroclapp]

Probably also worth checking that, without the map to null, a Stream<String> works just fine with .forEach(Test::callHashCode);, no?

[coderabbitai]

Walkthrough

The PR modifies GenericsChecks.java to call maybeStorePolyExpressionTypeFromTarget for MemberReferenceTree before processing method-reference type relations, ensuring subsequent type lookups use target-derived inferred poly-expression types. Additionally, WildcardTests.java is updated to expand the mapStreamValuesToNullable test with separate positive and negative test methods, introducing helper functions and adjusting BUG diagnostic expectations to cover nullability mismatches in mapped values and functional-interface arguments.

Possibly related issues

• Issue #1552: This PR directly addresses the missing error with wildcard inference when passing a method reference instead of a lambda by implementing the poly-expression type storage for MemberReferenceTree that was referenced in the TODO comment.

Possibly related PRs

• PR #1430: Both PRs modify GenericsChecks to treat MemberReferenceTree like poly-expressions by storing and using inferred poly-expression types for method references.
• PR #1438: Both PRs improve handling of MemberReferenceTree in generic and method-reference inference within GenericsChecks.
• PR #1556: Both changes address nullability inference for method references in call-type checking by storing target-derived poly-expression types for MemberReferenceTree.

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 50.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 'Fix wildcard inference bug with method references' directly summarizes the main change: fixing a wildcard inference bug that occurs when method references are used instead of lambdas.
Linked Issues check	✅ Passed	The code changes in GenericsChecks.java and WildcardTests.java directly address issue #1552 by implementing the fix for missing error detection with wildcard inference when method references are passed.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the wildcard inference bug with method references as specified in issue #1552; no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 wildcard-inference-2

---

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

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[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/main/java/com/uber/nullaway/generics/GenericsChecks.java (1)

1601-1622: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Store the target-derived poly type before relation checks.

At Line 1621, maybeStorePolyExpressionTypeFromTarget(...) runs after processMethodRefTypeRelations(...) and this branch then returns. That ordering can miss target-annotated type info during the relation check itself.

Suggested fix

               if (currentActualParam instanceof MemberReferenceTree memberReferenceTree) {
+                maybeStorePolyExpressionTypeFromTarget(currentActualParam, formalParameter);
                 // the type of the method reference tree provided by javac may not capture
                 // nullability of nested types. So, do explicit type checks based on the return and
                 // parameter types of the referenced method
                 GenericsUtils.processMethodRefTypeRelations(
                     this,
@@
                             reportInvalidMethodReferenceParameterTypeError(
                                 memberReferenceTree, subtype, supertype, state);
                           }
                         }
                     });
-                maybeStorePolyExpressionTypeFromTarget(currentActualParam, formalParameter);
                 return;
               }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java` around
lines 1601 - 1622, The poly-expression target-derived type is being stored too
late: call maybeStorePolyExpressionTypeFromTarget(currentActualParam,
formalParameter) before invoking
GenericsUtils.processMethodRefTypeRelations(...) so that the relation checks
(inside processMethodRefTypeRelations involving
reportInvalidMethodReferenceReturnTypeError and
reportInvalidMethodReferenceParameterTypeError) see any target-provided
annotations; move the maybeStorePolyExpressionTypeFromTarget call to immediately
prior to the processMethodRefTypeRelations(...) invocation in the branch
handling currentActualParam instanceof MemberReferenceTree.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java`:
- Around line 1601-1622: The poly-expression target-derived type is being stored
too late: call maybeStorePolyExpressionTypeFromTarget(currentActualParam,
formalParameter) before invoking
GenericsUtils.processMethodRefTypeRelations(...) so that the relation checks
(inside processMethodRefTypeRelations involving
reportInvalidMethodReferenceReturnTypeError and
reportInvalidMethodReferenceParameterTypeError) see any target-provided
annotations; move the maybeStorePolyExpressionTypeFromTarget call to immediately
prior to the processMethodRefTypeRelations(...) invocation in the branch
handling currentActualParam instanceof MemberReferenceTree.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2b83059c-dcbe-4ccc-a131-2ea10ae66901

📥 Commits

Reviewing files that changed from the base of the PR and between 29a896a and f379612.

📒 Files selected for processing (2)

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