Skip to content

Commit 6b6ef50

Browse files
committed
Reject lack of **kwargs
1 parent be939d6 commit 6b6ef50

2 files changed

Lines changed: 66 additions & 39 deletions

File tree

crates/ty_python_semantic/resources/mdtest/typed_dict.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3012,10 +3012,10 @@ func(v1=1, v2="optional", v3="ok")
30123012
func(v1=1, v3="ok", v4=1)
30133013
```
30143014

3015-
### Assignability to explicit keyword-only signatures
3015+
### Assignability from unpacked kwargs to explicit keyword-only signatures
30163016

30173017
A callable using `**kwargs: Unpack[TD2]` should line up with equivalent explicit keyword-only
3018-
signatures, and the assignability should work in both directions.
3018+
signatures when assigning to the explicit form.
30193019

30203020
```py
30213021
from typing import Protocol
@@ -3041,12 +3041,12 @@ explicit_ok: ExplicitKwargs = func
30413041
typed_dict_ok: TypedDictKwargs = func
30423042

30433043
def _(explicit: ExplicitKwargs, typed_dict: TypedDictKwargs) -> None:
3044-
typed_dict_2: TypedDictKwargs = explicit
30453044
explicit_2: ExplicitKwargs = typed_dict
30463045

30473046
def func7(*, v1: int, v3: str, v2: str = "") -> None:
30483047
pass
30493048

