Improve handling of wildcard upper bounds in generic method inference#1454
Improve handling of wildcard upper bounds in generic method inference#1454msridhar merged 27 commits intouber:masterfrom
Conversation
…om/dhruv-agr/NullAway into generic-method-lambda-arg-wildcard
…om/dhruv-agr/NullAway into generic-method-lambda-arg-wildcard
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1454 +/- ##
=========================================
Coverage 88.41% 88.41%
Complexity 2715 2715
=========================================
Files 99 99
Lines 9019 9022 +3
Branches 1803 1805 +2
=========================================
+ Hits 7974 7977 +3
Misses 517 517
Partials 528 528 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThis change adds special-case handling in TypeSubstitutionUtils.RestoreNullnessAnnotationsVisitor.visitClassType to detect 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 unit tests (beta)
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 |
msridhar
left a comment
There was a problem hiding this comment.
Nice find @dhruv-agr!
I'm a bit uncomfortable with this change; I feel like it should break some case. But, I can't come up with a test that fails with it. So, let's go ahead and land it, but re-visit whether we need this when we implement more full support for wildcards.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodLambdaOrMethodRefArgTests.java`:
- Line 448: Remove the unused inner class Foo declared as "class Foo<T extends
`@Nullable` Object> {}" inside the GenericMethodLambdaOrMethodRefArgTests test;
locate the declaration of the inner class Foo and delete it so the test class no
longer contains this unused leftover type.
nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodLambdaOrMethodRefArgTests.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java`:
- Around line 296-303: Replace the enum comparison
wt.kind.equals(BoundKind.EXTENDS) with the idiomatic and null-safe reference
equality check wt.kind == BoundKind.EXTENDS inside the TypeSubstitutionUtils
logic that handles other instanceof Type.WildcardType (the branch using wt and
BoundKind.EXTENDS) so the code compares enum values by == rather than .equals().
Fixes #1350
TypeSubstitutionUtils.updateTypeWithInferredNullabilitywas treating the wildcard as a mismatch for the concrete type and failed to copy the@Nullableannotation from the bound (R) to the concrete type (Object). ModifiedTypeSubstitutionUtils.javato explicitly handleType.WildcardTypein theRestoreNullnessAnnotationsVisitor. Specifically, when the "other" type is anextendswildcard, we should recurse on its upper boundSummary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.