mirror of
https://github.com/astral-sh/ruff.git
synced 2026-10-28 10:45:37 +08:00
[ty] Preserve keyword safety for unpacked callables (#27517)
## Summary Follow-up to #27516. Previously, callable assignability could overlook keyword arguments that reach explicit source parameters through a target's `**kwargs`: ```python from typing import Protocol class Callback(Protocol): def __call__(self, *args: *tuple[*tuple[int, ...], int], **kwargs: str) -> None: ... def source(*args: object, flag: int = 0, **kwargs: str) -> None: ... callback: Callback = source # error: [invalid-assignment] ``` The target accepts `callback(1, flag="wrong")`, but `source` requires `flag` to be an integer. We now validate target keyword-variadic values against explicit source parameters, reject names that collide with source parameters already occupied positionally, and prevent positional target arguments from satisfying required source keyword-only parameters. Existing protected names and uninhabited keyword annotations remain valid. --------- Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
This commit is contained in:
1 parent
4e9640e155
commit
ec92fb2b9c
6 files changed
+281
-2
No files matched your search
@@ -570,6 +570,24 @@ def unpack_variadic(*args: *tuple[int, *tuple[str, ...]], **kwargs: int) -> None
|
||||
reveal_type(unpack_variadic) # revealed: (*args: str, **kwargs: int) -> None
|
||||
```
|
||||
|
||||
### Function with a named prefix and required unpacked suffix
|
||||
|
||||
`Concatenate` can remove a named positional prefix without discarding the required suffix of an
|
||||
unpacked variadic parameter.
|
||||
|
||||
```py
|
||||
from typing import Callable, Concatenate
|
||||
|
||||
def remove_first[**P](callback: Callable[Concatenate[int, P], None]) -> Callable[P, None]:
|
||||
raise NotImplementedError
|
||||
|
||||
def named_prefix_and_suffix(name: int, *args: *tuple[*tuple[int, ...], int]) -> None: ...
|
||||
|
||||
# TODO: Preserve the unpacked tuple instead of exposing synthetic comparison parameters.
|
||||
# Should reveal `(*args: *tuple[*tuple[int, ...], int]) -> None`.
|
||||
reveal_type(remove_first(named_prefix_and_suffix)) # revealed: (*args: int, int, /) -> None
|
||||
```
|
||||
|
||||
## `Concatenate` with `ParamSpec` in generic function calls
|
||||
|
||||
### Basic call with inferred `ParamSpec`
|
||||
|
||||
@@ -1805,6 +1805,27 @@ reveal_type(invoke(accepts_int)) # revealed: int
|
||||
reveal_type(invoke(needs_str)) # revealed: int
|
||||
```
|
||||
|
||||
### Inferring uninhabited keyword types
|
||||
|
||||
Inferring a keyword type as `Never` can eliminate a collision with an occupied positional parameter.
|
||||
|
||||
The callback's return type must still contribute its own inference constraint.
|
||||
|
||||
```py
|
||||
from typing import Protocol
|
||||
|
||||
class Callback[T, R](Protocol):
|
||||
def __call__(self, x: int, /, *args: *tuple[*tuple[int, ...], int], **kwargs: T) -> R: ...
|
||||
|
||||
def infer[T, R](callback: Callback[T, R]) -> tuple[list[T], R]:
|
||||
raise NotImplementedError
|
||||
|
||||
def source(a: int, *args: *tuple[*tuple[int, ...], int], **kwargs: int) -> str:
|
||||
return ""
|
||||
|
||||
reveal_type(infer(source)) # revealed: tuple[list[Never], str]
|
||||
```
|
||||
|
||||
### Class constructors
|
||||
|
||||
We can recurse into the parameters and return values of `Callable` parameters to infer
|
||||
|
||||
@@ -620,6 +620,24 @@ f3(1)
|
||||
f3("a", "b")
|
||||
```
|
||||
|
||||
### Preserve an unpacked required suffix
|
||||
|
||||
A `ParamSpec` preserves a named positional prefix and the required suffix of an unpacked variadic
|
||||
parameter when inferring a callback signature.
|
||||
|
||||
```py
|
||||
from typing import Callable
|
||||
|
||||
def preserve[**P](callback: Callable[P, None]) -> Callable[P, None]:
|
||||
return callback
|
||||
|
||||
def named_prefix_and_suffix(name: int, *args: *tuple[*tuple[int, ...], int]) -> None: ...
|
||||
|
||||
# TODO: Preserve the unpacked tuple instead of exposing synthetic comparison parameters.
|
||||
# Should reveal `(name: int, *args: *tuple[*tuple[int, ...], int]) -> None`.
|
||||
reveal_type(preserve(named_prefix_and_suffix)) # revealed: (name: int, *args: int, int, /) -> None
|
||||
```
|
||||
|
||||
### Return type change using the same `ParamSpec` multiple times
|
||||
|
||||
```py
|
||||
|
||||
@@ -1084,7 +1084,7 @@ parameter following that tuple.
|
||||
|
||||
```py
|
||||
from typing import Any, Callable, Never, Unpack, cast
|
||||
from ty_extensions import static_assert
|
||||
from ty_extensions import Top, static_assert
|
||||
from ty_extensions._internal import RegularCallableTypeOf, is_assignable_to
|
||||
|
||||
def expects_suffix(callback: Callable[[Unpack[tuple[str, ...]], None], None]) -> None: ...
|
||||
@@ -1194,6 +1194,17 @@ static_assert(is_assignable_to(OneOrMoreIntegers, GradualSuffix))
|
||||
static_assert(is_assignable_to(GradualSuffix, OneOrMoreIntegers))
|
||||
```
|
||||
|
||||
Gradual and top callable signatures accept a named positional prefix before an unpacked required
|
||||
suffix. Their synthetic keyword parameters do not represent concrete keyword arguments that can
|
||||
collide with the prefix.
|
||||
|
||||
```py
|
||||
def named_prefix_and_suffix(name: int, *args: *tuple[*tuple[int, ...], int]) -> None: ...
|
||||
|
||||
static_assert(is_assignable_to(RegularCallableTypeOf[named_prefix_and_suffix], Callable[..., None]))
|
||||
static_assert(is_assignable_to(RegularCallableTypeOf[named_prefix_and_suffix], Top[Callable[..., None]]))
|
||||
```
|
||||
|
||||
A positional parameter cannot also be filled by a target keyword argument.
|
||||
|
||||
```py
|
||||
@@ -1225,6 +1236,30 @@ A suffix cannot be extended with elements that the source variadic parameter rej
|
||||
incompatible_suffix: Callable[[*tuple[int, ...], str, str], None] = requires_string_after_integers
|
||||
```
|
||||
|
||||
### Gradual keyword collisions with unpacked positional parameters
|
||||
|
||||
A gradual keyword type can materialize to `Never`, eliminating an otherwise possible collision.
|
||||
|
||||
```py
|
||||
from typing import Any
|
||||
from ty_extensions import static_assert
|
||||
from ty_extensions._internal import RegularCallableTypeOf, is_assignable_to
|
||||
|
||||
def source(a: int, *args: *tuple[*tuple[int, ...], int], **kwargs: int) -> None: ...
|
||||
def target(x: int, /, *args: *tuple[*tuple[int, ...], int], **kwargs: Any) -> None: ...
|
||||
|
||||
static_assert(is_assignable_to(RegularCallableTypeOf[source], RegularCallableTypeOf[target]))
|
||||
```
|
||||
|
||||
A gradual source tail does not remove a real named prefix or permit a duplicate argument.
|
||||
|
||||
```py
|
||||
def gradual_source(a: int, *args: Any, **kwargs: Any) -> None: ...
|
||||
def concrete_target(x: int, /, *args: *tuple[*tuple[int, ...], int], **kwargs: int) -> None: ...
|
||||
|
||||
static_assert(not is_assignable_to(RegularCallableTypeOf[gradual_source], RegularCallableTypeOf[concrete_target]))
|
||||
```
|
||||
|
||||
### Fixed-length unpacked positional parameters
|
||||
|
||||
An unpacked fixed-length tuple accepts exactly its declared positional arguments, including when the
|
||||
|
||||
@@ -1426,6 +1426,21 @@ static_assert(is_subtype_of(StringSuffix, Callable[[*tuple[object, ...], int, st
|
||||
static_assert(is_subtype_of(IntegerStringSuffix, Callable[[*tuple[int, ...], int, str], None]))
|
||||
```
|
||||
|
||||
A named positional prefix cannot make a callback with a required unpacked suffix compatible with a
|
||||
target that accepts no positional arguments.
|
||||
|
||||
```py
|
||||
def named_prefix_and_suffix(name: int, *args: *tuple[*tuple[int, ...], int]) -> None: ...
|
||||
def accepts_no_positional_arguments() -> None: ...
|
||||
|
||||
static_assert(
|
||||
not is_subtype_of(
|
||||
RegularCallableTypeOf[named_prefix_and_suffix],
|
||||
RegularCallableTypeOf[accepts_no_positional_arguments],
|
||||
)
|
||||
)
|
||||
```
|
||||
|
||||
A positional parameter cannot also be filled by a target keyword argument.
|
||||
|
||||
```py
|
||||
@@ -1450,6 +1465,71 @@ static_assert(is_subtype_of(OccupiesKeyword, RegularCallableTypeOf[rejects_keywo
|
||||
static_assert(is_subtype_of(OccupiesKeyword, RegularCallableTypeOf[rejects_named_keyword]))
|
||||
```
|
||||
|
||||
Variadic target keywords must be compatible with optional source keyword-only parameters unless an
|
||||
occupied target prefix prevents the corresponding name from being passed.
|
||||
|
||||
```py
|
||||
def optional_keyword_source(*args: object, flag: int = 0, **kwargs: str) -> None: ...
|
||||
def unprotected_keyword_target(*args: *tuple[*tuple[int, ...], int], **kwargs: str) -> None: ...
|
||||
def protected_keyword_target(flag: int, *args: *tuple[*tuple[int, ...], int], **kwargs: str) -> None: ...
|
||||
|
||||
static_assert(
|
||||
not is_subtype_of(
|
||||
RegularCallableTypeOf[optional_keyword_source],
|
||||
RegularCallableTypeOf[unprotected_keyword_target],
|
||||
)
|
||||
)
|
||||
static_assert(
|
||||
is_subtype_of(
|
||||
RegularCallableTypeOf[optional_keyword_source],
|
||||
RegularCallableTypeOf[protected_keyword_target],
|
||||
)
|
||||
)
|
||||
```
|
||||
|
||||
Passing a target argument positionally cannot satisfy a required source keyword-only parameter. A
|
||||
required target keyword-only parameter with the same name does satisfy it.
|
||||
|
||||
```py
|
||||
def requires_named_keyword(*args: int, a: int) -> None: ...
|
||||
def positional_keyword_target(a: int, *args: *tuple[*tuple[int, ...], int]) -> None: ...
|
||||
def required_keyword_target(*args: *tuple[*tuple[int, ...], int], a: int) -> None: ...
|
||||
|
||||
static_assert(
|
||||
not is_subtype_of(
|
||||
RegularCallableTypeOf[requires_named_keyword],
|
||||
RegularCallableTypeOf[positional_keyword_target],
|
||||
)
|
||||
)
|
||||
static_assert(
|
||||
is_subtype_of(
|
||||
RegularCallableTypeOf[requires_named_keyword],
|
||||
RegularCallableTypeOf[required_keyword_target],
|
||||
)
|
||||
)
|
||||
```
|
||||
|
||||
The same mismatch produces an assignment diagnostic when the target signature is used as an
|
||||
annotation.
|
||||
|
||||
```py
|
||||
type PositionalKeywordTarget = RegularCallableTypeOf[positional_keyword_target]
|
||||
|
||||
# error: [invalid-assignment]
|
||||
callback: PositionalKeywordTarget = requires_named_keyword
|
||||
```
|
||||
|
||||
A positional-only target prefix cannot prevent variadic keywords from colliding with an occupied
|
||||
source parameter. A matching positional-or-keyword target prefix does prevent that collision.
|
||||
|
||||
```py
|
||||
def unprotected_positional_prefix(a: int, /, *args: *tuple[*tuple[int, ...], int], **kwargs: int) -> None: ...
|
||||
def protected_positional_prefix(a: int, *args: *tuple[*tuple[int, ...], int], **kwargs: int) -> None: ...
|
||||
|
||||
static_assert(not is_subtype_of(OccupiesKeyword, RegularCallableTypeOf[unprotected_positional_prefix]))
|
||||
static_assert(is_subtype_of(OccupiesKeyword, RegularCallableTypeOf[protected_positional_prefix]))
|
||||
```
|
||||
|
||||
Equivalent empty or fixed-length unpacked parameters are compatible, but cannot be reused for
|
||||
additional positional arguments.
|
||||
|
||||
|
||||
@@ -2524,6 +2524,20 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
|
||||
let mut source_parameters = source.parameters.expand_starred_variadic_annotations(db);
|
||||
let mut target_parameters = target.parameters.expand_starred_variadic_annotations(db);
|
||||
|
||||
// Expanding `*args: *tuple[*tuple[int, ...], str]` creates a synthetic positional `str`
|
||||
// parameter immediately after the variadic `int` parameter. Check whether either
|
||||
// signature contains such a positional suffix.
|
||||
let source_has_unpacked_suffix = source_parameters.variadic().is_some_and(|(index, _)| {
|
||||
source_parameters
|
||||
.get(index + 1)
|
||||
.is_some_and(Parameter::is_positional)
|
||||
});
|
||||
let target_has_unpacked_suffix = target_parameters.variadic().is_some_and(|(index, _)| {
|
||||
target_parameters
|
||||
.get(index + 1)
|
||||
.is_some_and(Parameter::is_positional)
|
||||
});
|
||||
|
||||
// Gradual variadics and TypeVarTuples need their original suffix boundaries for
|
||||
// materialization and inference. Named source prefixes must also remain visible when a
|
||||
// target keyword could fill the same parameter.
|
||||
@@ -2674,6 +2688,55 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
|
||||
});
|
||||
}
|
||||
|
||||
// Concrete target keywords can collide with a fixed source prefix even when the source
|
||||
// has a gradual or ParamSpec tail. Check them before those specialized paths can return.
|
||||
let mut keyword_collision_checks = true;
|
||||
if (source_has_unpacked_suffix || target_has_unpacked_suffix)
|
||||
&& target_parameters.is_standard()
|
||||
{
|
||||
// A target call can fill a source parameter positionally and also pass its name as a
|
||||
// keyword. A matching target prefix protects that name only when the same call must
|
||||
// already have filled the target parameter, including every prefix before a suffix.
|
||||
for (source_index, source_parameter) in source_parameters.positional().enumerate() {
|
||||
let Some(source_name) = source_parameter.keyword_name() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if target_parameters.variadic().is_none()
|
||||
&& source_index >= target_parameters.positional().count()
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let target_keyword = target_parameters.keyword_by_name(source_name.as_str());
|
||||
if target_keyword.is_some_and(|(target_index, target_parameter)| {
|
||||
target_parameter.is_positional()
|
||||
&& (target_index <= source_index || target_has_unpacked_suffix)
|
||||
}) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let Some((_, keyword)) =
|
||||
target_keyword.or_else(|| target_parameters.keyword_variadic())
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
// The keyword must be uninhabited to avoid the collision. Keep this as a
|
||||
// constraint so gradual types can materialize to `Never` and inferable type
|
||||
// variables can be constrained to it.
|
||||
let no_collision = self.check_type_pair(db, keyword.annotated_type(), Type::Never);
|
||||
if result
|
||||
.intersect(db, self.constraints, no_collision)
|
||||
.is_never_satisfied(db, env)
|
||||
{
|
||||
// Still allow the ParamSpec handling below to preserve its inferred binding.
|
||||
keyword_collision_checks = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let check_types = |result: &mut ConstraintSet<'db, 'c>,
|
||||
target_ty: Type<'db>,
|
||||
source_ty: Type<'db>,
|
||||
@@ -3307,7 +3370,7 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
|
||||
}
|
||||
}
|
||||
|
||||
if !return_type_checks {
|
||||
if !return_type_checks || !keyword_collision_checks {
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -4105,6 +4168,24 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
|
||||
default_type: target_default,
|
||||
} => {
|
||||
if let Some(source_param) = source_keywords.remove(&**target_name) {
|
||||
// The suffix forces a target prefix to be positional, so it cannot
|
||||
// provide a source parameter that must be supplied by keyword.
|
||||
if target_has_unpacked_suffix
|
||||
&& source_param.is_keyword_only()
|
||||
&& source_param.default_type().is_none()
|
||||
&& !target_param.is_keyword_only()
|
||||
{
|
||||
if let Some(context) = self.report_context() {
|
||||
context.push(ErrorContext::ExtraRequiredParameter {
|
||||
parameter: ParameterDescription::new(
|
||||
target_index,
|
||||
source_param.name(),
|
||||
),
|
||||
});
|
||||
}
|
||||
return self.never();
|
||||
}
|
||||
|
||||
match source_param.kind() {
|
||||
ParameterKind::PositionalOrKeyword {
|
||||
default_type: source_default,
|
||||
@@ -4167,6 +4248,32 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
|
||||
}
|
||||
return self.never();
|
||||
};
|
||||
|
||||
// An explicit source keyword takes precedence over its `**kwargs`. Unless
|
||||
// the target also binds that name explicitly, values from its keyword
|
||||
// variadic must therefore be compatible with the source parameter too.
|
||||
for source_param in &source_parameters {
|
||||
let Some(source_name) = source_param.keyword_name() else {
|
||||
continue;
|
||||
};
|
||||
if !source_keywords.contains_key(source_name.as_str())
|
||||
|| target_parameters
|
||||
.keyword_by_name(source_name.as_str())
|
||||
.is_some()
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if !check_types(
|
||||
&mut result,
|
||||
target_param.annotated_type(),
|
||||
source_param.annotated_type(),
|
||||
Some(source_name),
|
||||
target_index,
|
||||
) {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
if !check_types(
|
||||
&mut result,
|
||||
target_param.annotated_type(),
|
||||
|
||||
Reference in new issue
Block a user