3049+
# error: [invalid-assignment]
30503050
typed_dict_from_explicit: TypedDictKwargs = func7
30513051
```
30523052

crates/ty_python_semantic/src/types/signatures.rs

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,12 @@ impl<'db> Signature<'db> {
709709
parameters.next();
710710
}
711711

712-
let mut parameters = Parameters::new(db, parameters);
712+
let mut parameters = Parameters::new(
713+
db,
714+
parameters
715+
.chain(self.parameters.unpacked.iter().map(Unpacked::to_parameter))
716+
.collect::<Vec<_>>(),
717+
);
713718
let mut return_ty = self.return_ty;
714719
let binding_context = self.definition.map(BindingContext::Definition);
715720
if let Some(self_type) = self_type {
@@ -2321,6 +2326,17 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
23212326
}
23222327
}
23232328

2329+
// `**kwargs: Unpack[TypedDict]` must stay distinct from an equivalent explicit keyword-only
2330+
// surface. A target callable with unpacked kwargs can still be invoked with additional
2331+
// keyword arguments supplied through a TypedDict subtype, so a source callable needs a real
2332+
// `**kwargs` parameter as well.
2333+
if target.parameters.unpacked.is_some()
2334+
&& source_keyword_variadic.is_none()
2335+
&& source.parameters.unpacked.is_none()
2336+
{
2337+
return self.never();
2338+
}
2339+
23242340
for target_param in target_keywords.into_iter().chain(target_params) {
23252341
match target_param.kind() {
23262342
ParameterKind::KeywordOnly {
@@ -2516,6 +2532,19 @@ impl<'db> Unpacked<'db> {
25162532
),
25172533
}
25182534
}
2535+
2536+
fn to_parameter(&self) -> Parameter<'db> {
2537+
Parameter {
2538+
annotated_type: self.annotated_type,
2539+
inferred_annotation: false,
2540+
has_starred_annotation: false,
2541+
has_unpacked_kwargs_annotation: true,
2542+
kind: ParameterKind::KeywordVariadic {
2543+
name: self.name.clone(),
2544+
},
2545+
form: ParameterForm::Value,
2546+
}
2547+
}
25192548
}
25202549

25212550
impl<'db> Parameters<'db> {
@@ -2524,22 +2553,42 @@ impl<'db> Parameters<'db> {
25242553
/// The kind of the parameter list is determined based on the provided parameters. Specifically,
25252554
/// if the parameter list contains `*args` and `**kwargs`, then it checks their annotated types
25262555
/// and the presence of other parameter kinds to determine if they represent a gradual form, a
2527-
/// `ParamSpec`, or a `Concatenate` form.
2528-
///
2529-
/// Callers that need to retain `**kwargs: Unpack[TypedDict]` metadata separately from the
2530-
/// signature surface should use [`Self::from_value_and_unpacked`].
2556+
/// `ParamSpec`, or a `Concatenate` form. `**kwargs: Unpack[TypedDict]` is normalized here by
2557+
/// removing the original variadic keyword parameter from the signature surface, synthesizing
2558+
/// keyword-only parameters for the unpacked keys, and storing the original parameter
2559+
/// separately in [`Self::unpacked`].
25312560
pub(crate) fn new(
25322561
db: &'db dyn Db,
25332562
parameters: impl IntoIterator<Item = Parameter<'db>>,
25342563
) -> Self {
2535-
Self::from_value_and_unpacked(db, parameters.into_iter().collect(), None)
2536-
}
2564+
let mut value: Vec<Parameter<'db>> = Vec::new();
2565+
let mut unpacked = None;
2566+
2567+
for parameter in parameters {
2568+
if let Some(unpacked_keys) = parameter.unpacked_typed_dict_keys(db) {
2569+
for (name, unpacked_key) in unpacked_keys {
2570+
if value
2571+
.iter()
2572+
.any(|existing| existing.callable_by_name(name.as_str()))
2573+
{
2574+
continue;
2575+
}
2576+
2577+
value.push(
2578+
Parameter::keyword_only(name)
2579+
.with_annotated_type(unpacked_key.value_ty)
2580+
.with_optional_default_type(
2581+
(!unpacked_key.is_required).then_some(Type::unknown()),
2582+
),
2583+
);
2584+
}
2585+
2586+
unpacked = Some(Unpacked::from_parameter(db, &parameter));
2587+
} else {
2588+
value.push(parameter);
2589+
}
2590+
}
25372591

2538-
pub(crate) fn from_value_and_unpacked(
2539-
db: &'db dyn Db,
2540-
value: Vec<Parameter<'db>>,
2541-
unpacked: Option<Unpacked<'db>>,
2542-
) -> Self {
25432592
debug_assert!(value.iter().all(|parameter| !parameter.is_unpacked(db)));
25442593

25452594
let mut kind = ParametersKind::Standard;
@@ -2914,7 +2963,6 @@ impl<'db> Parameters<'db> {
29142963
.chain(variadic)
29152964
.chain(keyword_only)
29162965
.collect::<Vec<_>>();
2917-
let mut unpacked = None;
29182966

29192967
if let Some(arg) = kwarg.as_ref() {
29202968
let keywords = Parameter::from_node_and_kind(
@@ -2925,31 +2973,10 @@ impl<'db> Parameters<'db> {
29252973
name: arg.name.id.clone(),
29262974
},
29272975
);
2928-
2929-
if let Some(unpacked_keys) = keywords.unpacked_typed_dict_keys(db) {
2930-
for (name, unpacked_key) in unpacked_keys {
2931-
if value
2932-
.iter()
2933-
.any(|parameter| parameter.callable_by_name(name.as_str()))
2934-
{
2935-
continue;
2936-
}
2937-
2938-
value.push(
2939-
Parameter::keyword_only(name)
2940-
.with_annotated_type(unpacked_key.value_ty)
2941-
.with_optional_default_type(
2942-
(!unpacked_key.is_required).then_some(Type::unknown()),
2943-
),
2944-
);
2945-
}
2946-
unpacked = Some(Unpacked::from_parameter(db, &keywords));
2947-
} else {
2948-
value.push(keywords);
2949-
}
2976+
value.push(keywords);
29502977
}
29512978

2952-
Self::from_value_and_unpacked(db, value, unpacked)
2979+
Self::new(db, value)
29532980
}
29542981

29552982
fn apply_type_mapping_impl<'a>(

0 commit comments

Comments
 (0)