Fix type parameter nullability matching for covariant types#1533
Fix type parameter nullability matching for covariant types#1533utafrali wants to merge 1 commit intouber:masterfrom
Conversation
|
|
|
Hi @utafrali at a high level this seems wrong to me. Can you point to where in the JLS / JSpecify specification this covariant mode for type arguments is discussed? |
|
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 pull request adds covariant nullability checking for generic type parameters in method references. It modifies 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 |
|
Good point, let me double-check this against the spec and revise if needed. |
|
Closing as this fix is invalid; see #1528 (comment) |
Fixes #1528
There's a regression in type parameter nullability matching for covariant generics. CheckIdenticalNullabilityVisitor was being too strict, requiring identical nullability on both sides even when the type system allows for more flexibility.
Added a covariant mode to the visitor that handles this correctly. In covariant positions, RHS can be @nullable while LHS isn't (but not the reverse). Updated the comparison logic for both type arguments and array components.
Tests added to cover this scenario.
Summary by CodeRabbit
Bug Fixes
Tests