Skip to content

Commit b4b8299

Browse files
[ty] Make NamedTuple(...) and namedtuple(...) calls stricter (#22601)
## Summary Closes astral-sh/ty#2513.
1 parent fd9f87d commit b4b8299

2 files changed

Lines changed: 186 additions & 31 deletions

File tree

crates/ty_python_semantic/resources/mdtest/named_tuple.md

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ static_assert(is_subtype_of(Url, tuple[str, int]))
173173

174174
# Invalid type expressions in fields produce a diagnostic.
175175
invalid_fields = (("x", 42),) # 42 is not a valid type
176-
# error: [invalid-type-form] "Invalid type `Literal[42]` in `NamedTuple` field type"
176+
# error: [invalid-type-form] "Object of type `Literal[42]` is not valid as a `NamedTuple` field type"
177177
InvalidNT = NamedTuple("InvalidNT", invalid_fields)
178178
reveal_type(InvalidNT) # revealed: <class 'InvalidNT'>
179179

@@ -559,35 +559,96 @@ Bad5 = NamedTuple("Bad5", [("x", int)], foobarbaz=42)
559559
# error: [invalid-argument-type] "Invalid argument to parameter `fields` of `NamedTuple()`"
560560
Bad6 = NamedTuple("Bad6", 12345)
561561
reveal_type(Bad6) # revealed: <class 'Bad6'>
562+
563+
# Invalid field definitions: strings instead of (name, type) tuples
564+
# error: [invalid-argument-type] "Invalid `NamedTuple()` field definition"
565+
# error: [invalid-argument-type] "Invalid `NamedTuple()` field definition"
566+
Bad7 = NamedTuple("Bad7", ["a", "b"])
567+
reveal_type(Bad7) # revealed: <class 'Bad7'>
568+
569+
# Invalid field definitions: type is not a valid type expression (e.g., int literals)
570+
# error: [invalid-type-form] "Object of type `Literal[123]` is not valid as a `NamedTuple` field type"
571+
# error: [invalid-type-form] "Object of type `Literal[456]` is not valid as a `NamedTuple` field type"
572+
Bad8 = NamedTuple("Bad8", [("a", 123), ("b", 456)])
573+
reveal_type(Bad8) # revealed: <class 'Bad8'>
562574
```
563575

564-
### Starred and double-starred arguments
576+
### Missing required arguments
565577

566-
When starred (`*args`) or double-starred (`**kwargs`) arguments are used, we fall back to normal
567-
call binding since we can't statically determine the arguments. This results in `NamedTupleFallback`
568-
being returned:
578+
`NamedTuple` and `namedtuple` require at least two positional arguments: `typename` and
579+
`fields`/`field_names`.
569580

570581
```py
571582
import collections
572583
from typing import NamedTuple
573584

585+
# Missing both typename and fields
586+
# error: [missing-argument] "Missing required arguments `typename` and `fields` to `NamedTuple()`"
587+
Bad1 = NamedTuple()
588+
reveal_type(Bad1) # revealed: type[NamedTupleFallback]
589+
590+
# Missing fields argument
591+
# error: [missing-argument] "Missing required argument `fields` to `NamedTuple()`"
592+
Bad2 = NamedTuple("Bad2")
593+
reveal_type(Bad2) # revealed: type[NamedTupleFallback]
594+
595+
# Missing both typename and field_names for collections.namedtuple
596+
# error: [missing-argument] "Missing required arguments `typename` and `field_names` to `namedtuple()`"
597+
Bad3 = collections.namedtuple()
598+
reveal_type(Bad3) # revealed: type[NamedTupleFallback]
599+
600+
# Missing field_names argument
601+
# error: [missing-argument] "Missing required argument `field_names` to `namedtuple()`"
602+
Bad4 = collections.namedtuple("Bad4")
603+
reveal_type(Bad4) # revealed: type[NamedTupleFallback]
604+
```
605+
606+
### Starred and double-starred arguments
607+
608+
For `collections.namedtuple`, starred (`*args`) or double-starred (`**kwargs`) arguments cause us to
609+
fall back to `NamedTupleFallback` since we can't statically determine the arguments:
610+
611+
```py
612+
import collections
613+
574614
args = ("Point", ["x", "y"])
575615
kwargs = {"rename": True}
576616

577617
# Starred positional arguments - falls back to NamedTupleFallback
578618
Point1 = collections.namedtuple(*args)
579619
reveal_type(Point1) # revealed: type[NamedTupleFallback]
580620

581-
# Ideally we'd catch this false negative,
582-
# but it's unclear if it's worth the added complexity
583-
Point2 = NamedTuple(*args)
621+
# Double-starred keyword arguments - falls back to NamedTupleFallback
622+
Point2 = collections.namedtuple("Point", ["x", "y"], **kwargs)
584623
reveal_type(Point2) # revealed: type[NamedTupleFallback]
585624

586-
# Double-starred keyword arguments - falls back to NamedTupleFallback
587-
Point3 = collections.namedtuple("Point", ["x", "y"], **kwargs)
625+
# Both starred and double-starred
626+
Point3 = collections.namedtuple(*args, **kwargs)
627+
reveal_type(Point3) # revealed: type[NamedTupleFallback]
628+
```
629+
630+
For `typing.NamedTuple`, variadic arguments are not supported and result in an error:
631+
632+
```py
633+
from typing import NamedTuple
634+
635+
args = ("Point", [("x", int), ("y", int)])
636+
kwargs = {"extra": True}
637+
638+
# error: [invalid-argument-type] "Variadic positional arguments are not supported in `NamedTuple()` calls"
639+
Point1 = NamedTuple(*args)
640+
reveal_type(Point1) # revealed: type[NamedTupleFallback]
641+
642+
# error: [invalid-argument-type] "Variadic positional arguments are not supported in `NamedTuple()` calls"
643+
Point2 = NamedTuple("Point", *args)
644+
reveal_type(Point2) # revealed: type[NamedTupleFallback]
645+
646+
# error: [invalid-argument-type] "Variadic keyword arguments are not supported in `NamedTuple()` calls"
647+
Point3 = NamedTuple("Point", [("x", int), ("y", int)], **kwargs)
588648
reveal_type(Point3) # revealed: type[NamedTupleFallback]
589649

590-
Point4 = NamedTuple("Point", [("x", int), ("y", int)], **kwargs)
650+
# error: [invalid-argument-type] "Variadic positional and keyword arguments are not supported in `NamedTuple()` calls"
651+
Point4 = NamedTuple(*args, **kwargs)
591652
reveal_type(Point4) # revealed: type[NamedTupleFallback]
592653
```
593654

crates/ty_python_semantic/src/types/infer/builder.rs

Lines changed: 114 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ use crate::types::diagnostic::{
7474
INVALID_PARAMETER_DEFAULT, INVALID_PARAMSPEC, INVALID_PROTOCOL, INVALID_TYPE_ARGUMENTS,
7575
INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL, INVALID_TYPE_GUARD_DEFINITION,
7676
INVALID_TYPE_VARIABLE_CONSTRAINTS, INVALID_TYPED_DICT_STATEMENT, IncompatibleBases,
77-
NO_MATCHING_OVERLOAD, NOT_SUBSCRIPTABLE, POSSIBLY_MISSING_ATTRIBUTE,
77+
MISSING_ARGUMENT, NO_MATCHING_OVERLOAD, NOT_SUBSCRIPTABLE, POSSIBLY_MISSING_ATTRIBUTE,
7878
POSSIBLY_MISSING_IMPLICIT_CALL, POSSIBLY_MISSING_IMPORT, SUBCLASS_OF_FINAL_CLASS,
7979
TOO_MANY_POSITIONAL_ARGUMENTS, TypedDictDeleteErrorKind, UNDEFINED_REVEAL, UNKNOWN_ARGUMENT,
8080
UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE,
@@ -6531,6 +6531,29 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65316531
node_index: _,
65326532
} = &call_expr.arguments;
65336533

6534+
// Check for variadic arguments early, before extracting positional args.
6535+
let has_starred = args.iter().any(ast::Expr::is_starred_expr);
6536+
let has_double_starred = keywords.iter().any(|kw| kw.arg.is_none());
6537+
6538+
// Emit diagnostic for missing required arguments or unsupported variadic arguments.
6539+
// For `typing.NamedTuple`, emit a diagnostic since variadic arguments are not supported.
6540+
// For `collections.namedtuple`, silently fall back since it's more permissive at runtime.
6541+
if (has_starred || has_double_starred)
6542+
&& kind.is_typing()
6543+
&& let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, call_expr)
6544+
{
6545+
let arg_type = if has_starred && has_double_starred {
6546+
"Variadic positional and keyword arguments are"
6547+
} else if has_starred {
6548+
"Variadic positional arguments are"
6549+
} else {
6550+
"Variadic keyword arguments are"
6551+
};
6552+
builder.into_diagnostic(format_args!(
6553+
"{arg_type} not supported in `NamedTuple()` calls"
6554+
));
6555+
}
6556+
65346557
// Need at least typename and fields/field_names.
65356558
let [name_arg, fields_arg, rest @ ..] = &**args else {
65366559
for arg in args {
@@ -6539,6 +6562,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65396562
for kw in keywords {
65406563
self.infer_expression(&kw.value, TypeContext::default());
65416564
}
6565+
6566+
if !has_starred && !has_double_starred {
6567+
let fields_param = match kind {
6568+
NamedTupleKind::Typing => "fields",
6569+
NamedTupleKind::Collections => "field_names",
6570+
};
6571+
let missing = if args.is_empty() {
6572+
format!("`typename` and `{fields_param}`")
6573+
} else {
6574+
format!("`{fields_param}`")
6575+
};
6576+
if let Some(builder) = self.context.report_lint(&MISSING_ARGUMENT, call_expr) {
6577+
builder.into_diagnostic(format_args!(
6578+
"Missing required argument{} {missing} to `{kind}()`",
6579+
if args.is_empty() { "s" } else { "" }
6580+
));
6581+
}
6582+
}
65426583
return KnownClass::NamedTupleFallback.to_subclass_of(self.db());
65436584
};
65446585

@@ -6551,15 +6592,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65516592

65526593
// If any argument is a starred expression or any keyword is a double-starred expression,
65536594
// we can't statically determine the arguments, so fall back to normal call binding.
6554-
if args.iter().any(ast::Expr::is_starred_expr) || keywords.iter().any(|kw| kw.arg.is_none())
6555-
{
6595+
if has_starred || has_double_starred {
65566596
for kw in keywords {
65576597
self.infer_expression(&kw.value, TypeContext::default());
65586598
}
65596599
return KnownClass::NamedTupleFallback.to_subclass_of(self.db());
65606600
}
65616601

6562-
// Check for excess positional arguments (only typename and fields are expected).
6602+
// Check for excess positional arguments (only `typename` and `fields` are expected).
65636603
if !rest.is_empty() {
65646604
if let Some(builder) = self
65656605
.context
@@ -6724,21 +6764,57 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
67246764
);
67256765
}
67266766

6727-
// Emit diagnostic if the type is outright invalid (not an iterable).
6767+
// Emit diagnostic if the type is outright invalid (not an iterable) or
6768+
// if we have a list/tuple literal with invalid field specs.
67286769
if fields.is_none() {
67296770
let iterable_any =
67306771
KnownClass::Iterable.to_specialized_instance(db, &[Type::any()]);
6731-
if !fields_type.is_assignable_to(db, iterable_any)
6732-
&& let Some(builder) =
6772+
if !fields_type.is_assignable_to(db, iterable_any) {
6773+
if let Some(builder) =
67336774
self.context.report_lint(&INVALID_ARGUMENT_TYPE, fields_arg)
6734-
{
6735-
let mut diagnostic = builder.into_diagnostic(format_args!(
6736-
"Invalid argument to parameter `fields` of `NamedTuple()`"
6737-
));
6738-
diagnostic.set_primary_message(format_args!(
6739-
"Expected an iterable of `(name, type)` pairs, found `{}`",
6740-
fields_type.display(db)
6741-
));
6775+
{
6776+
let mut diagnostic = builder.into_diagnostic(format_args!(
6777+
"Invalid argument to parameter `fields` of `NamedTuple()`"
6778+
));
6779+
diagnostic.set_primary_message(format_args!(
6780+
"Expected an iterable of `(name, type)` pairs, found `{}`",
6781+
fields_type.display(db)
6782+
));
6783+
}
6784+
} else {
6785+
// Check if we have a list/tuple literal with invalid elements
6786+
// (e.g., strings instead of (name, type) tuples).
6787+
let elements: Option<&[ast::Expr]> = match fields_arg {
6788+
ast::Expr::List(list) => Some(&list.elts),
6789+
ast::Expr::Tuple(tuple) => Some(&tuple.elts),
6790+
_ => None,
6791+
};
6792+
if let Some(elements) = elements {
6793+
for elt in elements {
6794+
let is_valid_field_spec = matches!(
6795+
elt,
6796+
ast::Expr::Tuple(t) if t.elts.len() == 2
6797+
) || matches!(
6798+
elt,
6799+
ast::Expr::List(l) if l.elts.len() == 2
6800+
);
6801+
if !is_valid_field_spec {
6802+
let elt_type = self.expression_type(elt);
6803+
if let Some(builder) =
6804+
self.context.report_lint(&INVALID_ARGUMENT_TYPE, elt)
6805+
{
6806+
let mut diagnostic =
6807+
builder.into_diagnostic(format_args!(
6808+
"Invalid `NamedTuple()` field definition"
6809+
));
6810+
diagnostic.set_primary_message(format_args!(
6811+
"Expected a `(name, type)` tuple, found `{}`",
6812+
elt_type.display(db)
6813+
));
6814+
}
6815+
}
6816+
}
6817+
}
67426818
}
67436819
}
67446820

@@ -6930,7 +7006,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
69307006
self.context.report_lint(&INVALID_TYPE_FORM, fields_arg)
69317007
{
69327008
builder.into_diagnostic(format_args!(
6933-
"Invalid type `{}` in `NamedTuple` field type",
7009+
"Object of type `{}` is not valid as a `NamedTuple` field type",
69347010
field_type.display(db)
69357011
));
69367012
}
@@ -6999,6 +7075,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
69997075
fields_arg: &ast::Expr,
70007076
) -> Option<Box<[(Name, Type<'db>, Option<Type<'db>>)]>> {
70017077
let db = self.db();
7078+
let scope_id = self.scope();
7079+
let typevar_binding_context = self.typevar_binding_context;
70027080

70037081
// Get the elements from the list or tuple literal.
70047082
let elements: &[ast::Expr] = match fields_arg {
@@ -7028,10 +7106,22 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
70287106

70297107
// Second element: field type (infer as type expression).
70307108
let field_type_expr = &field_spec_elts[1];
7031-
let field_ty = self
7032-
.expression_type(field_type_expr)
7033-
.in_type_expression(db, self.scope(), self.typevar_binding_context)
7034-
.ok()?;
7109+
let field_value_ty = self.expression_type(field_type_expr);
7110+
let field_ty = field_value_ty
7111+
.in_type_expression(db, scope_id, typevar_binding_context)
7112+
.unwrap_or_else(|error| {
7113+
// Report diagnostic for invalid type expression.
7114+
if let Some(builder) = self
7115+
.context
7116+
.report_lint(&INVALID_TYPE_FORM, field_type_expr)
7117+
{
7118+
builder.into_diagnostic(format_args!(
7119+
"Object of type `{}` is not valid as a `NamedTuple` field type",
7120+
field_value_ty.display(db)
7121+
));
7122+
}
7123+
error.fallback_type
7124+
});
70357125

70367126
Some((field_name, field_ty, None))
70377127
})
@@ -15508,6 +15598,10 @@ impl NamedTupleKind {
1550815598
matches!(self, Self::Collections)
1550915599
}
1551015600

15601+
const fn is_typing(self) -> bool {
15602+
matches!(self, Self::Typing)
15603+
}
15604+
1551115605
fn from_type<'db>(db: &'db dyn Db, ty: Type<'db>) -> Option<Self> {
1551215606
match ty {
1551315607
Type::SpecialForm(SpecialFormType::NamedTuple) => Some(NamedTupleKind::Typing),

0 commit comments

Comments
 (0)