Allow positional-only params with defaults in method overrides#23037
Merged
Allow positional-only params with defaults in method overrides#23037
Conversation
Typing conformance resultsNo changes detected ✅ |
|
… defaults When a subclass method has a positional-only parameter with a default value and the base class overload has only keyword parameters (`**kwargs`), the override should be valid because callers don't need to provide the positional argument. Previously, the signature comparison logic at `signatures.rs:1618-1624` would unconditionally reject any unmatched positional-only parameter when comparing against a base signature that only had keyword parameters. This fix adds a check for whether the positional-only parameter has a default value, and only rejects it if it doesn't. Fixes astral-sh/ty#2693 https://claude.ai/code/session_011QG3kSTsewc8vNsUTtVL9G
carljm
force-pushed
the
claude/liskov-substitution-analysis-Glysa
branch
from
d4beb78 to
104574f
Compare
carljm
requested review from
AlexWaygood,
dcreager and
sharkdp
as code owners
dhruvmanila
approved these changes
Feb 3, 2026
carljm
commented
Feb 3, 2026
| static_assert(is_subtype_of(CallableTypeOf[positional_only_with_default_and_kwargs], CallableTypeOf[empty])) | ||
| static_assert(not is_subtype_of(CallableTypeOf[empty], CallableTypeOf[positional_only_with_default_and_kwargs])) | ||
|
|
||
| static_assert(is_subtype_of(CallableTypeOf[positional_only_with_default_and_kwargs], CallableTypeOf[kwargs_int])) |
Contributor
Author
There was a problem hiding this comment.
This is the only line here that fails on main; the rest are just added for completeness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change fixes assignability (and thus Liskov checking) to overloaded methods that have positional-only parameters with defaults.
Previously, when a base class had an overloaded method where one overload accepts only keyword arguments (
**kwargs), and a subclass tried to override it with a positional-only parameter, the override was always rejected as invalid. However, if that positional-only parameter has a default value, the override is actually valid because callers can still invoke the method using keyword arguments without providing the positional argument.The fix updates the signature compatibility check in
signatures.rsto allow positional-only parameters in the override if they have defaults, since they don't require callers to provide positional arguments.This partially addresses astral-sh/ty#2693 -- it fixes one issue in that example, but there's a remaining problem involving annotated generic self types.
Test Plan
Added an mdtest. Although the fix is to the general signature type-relation code, a Liskov test is the most intuitive way to demonstrate it, since otherwise it's finicky to construct assignability checks between more complex callable types. (A protocol could be another way to test it, but the original bug report here was a Liskov violation, so I just went with that.)