Use ground target types when handling lambdas and method refs passed to generic methods#1575
Conversation
|
This change is part of the following stack:
Change managed by git-spice. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1575 +/- ##
============================================
- Coverage 88.30% 88.20% -0.11%
- Complexity 2926 2934 +8
============================================
Files 104 104
Lines 9776 9807 +31
Branches 1967 1974 +7
============================================
+ Hits 8633 8650 +17
- Misses 543 557 +14
Partials 600 600 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lazaroclapp
left a comment
There was a problem hiding this comment.
Some questions just out of curiosity, but LGTM!
| } | ||
| static void testNestedWildcard() { | ||
| invokeNested(box -> { | ||
| // BUG: Diagnostic contains: dereferenced expression box.get() is @Nullable |
There was a problem hiding this comment.
What does the true negative case look like here? Changing Function<Box<? super String>, R> mapper with Function<Box<? extends String>, R> mapper?
There was a problem hiding this comment.
Yes, that's correct. I've enhanced the test to show this.
| * rules for functional interface target types in JLS <a | ||
| * href="https://docs.oracle.com/javase/specs/jls/se21/html/jls-9.html#jls-9.9">9.9</a>. | ||
| */ | ||
| private static Type groundTypeArgument(Type typeArgument, VisitorState state) { |
There was a problem hiding this comment.
This works the same regardless of whether we are looking at the types used for the arguments of the function vs the return argument of the function? This looks correct to me from 9.9, but feels a bit counter-intuitive...
There was a problem hiding this comment.
Yes, that's correct. It was counterintuitive to me too. It's basically strictly on the bounds of the wildcard type, independently of where it is used.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR implements target-type grounding for wildcard generics in NullAway's poly-expression (lambda and method-reference) nullability inference. A new Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
Fixes #1522
When handling lambdas and method references passed to a generic method, if the generic method's formal parameter type is a class type with wildcards at the top level, we need to compute the ground target type for checking. In the ground target type, wildcards are replaced with their lower bound if explicit, and otherwise replaced with their upper bound.
So, for the test from #1522:
We have a lambda passed to
mapNotNull, whose parameter type isFunction<? super T, ? extends @Nullable V>. The ground target type for this parameter isFunction<T, @Nullable V>, which should be used to infer types at the call site and check the lambda. In this case, inference succeeds, and the return ofx.orElse(null)from the lambda is fine since the return type is@Nullable V.Summary by CodeRabbit
Release Notes
Bug Fixes
Tests