Skip to content

Commit b07a53a

Browse files
[ty] Emit diagnostics for invalid dynamic namedtuple fields (#22575)
## Summary Removes some TODOs from the dynamic namedtuple implementation.
1 parent 87eec9b commit b07a53a

2 files changed

Lines changed: 96 additions & 16 deletions

File tree

crates/ty_python_semantic/resources/mdtest/named_tuple.md

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,27 @@ Point2 = collections.namedtuple("Point2", ["_x", "class"], rename=1)
466466
reveal_type(Point2) # revealed: <class 'Point2'>
467467
reveal_type(Point2.__new__) # revealed: (cls: type, _0: Any, _1: Any) -> Point2
468468

469+
# Without `rename=True`, invalid field names emit diagnostics:
470+
# - Field names starting with underscore
471+
# error: [invalid-named-tuple] "Field name `_x` in `namedtuple()` cannot start with an underscore"
472+
Underscore = collections.namedtuple("Underscore", ["_x", "y"])
473+
reveal_type(Underscore) # revealed: <class 'Underscore'>
474+
475+
# - Python keywords
476+
# error: [invalid-named-tuple] "Field name `class` in `namedtuple()` cannot be a Python keyword"
477+
Keyword = collections.namedtuple("Keyword", ["x", "class"])
478+
reveal_type(Keyword) # revealed: <class 'Keyword'>
479+
480+
# - Duplicate field names
481+
# error: [invalid-named-tuple] "Duplicate field name `x` in `namedtuple()`"
482+
Duplicate = collections.namedtuple("Duplicate", ["x", "y", "x"])
483+
reveal_type(Duplicate) # revealed: <class 'Duplicate'>
484+
485+
# - Invalid identifiers (e.g., containing spaces)
486+
# error: [invalid-named-tuple] "Field name `not valid` in `namedtuple()` is not a valid identifier"
487+
Invalid = collections.namedtuple("Invalid", ["not valid", "ok"])
488+
reveal_type(Invalid) # revealed: <class 'Invalid'>
489+
469490
# `defaults` provides default values for the rightmost fields
470491
Person = collections.namedtuple("Person", ["name", "age", "city"], defaults=["Unknown"])
471492
reveal_type(Person) # revealed: <class 'Person'>
@@ -483,8 +504,8 @@ reveal_type(person2.city) # revealed: Any
483504
Config = collections.namedtuple("Config", ["host", "port"], module="myapp")
484505
reveal_type(Config) # revealed: <class 'Config'>
485506

486-
# When more defaults are provided than fields, we treat all fields as having defaults.
487-
# TODO: This should emit a diagnostic since it would fail at runtime.
507+
# When more defaults are provided than fields, an error is emitted.
508+
# error: [invalid-named-tuple] "Too many defaults for `namedtuple()`"
488509
TooManyDefaults = collections.namedtuple("TooManyDefaults", ["x", "y"], defaults=("a", "b", "c"))
489510
reveal_type(TooManyDefaults) # revealed: <class 'TooManyDefaults'>
490511
reveal_type(TooManyDefaults.__new__) # revealed: (cls: type, x: Any = "a", y: Any = "b") -> TooManyDefaults

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

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6565,6 +6565,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65656565

65666566
// Infer keyword arguments.
65676567
let mut default_types: Vec<Type<'db>> = vec![];
6568+
let mut defaults_kw: Option<&ast::Keyword> = None;
65686569
let mut rename_type = None;
65696570

65706571
for kw in keywords {
@@ -6575,6 +6576,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65756576
};
65766577
match arg.id.as_str() {
65776578
"defaults" if kind.is_collections() => {
6579+
defaults_kw = Some(kw);
65786580
// Extract element types from AST literals (using already-inferred types)
65796581
// or fall back to the inferred tuple spec.
65806582
match &kw.value {
@@ -6779,16 +6781,60 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
67796781
}
67806782

67816783
if let Some(mut field_names) = maybe_field_names {
6782-
// TODO: When `rename` is false (or not specified), emit diagnostics for:
6783-
// - Duplicate field names (e.g., `namedtuple("Foo", "x x")`)
6784-
// - Field names starting with underscore (e.g., `namedtuple("Bar", "_x")`)
6785-
// - Field names that are Python keywords (e.g., `namedtuple("Baz", "class")`)
6786-
// - Field names that are not valid identifiers
6787-
// These all raise ValueError at runtime. When `rename=True`, invalid names
6788-
// are automatically replaced with `_0`, `_1`, etc., so no diagnostic is needed.
6789-
6790-
// Apply rename logic.
6791-
if rename {
6784+
// When `rename` is false (or not specified), emit diagnostics for invalid
6785+
// field names. These all raise ValueError at runtime. When `rename=True`,
6786+
// invalid names are automatically replaced with `_0`, `_1`, etc., so no
6787+
// diagnostic is needed.
6788+
if !rename {
6789+
for (i, field_name) in field_names.iter().enumerate() {
6790+
let name_str = field_name.as_str();
6791+
6792+
if field_names[..i].iter().any(|f| f.as_str() == name_str)
6793+
&& let Some(builder) =
6794+
self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
6795+
{
6796+
let mut diagnostic = builder.into_diagnostic(format_args!(
6797+
"Duplicate field name `{name_str}` in `namedtuple()`"
6798+
));
6799+
diagnostic.set_primary_message(format_args!(
6800+
"Field `{name_str}` already defined; will raise `ValueError` at runtime"
6801+
));
6802+
}
6803+
6804+
if name_str.starts_with('_')
6805+
&& let Some(builder) =
6806+
self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
6807+
{
6808+
let mut diagnostic = builder.into_diagnostic(format_args!(
6809+
"Field name `{name_str}` in `namedtuple()` cannot start with an underscore"
6810+
));
6811+
diagnostic.set_primary_message(format_args!(
6812+
"Will raise `ValueError` at runtime"
6813+
));
6814+
} else if is_keyword(name_str)
6815+
&& let Some(builder) =
6816+
self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
6817+
{
6818+
let mut diagnostic = builder.into_diagnostic(format_args!(
6819+
"Field name `{name_str}` in `namedtuple()` cannot be a Python keyword"
6820+
));
6821+
diagnostic.set_primary_message(format_args!(
6822+
"Will raise `ValueError` at runtime"
6823+
));
6824+
} else if !is_identifier(name_str)
6825+
&& let Some(builder) =
6826+
self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
6827+
{
6828+
let mut diagnostic = builder.into_diagnostic(format_args!(
6829+
"Field name `{name_str}` in `namedtuple()` is not a valid identifier"
6830+
));
6831+
diagnostic.set_primary_message(format_args!(
6832+
"Will raise `ValueError` at runtime"
6833+
));
6834+
}
6835+
}
6836+
} else {
6837+
// Apply rename logic.
67926838
let mut seen_names = FxHashSet::<&str>::default();
67936839
for (i, field_name) in field_names.iter_mut().enumerate() {
67946840
let name_str = field_name.as_str();
@@ -6803,11 +6849,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
68036849
}
68046850
}
68056851

6806-
// Build fields with `Any` type and optional defaults.
6807-
// TODO: emit a diagnostic when `defaults_count > num_fields` (which would
6808-
// fail at runtime with `TypeError: Got more default values than field names`).
68096852
let num_fields = field_names.len();
6810-
let defaults_count = default_types.len().min(num_fields);
6853+
let defaults_count = default_types.len();
6854+
6855+
if defaults_count > num_fields
6856+
&& let Some(defaults_kw) = defaults_kw
6857+
&& let Some(builder) =
6858+
self.context.report_lint(&INVALID_NAMED_TUPLE, defaults_kw)
6859+
{
6860+
let mut diagnostic = builder.into_diagnostic(format_args!(
6861+
"Too many defaults for `namedtuple()`"
6862+
));
6863+
diagnostic.set_primary_message(format_args!(
6864+
"Got {defaults_count} default values but only {num_fields} field names"
6865+
));
6866+
diagnostic.info("This will raise `TypeError` at runtime");
6867+
}
6868+
6869+
let defaults_count = defaults_count.min(num_fields);
68116870
let fields = field_names
68126871
.iter()
68136872
.enumerate()

0 commit comments

Comments
 (0)