From 433610cde500032e07a2875b5f5084f4cbee0795 Mon Sep 17 00:00:00 2001 From: yangdanny97 Date: Sat, 29 Aug 2026 00:34:16 -0400 Subject: [PATCH] make pyrefly pass the new TypedDict conformance tests --- crates/pyrefly_types/src/typed_dict.rs | 9 +- pyrefly/lib/alt/callable.rs | 118 +++++++++++++++++++------ pyrefly/lib/error/context.rs | 6 ++ pyrefly/lib/error/display.rs | 13 +++ pyrefly/lib/test/callable.rs | 76 +++++++++++++++- pyrefly/lib/test/typed_dict.rs | 19 +++- website/docs/error-kinds.mdx | 10 +++ 7 files changed, 215 insertions(+), 36 deletions(-) diff --git a/crates/pyrefly_types/src/typed_dict.rs b/crates/pyrefly_types/src/typed_dict.rs index accc807cc7..521e0fe06e 100644 --- a/crates/pyrefly_types/src/typed_dict.rs +++ b/crates/pyrefly_types/src/typed_dict.rs @@ -160,12 +160,13 @@ pub enum ExtraItems { impl ExtraItems { pub fn extra(ty: Type, qualifiers: &[Qualifier]) -> Self { - match &ty { - Type::Type(inner) if inner.is_never() => Self::Closed, - _ => Self::Extra(ExtraItem { + if ty.is_never() { + Self::Closed + } else { + Self::Extra(ExtraItem { ty, read_only: qualifiers.iter().any(|q| q == &Qualifier::ReadOnly), - }), + }) } } diff --git a/pyrefly/lib/alt/callable.rs b/pyrefly/lib/alt/callable.rs index 2d1e0c0f4c..63f9676bef 100644 --- a/pyrefly/lib/alt/callable.rs +++ b/pyrefly/lib/alt/callable.rs @@ -718,6 +718,14 @@ enum NameOrigin<'a> { UnpackedKwargs(Option<&'a Name>), } +/// Where a value that may land on an unmatched keyword parameter came from. +enum SplatSource { + /// The value type of a splatted mapping, e.g. `f(**d)` where `d: dict[str, int]`. + MappingValue, + OpenExtraItems, + DeclaredExtraItems, +} + impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { /// Flag a call argument whose type is an implicit `Any` (unknown). Emitted into /// `arg_errors` (not `call_errors`), which is not used to decide overload/hint @@ -1366,11 +1374,13 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ); }, ); - if let ExtraItems::Extra(extra) = self.typed_dict_extra_items(typed_dict) { - kwargs = Some((name.as_ref(), Some(type_owner.push(extra.ty)))) - } else { - kwargs = Some((name.as_ref(), None)) - } + kwargs = match self.typed_dict_extra_items(typed_dict) { + ExtraItems::Closed => None, + ExtraItems::Extra(extra) => { + Some((name.as_ref(), Some(type_owner.push(extra.ty)))) + } + ExtraItems::Default => Some((name.as_ref(), None)), + }; } Param::Kwargs(name, ty) => { kwargs = Some((name.as_ref(), Some(ty))); @@ -1394,30 +1404,67 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ); } }; - let mut splat_kwargs = Vec::new(); + let mut splat_kwargs: Vec<(Type, TextRange, SplatSource)> = Vec::new(); for kw in keywords { match kw.arg { None => { let ty = kw.value.infer(self, arg_errors); self.maybe_error_unknown_argument_type(&ty, kw.range, arg_errors); if let Type::TypedDict(typed_dict) = ty { - // Splatting an open TypedDict into a callable without `**kwargs` is - // unsafe, since the TypedDict may contain additional, unknown keys - if kwargs.is_none() - && !matches!( - self.typed_dict_extra_items(&typed_dict), - ExtraItems::Closed - ) + // A non-closed TypedDict may carry arbitrary unknown keys, which can + // match the callee's kwargs or any of its unmatched keyword params. An + // anonymous TypedDict comes from a dict display, whose keys are all known. + let extra_items = self.typed_dict_extra_items(&typed_dict); + if !typed_dict.is_anonymous() && !matches!(extra_items, ExtraItems::Closed) { - error( - call_errors, + let open = matches!(extra_items, ExtraItems::Default); + let extra_ty = extra_items.extra_item(self.stdlib).ty; + match &kwargs { + None => { + error( + call_errors, + kw.range, + if open { + ErrorKind::OpenUnpacking + } else { + ErrorKind::UnexpectedKeyword + }, + format!( + "`{}` may contain extra items of type `{}`, which cannot be unpacked into a callable that accepts no extra keyword arguments", + typed_dict.name(), + self.for_display(extra_ty.clone()), + ), + ); + } + Some((kwargs_name, Some(want))) => { + self.check_type_with_options( + &extra_ty, + want, + kw.range, + TypeCheckOptions::new(call_errors, &|| { + TypeCheckContext::of_kind( + TypeCheckKind::CallExtraItems( + open, + kwargs_name.cloned(), + callable_name.cloned(), + ), + ) + .with_context(context.map(|ctx| ctx())) + }) + .with_call_context(call_context), + ); + } + Some((_, None)) => {} + } + splat_kwargs.push(( + extra_ty, kw.range, - ErrorKind::OpenUnpacking, - format!( - "`{}` is an open TypedDict with unknown extra items, which cannot be unpacked into a callable without `**kwargs`", - typed_dict.name() - ), - ); + if open { + SplatSource::OpenExtraItems + } else { + SplatSource::DeclaredExtraItems + }, + )); } for (name, field) in self.typed_dict_fields(&typed_dict).into_iter() { let name = name_owner.push(name); @@ -1488,7 +1535,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { .with_call_context(call_context), ); }; - splat_kwargs.push((value, kw.range)); + splat_kwargs.push((value, kw.range, SplatSource::MappingValue)); } else { error( call_errors, @@ -1634,11 +1681,14 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { extra_posargs_iter.next(); } let mut extra_posargs_matched = 0; + let splat_may_supply_missing_args = splat_kwargs + .iter() + .any(|(_, _, source)| matches!(source, SplatSource::MappingValue)); for (name, (want, origin, required)) in kwparams.iter() { if !seen_names.contains_key(name) { match required { Required::Required => { - if splat_kwargs.is_empty() { + if !splat_may_supply_missing_args { if let Some(arg_range) = extra_posargs_iter.next() { error( call_errors, @@ -1666,16 +1716,28 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } Required::Optional(None) => {} } - for (ty, range) in &splat_kwargs { + for (ty, range, source) in &splat_kwargs { self.check_type_with_options( ty, want, *range, TypeCheckOptions::new(call_errors, &|| { - TypeCheckContext::of_kind(TypeCheckKind::CallUnpackKwArg( - (*name).clone(), - callable_name.cloned(), - )) + TypeCheckContext::of_kind(match source { + SplatSource::MappingValue => TypeCheckKind::CallUnpackKwArg( + (*name).clone(), + callable_name.cloned(), + ), + SplatSource::OpenExtraItems => TypeCheckKind::CallExtraItems( + true, + Some((*name).clone()), + callable_name.cloned(), + ), + SplatSource::DeclaredExtraItems => TypeCheckKind::CallExtraItems( + false, + Some((*name).clone()), + callable_name.cloned(), + ), + }) .with_context(context.map(|ctx| ctx())) }) .with_call_context(call_context), diff --git a/pyrefly/lib/error/context.rs b/pyrefly/lib/error/context.rs index e7c35bf6a8..38f4da018f 100644 --- a/pyrefly/lib/error/context.rs +++ b/pyrefly/lib/error/context.rs @@ -167,6 +167,10 @@ pub enum TypeCheckKind { CallKwArgs(Option, Option, Option), /// Unpacked keyword argument against named parameter. CallUnpackKwArg(Name, Option), + /// The extra items of an unpacked TypedDict against a parameter they may land on. The bool + /// indicates whether the extra items are implied by the TypedDict being open, rather than + /// declared with `extra_items`. The name is the parameter's name, or `None` for `**kwargs`. + CallExtraItems(bool, Option, Option), /// Check of a parameter's default value against its type annotation. FunctionParameterDefault(Name), /// Check against the key type of a dict. @@ -242,6 +246,8 @@ impl TypeCheckKind { Self::CallVarArgs(..) => ErrorKind::BadArgumentType, Self::CallKwArgs(..) => ErrorKind::BadArgumentType, Self::CallUnpackKwArg(..) => ErrorKind::BadArgumentType, + Self::CallExtraItems(true, ..) => ErrorKind::OpenUnpacking, + Self::CallExtraItems(false, ..) => ErrorKind::BadArgumentType, Self::FunctionParameterDefault(..) => ErrorKind::BadFunctionDefinition, Self::DictKey | Self::DictValue | Self::TypedDictKey(_, _) => ErrorKind::BadAssignment, Self::TypedDictUnpacking => ErrorKind::BadUnpacking, diff --git a/pyrefly/lib/error/display.rs b/pyrefly/lib/error/display.rs index 5a02abd2e9..feaf115fe4 100644 --- a/pyrefly/lib/error/display.rs +++ b/pyrefly/lib/error/display.rs @@ -159,6 +159,19 @@ impl TypeCheckKind { ctx.display(want), function_suffix(func_id.as_ref(), current_module), ), + Self::CallExtraItems(_, param, func_id) => { + let param_desc = match param { + Some(param) => format!("parameter `{param}` with type"), + None => "kwargs type".to_owned(), + }; + format!( + "Extra items of type `{}` are not assignable to {} `{}`{}", + ctx.display(got), + param_desc, + ctx.display(want), + function_suffix(func_id.as_ref(), current_module), + ) + } Self::FunctionParameterDefault(param) => format!( "Default `{}` is not assignable to parameter `{}` with type `{}`", ctx.display(got), diff --git a/pyrefly/lib/test/callable.rs b/pyrefly/lib/test/callable.rs index 89fc4b030c..fca0796966 100644 --- a/pyrefly/lib/test/callable.rs +++ b/pyrefly/lib/test/callable.rs @@ -1035,18 +1035,90 @@ testcase!( test_forwarding_unpack_kwargs_to_fixed_signature, TestEnv::new().enable_open_unpacking_error(), r#" -from typing import TypedDict, Unpack +from typing import Never, TypedDict, Unpack class Open(TypedDict): name: str class Closed(TypedDict, closed=True): name: str +class ExtraNever(TypedDict, extra_items=Never): + name: str def has_kwargs(**kwargs: Unpack[Open]) -> None: ... def takes_name(name: str) -> None: ... def forward_open(**kwargs: Unpack[Open]) -> None: has_kwargs(**kwargs) # OK: target accepts **kwargs - takes_name(**kwargs) # E: `Open` is an open TypedDict with unknown extra items, which cannot be unpacked into a callable without `**kwargs` + takes_name(**kwargs) # E: `Open` may contain extra items of type `object`, which cannot be unpacked into a callable that accepts no extra keyword arguments def forward_closed(**kwargs: Unpack[Closed]) -> None: takes_name(**kwargs) # OK: closed TypedDict has no extra keys +def forward_extra_never(**kwargs: Unpack[ExtraNever]) -> None: + takes_name(**kwargs) # OK: `extra_items=Never` means the same as `closed=True` + "#, +); + +testcase!( + test_unpacking_open_typed_dict_into_call, + TestEnv::new().enable_open_unpacking_error(), + r#" +from typing import TypedDict, Unpack +class Open(TypedDict): + name: str +class Closed(TypedDict, closed=True): + name: str +def takes_name(name: str) -> None: ... +def takes_closed(**kwargs: Unpack[Closed]) -> None: ... +def takes_untyped_kwargs(name: str, **kwargs) -> None: ... +def takes_object_kwargs(name: str, **kwargs: object) -> None: ... +def takes_str_kwargs(name: str, **kwargs: str) -> None: ... +def f(open: Open, closed: Closed) -> None: + takes_name(**open) # E: `Open` may contain extra items of type `object`, which cannot be unpacked into a callable that accepts no extra keyword arguments + takes_closed(**open) # E: `Open` may contain extra items of type `object`, which cannot be unpacked into a callable that accepts no extra keyword arguments + takes_str_kwargs(**open) # E: Extra items of type `object` are not assignable to parameter `kwargs` with type `str` + takes_untyped_kwargs(**open) # OK + takes_object_kwargs(**open) # OK + takes_closed(**closed) # OK + takes_closed(bogus=1) # E: Missing argument `name` # E: Unexpected keyword argument `bogus` + "#, +); + +// An open TypedDict's extra items are only speculative, so they are reported under the opt-in +// `open-unpacking` kind. Extra items declared with `extra_items` are always reported. +testcase!( + test_unpacking_open_typed_dict_into_call_without_open_unpacking, + r#" +from typing import TypedDict, Unpack +class Open(TypedDict): + name: str +class ExtraInt(TypedDict, extra_items=int): + name: str +def takes_name(name: str) -> None: ... +def takes_str_kwargs(name: str, **kwargs: str) -> None: ... +def f(open: Open, extra_int: ExtraInt) -> None: + takes_name(**open) # OK + takes_str_kwargs(**open) # OK + takes_name(**extra_int) # E: `ExtraInt` may contain extra items of type `int`, which cannot be unpacked into a callable that accepts no extra keyword arguments + takes_str_kwargs(**extra_int) # E: Extra items of type `int` are not assignable to parameter `kwargs` with type `str` + "#, +); + +testcase!( + test_unpacking_typed_dict_extra_items_into_call, + r#" +from typing import TypedDict, Unpack +class Closed(TypedDict, closed=True): + name: str +class ExtraInt(TypedDict, extra_items=int): + name: str +def takes_closed(**kwargs: Unpack[Closed]) -> None: ... +def takes_int_kwargs(name: str, **kwargs: int) -> None: ... +def takes_label(name: str, *, label: str = "", **kwargs: int) -> None: ... +def takes_count(name: str, *, count: int = 0, **kwargs: int) -> None: ... +def takes_other(name: str, *, other: str, **kwargs: int) -> None: ... +def f(extra_int: ExtraInt) -> None: + takes_closed(**extra_int) # E: `ExtraInt` may contain extra items of type `int`, which cannot be unpacked into a callable that accepts no extra keyword arguments + takes_int_kwargs(**extra_int) # OK + takes_count(**extra_int) # OK + takes_label(**extra_int) # E: Extra items of type `int` are not assignable to parameter `label` with type `str` + # Extra items don't excuse a missing required argument. + takes_other(**extra_int) # E: Missing argument `other` # E: Extra items of type `int` are not assignable to parameter `other` with type `str` "#, ); diff --git a/pyrefly/lib/test/typed_dict.rs b/pyrefly/lib/test/typed_dict.rs index 7c66652b18..700d350e62 100644 --- a/pyrefly/lib/test/typed_dict.rs +++ b/pyrefly/lib/test/typed_dict.rs @@ -1356,6 +1356,21 @@ class TD(TypedDict, extra_items=ReadOnly[int]): "#, ); +testcase!( + test_extra_items_never_is_closed, + r#" +from typing import Never, TypedDict +class Closed(TypedDict, closed=True): + x: int +class ExtraNever(TypedDict, extra_items=Never): + x: int +class OpenChild(ExtraNever, closed=False): # E: Non-closed TypedDict cannot inherit from closed TypedDict `ExtraNever` + pass +c: Closed = {'x': 0} +n: ExtraNever = c + "#, +); + testcase!( test_bad_extra_items, r#" @@ -2139,8 +2154,8 @@ def fun(field1: str, field2: str): pass def test(x: TD, y: TD2, z: TD3): - fun(**x) - fun(**y) # E: Missing argument `field2` in function `fun` + fun(**x) # E: `TD` may contain extra items of type `str`, which cannot be unpacked into a callable that accepts no extra keyword arguments + fun(**y) # E: Missing argument `field2` in function `fun` # E: `TD2` may contain extra items of type `str` fun(**z) "#, ); diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 5b449788b7..f89d9f6f4d 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1669,6 +1669,16 @@ def f(o: OpenTypedDict) -> UnpackingTarget: return {"y": "", **o} ``` +The same error is reported when an open TypedDict is unpacked into a call whose target cannot +accept the unknown items: +```python +def g(x: int) -> None: ... + +def h(o: OpenTypedDict) -> None: + # Error: `o` could carry items under keys that `g` does not accept. + g(**o) +``` + To fix this error, close the open TypedDict to indicate it does not contain any unknown items: ```python class OpenTypedDict(TypedDict, closed=True): ...