Skip to content

Commit d29d495

Browse files
authored
[red-knot] Fix callable subtyping for standard parameters (#17125)
## 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.
1 parent c74ba00 commit d29d495

File tree

2 files changed

+85
-9
lines changed

2 files changed

+85
-9
lines changed

crates/red_knot_python_semantic/resources/mdtest/type_properties/is_subtype_of.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,52 @@ static_assert(not is_subtype_of(CallableTypeOf[variadic], CallableTypeOf[keyword
858858
static_assert(not is_subtype_of(CallableTypeOf[variadic], CallableTypeOf[keyword_variadic]))
859859
```
860860

861+
But, there are special cases when matching against standard parameters. This is due to the fact that
862+
a standard parameter can be passed as a positional or keyword parameter. This means that the
863+
subtyping relation needs to consider both cases.
864+
865+
```py
866+
def variadic_keyword(*args: int, **kwargs: int) -> None: ...
867+
def standard_int(a: int) -> None: ...
868+
def standard_float(a: float) -> None: ...
869+
870+
static_assert(is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_int]))
871+
static_assert(not is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_float]))
872+
```
873+
874+
If the type of either the variadic or keyword-variadic parameter is not a supertype of the standard
875+
parameter, then the subtyping relation is invalid.
876+
877+
```py
878+
def variadic_bool(*args: bool, **kwargs: int) -> None: ...
879+
def keyword_variadic_bool(*args: int, **kwargs: bool) -> None: ...
880+
881+
static_assert(not is_subtype_of(CallableTypeOf[variadic_bool], CallableTypeOf[standard_int]))
882+
static_assert(not is_subtype_of(CallableTypeOf[keyword_variadic_bool], CallableTypeOf[standard_int]))
883+
```
884+
885+
The standard parameter can follow a variadic parameter in the subtype.
886+
887+
```py
888+
def standard_variadic_int(a: int, *args: int) -> None: ...
889+
def standard_variadic_float(a: int, *args: float) -> None: ...
890+
891+
static_assert(is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_variadic_int]))
892+
static_assert(not is_subtype_of(CallableTypeOf[variadic_keyword], CallableTypeOf[standard_variadic_float]))
893+
```
894+
895+
The keyword part of the standard parameter can be matched against keyword-only parameter with the
896+
same name if the keyword-variadic parameter is absent.
897+
898+
```py
899+
def variadic_a(*args: int, a: int) -> None: ...
900+
def variadic_b(*args: int, b: int) -> None: ...
901+
902+
static_assert(is_subtype_of(CallableTypeOf[variadic_a], CallableTypeOf[standard_int]))
903+
# The parameter name is different
904+
static_assert(not is_subtype_of(CallableTypeOf[variadic_b], CallableTypeOf[standard_int]))
905+
```
906+
861907
#### Keyword-only
862908

863909
For keyword-only parameters, the name should be the same:

crates/red_knot_python_semantic/src/types.rs

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4644,6 +4644,10 @@ impl<'db> GeneralCallableType<'db> {
46444644
iter_other: other_signature.parameters().iter(),
46454645
};
46464646

4647+
// Collect all the standard parameters that have only been matched against a variadic
4648+
// parameter which means that the keyword variant is still unmatched.
4649+
let mut other_keywords = Vec::new();
4650+
46474651
loop {
46484652
let Some(next_parameter) = parameters.next() else {
46494653
// All parameters have been checked or both the parameter lists were empty. In
@@ -4653,6 +4657,14 @@ impl<'db> GeneralCallableType<'db> {
46534657

46544658
match next_parameter {
46554659
EitherOrBoth::Left(self_parameter) => match self_parameter.kind() {
4660+
ParameterKind::KeywordOnly { .. } | ParameterKind::KeywordVariadic { .. }
4661+
if !other_keywords.is_empty() =>
4662+
{
4663+
// If there are any unmatched keyword parameters in `other`, they need to
4664+
// be checked against the keyword-only / keyword-variadic parameters that
4665+
// will be done after this loop.
4666+
break;
4667+
}
46564668
ParameterKind::PositionalOnly { default_type, .. }
46574669
| ParameterKind::PositionalOrKeyword { default_type, .. }
46584670
| ParameterKind::KeywordOnly { default_type, .. } => {
@@ -4727,14 +4739,25 @@ impl<'db> GeneralCallableType<'db> {
47274739
}
47284740
}
47294741

4730-
(ParameterKind::Variadic { .. }, ParameterKind::PositionalOnly { .. }) => {
4742+
(
4743+
ParameterKind::Variadic { .. },
4744+
ParameterKind::PositionalOnly { .. }
4745+
| ParameterKind::PositionalOrKeyword { .. },
4746+
) => {
47314747
if !check_types(
47324748
other_parameter.annotated_type(),
47334749
self_parameter.annotated_type(),
47344750
) {
47354751
return false;
47364752
}
47374753

4754+
if matches!(
4755+
other_parameter.kind(),
4756+
ParameterKind::PositionalOrKeyword { .. }
4757+
) {
4758+
other_keywords.push(other_parameter);
4759+
}
4760+
47384761
// We've reached a variadic parameter in `self` which means there can
47394762
// be no more positional parameters after this in a valid AST. But, the
47404763
// current parameter in `other` is a positional-only which means there
@@ -4749,14 +4772,17 @@ impl<'db> GeneralCallableType<'db> {
47494772
let Some(other_parameter) = parameters.peek_other() else {
47504773
break;
47514774
};
4752-
if !matches!(
4753-
other_parameter.kind(),
4775+
match other_parameter.kind() {
4776+
ParameterKind::PositionalOrKeyword { .. } => {
4777+
other_keywords.push(other_parameter);
4778+
}
47544779
ParameterKind::PositionalOnly { .. }
4755-
| ParameterKind::Variadic { .. }
4756-
) {
4757-
// Any other parameter kind cannot be checked against a
4758-
// variadic parameter and is deferred to the next iteration.
4759-
break;
4780+
| ParameterKind::Variadic { .. } => {}
4781+
_ => {
4782+
// Any other parameter kind cannot be checked against a
4783+
// variadic parameter and is deferred to the next iteration.
4784+
break;
4785+
}
47604786
}
47614787
if !check_types(
47624788
other_parameter.annotated_type(),
@@ -4828,11 +4854,15 @@ impl<'db> GeneralCallableType<'db> {
48284854
}
48294855
}
48304856

4831-
for other_parameter in other_parameters {
4857+
for other_parameter in other_keywords.into_iter().chain(other_parameters) {
48324858
match other_parameter.kind() {
48334859
ParameterKind::KeywordOnly {
48344860
name: other_name,
48354861
default_type: other_default,
4862+
}
4863+
| ParameterKind::PositionalOrKeyword {
4864+
name: other_name,
4865+
default_type: other_default,
48364866
} => {
48374867
if let Some(self_parameter) = self_keywords.remove(other_name) {
48384868
match self_parameter.kind() {

0 commit comments

Comments
 (0)