language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-03 18:28:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[red-knot] Fix callable subtyping for standard parameters (#17125)

Browse source 
## Summary

This PR fixes a bug in callable subtyping to consider both the
positional and keyword form of the standard parameter in the supertype
when matching against variadic, keyword-only and keyword-variadic
parameter in the subtype.

This is done by collecting the unmatched standard parameters and then
checking them against the keyword-only / keyword-variadic parameters
after the positional loop.

## Test Plan

Add test cases.
This commit is contained in:
Dhruv Manilawala 2025-04-01 23:37:35 +05:30 • committed by GitHub
parent c74ba00219
commit d29d4956de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 85 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

46
crates/red_knot_python_semantic/resources/mdtest/type_properties/is_subtype_of.md
View file

@ -858,6 +858,52 @@ static_assert(not is_subtype_of(CallableTypeOf[variadic], CallableTypeOf[keyword
static_assert(not is_subtype_of(CallableTypeOf[variadic], CallableTypeOf[keyword_variadic]))
```
But, there are special cases when matching against standard parameters. This is due to the fact that
a standard parameter can be passed as a positional or keyword parameter. This means that the
subtyping relation needs to consider both cases.
```py
def variadic_keyword(*args: int, **kwargs: int) -> None: ...
def standard_int(a: int) -> None: ...
def standard_float(a: float) -> None: ...
static_assert(is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_int]))
static_assert(not is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_float]))
```
If the type of either the variadic or keyword-variadic parameter is not a supertype of the standard
parameter, then the subtyping relation is invalid.
```py
def variadic_bool(*args: bool, **kwargs: int) -> None: ...
def keyword_variadic_bool(*args: int, **kwargs: bool) -> None: ...
static_assert(not is_subtype_of(CallableTypeOf[variadic_bool], CallableTypeOf[standard_int]))
static_assert(not is_subtype_of(CallableTypeOf[keyword_variadic_bool], CallableTypeOf[standard_int]))
```
The standard parameter can follow a variadic parameter in the subtype.
```py
def standard_variadic_int(a: int, *args: int) -> None: ...
def standard_variadic_float(a: int, *args: float) -> None: ...
static_assert(is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_variadic_int]))
static_assert(not is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_variadic_float]))
```
The keyword part of the standard parameter can be matched against keyword-only parameter with the
same name if the keyword-variadic parameter is absent.
```py
def variadic_a(*args: int, a: int) -> None: ...
def variadic_b(*args: int, b: int) -> None: ...
static_assert(is_subtype_of(CallableTypeOf[variadic_a], CallableTypeOf[standard_int]))
# The parameter name is different
static_assert(not is_subtype_of(CallableTypeOf[variadic_b], CallableTypeOf[standard_int]))
```
#### Keyword-only
For keyword-only parameters, the name should be the same:

48
crates/red_knot_python_semantic/src/types.rs
View file

@ -4644,6 +4644,10 @@ impl<'db> GeneralCallableType<'db> {
iter_other: other_signature.parameters().iter(),
};
// Collect all the standard parameters that have only been matched against a variadic
// parameter which means that the keyword variant is still unmatched.
let mut other_keywords = Vec::new();
loop {
let Some(next_parameter) = parameters.next() else {
// All parameters have been checked or both the parameter lists were empty. In
@ -4653,6 +4657,14 @@ impl<'db> GeneralCallableType<'db> {
match next_parameter {
EitherOrBoth::Left(self_parameter) => match self_parameter.kind() {
ParameterKind::KeywordOnly { .. } | ParameterKind::KeywordVariadic { .. }
if !other_keywords.is_empty() =>
{
// If there are any unmatched keyword parameters in `other`, they need to
// be checked against the keyword-only / keyword-variadic parameters that
// will be done after this loop.
break;
}
ParameterKind::PositionalOnly { default_type, .. }
| ParameterKind::PositionalOrKeyword { default_type, .. }
| ParameterKind::KeywordOnly { default_type, .. } => {
@ -4727,7 +4739,11 @@ impl<'db> GeneralCallableType<'db> {
}
}
(ParameterKind::Variadic { .. }, ParameterKind::PositionalOnly { .. }) => {
(
ParameterKind::Variadic { .. },
ParameterKind::PositionalOnly { .. }
| ParameterKind::PositionalOrKeyword { .. },
) => {
if !check_types(
other_parameter.annotated_type(),
self_parameter.annotated_type(),
@ -4735,6 +4751,13 @@ impl<'db> GeneralCallableType<'db> {
return false;
}
if matches!(
other_parameter.kind(),
ParameterKind::PositionalOrKeyword { .. }
) {
other_keywords.push(other_parameter);
}
// We've reached a variadic parameter in `self` which means there can
// be no more positional parameters after this in a valid AST. But, the
// current parameter in `other` is a positional-only which means there
@ -4749,14 +4772,17 @@ impl<'db> GeneralCallableType<'db> {
let Some(other_parameter) = parameters.peek_other() else {
break;
};
if !matches!(
other_parameter.kind(),
match other_parameter.kind() {
ParameterKind::PositionalOrKeyword { .. } => {
other_keywords.push(other_parameter);
}
ParameterKind::PositionalOnly { .. }
| ParameterKind::Variadic { .. }
) {
// Any other parameter kind cannot be checked against a
// variadic parameter and is deferred to the next iteration.
break;
| ParameterKind::Variadic { .. } => {}
_ => {
// Any other parameter kind cannot be checked against a
// variadic parameter and is deferred to the next iteration.
break;
}
}
if !check_types(
other_parameter.annotated_type(),
@ -4828,11 +4854,15 @@ impl<'db> GeneralCallableType<'db> {
}
}
for other_parameter in other_parameters {
for other_parameter in other_keywords.into_iter().chain(other_parameters) {
match other_parameter.kind() {
ParameterKind::KeywordOnly {
name: other_name,
default_type: other_default,
}
| ParameterKind::PositionalOrKeyword {
name: other_name,
default_type: other_default,
} => {
if let Some(self_parameter) = self_keywords.remove(other_name) {
match self_parameter.kind() {