[ty] Emit diagnostics for invalid dynamic namedtuple fields

crates/ty_python_semantic/resources/mdtest/named_tuple.md

@@ -466,6 +466,27 @@ Point2 = collections.namedtuple("Point2", ["_x", "class"], rename=1)
 reveal_type(Point2)  # revealed: <class 'Point2'>
 reveal_type(Point2.__new__)  # revealed: (cls: type, _0: Any, _1: Any) -> Point2
 
+# Without `rename=True`, invalid field names emit diagnostics:
+# - Field names starting with underscore
+# error: [invalid-named-tuple] "Field name `_x` in `namedtuple()` cannot start with an underscore"
+Underscore = collections.namedtuple("Underscore", ["_x", "y"])
+reveal_type(Underscore)  # revealed: <class 'Underscore'>
+
+# - Python keywords
+# error: [invalid-named-tuple] "Field name `class` in `namedtuple()` cannot be a Python keyword"
+Keyword = collections.namedtuple("Keyword", ["x", "class"])
+reveal_type(Keyword)  # revealed: <class 'Keyword'>
+
+# - Duplicate field names
+# error: [invalid-named-tuple] "Duplicate field name `x` in `namedtuple()`"
+Duplicate = collections.namedtuple("Duplicate", ["x", "y", "x"])
+reveal_type(Duplicate)  # revealed: <class 'Duplicate'>
+
+# - Invalid identifiers (e.g., containing spaces)
+# error: [invalid-named-tuple] "Field name `not valid` in `namedtuple()` is not a valid identifier"
+Invalid = collections.namedtuple("Invalid", ["not valid", "ok"])
+reveal_type(Invalid)  # revealed: <class 'Invalid'>
+
 # `defaults` provides default values for the rightmost fields
 Person = collections.namedtuple("Person", ["name", "age", "city"], defaults=["Unknown"])
 reveal_type(Person)  # revealed: <class 'Person'>
@@ -483,8 +504,8 @@ reveal_type(person2.city)  # revealed: Any
 Config = collections.namedtuple("Config", ["host", "port"], module="myapp")
 reveal_type(Config)  # revealed: <class 'Config'>
 
-# When more defaults are provided than fields, we treat all fields as having defaults.
-# TODO: This should emit a diagnostic since it would fail at runtime.
+# When more defaults are provided than fields, an error is emitted.
+# error: [invalid-named-tuple] "Too many defaults for `namedtuple()`"
 TooManyDefaults = collections.namedtuple("TooManyDefaults", ["x", "y"], defaults=("a", "b", "c"))
 reveal_type(TooManyDefaults)  # revealed: <class 'TooManyDefaults'>
 reveal_type(TooManyDefaults.__new__)  # revealed: (cls: type, x: Any = "a", y: Any = "b") -> TooManyDefaults
@@ -846,7 +867,6 @@ class Person(NamedTuple):
 
 reveal_type(Person._field_defaults)  # revealed: dict[str, Any]
 reveal_type(Person._fields)  # revealed: tuple[Literal["name"], Literal["age"]]
-reveal_type(Person.__slots__)  # revealed: tuple[()]
 reveal_type(Person._make)  # revealed: bound method <class 'Person'>._make(iterable: Iterable[Any]) -> Person
 reveal_type(Person._asdict)  # revealed: def _asdict(self) -> dict[str, Any]
 reveal_type(Person._replace)  # revealed: (self: Self, *, name: str = ..., age: int | None = ...) -> Self
@@ -889,8 +909,6 @@ Person = namedtuple("Person", ["id", "name", "age"], defaults=[None])
 
 alice = Person(1, "Alice", 42)
 bob = Person(2, "Bob")
-
-reveal_type(Person.__slots__)  # revealed: tuple[()]
 ```
 
 ## `collections.namedtuple` with tuple variable field names

crates/ty_python_semantic/src/types/class.rs

@@ -3244,10 +3244,7 @@ impl<'db> StaticClassLiteral<'db> {
                         )
                     })
             }
-            (
-                CodeGeneratorKind::NamedTuple,
-                "__new__" | "_replace" | "__replace__" | "_fields" | "__slots__",
-            ) => {
+            (CodeGeneratorKind::NamedTuple, "__new__" | "_replace" | "__replace__" | "_fields") => {
                 let fields = self.fields(db, specialization, field_policy);
                 let fields_iter = fields.iter().map(|(name, field)| {
                     let default_ty = match &field.kind {
@@ -5215,10 +5212,6 @@ fn synthesize_namedtuple_class_member<'db>(
                 fields.map(|(field_name, _, _)| Type::string_literal(db, &field_name));
             Some(Type::heterogeneous_tuple(db, field_types))
         }
-        "__slots__" => {
-            // __slots__: tuple[()] - always empty for namedtuples
-            Some(Type::empty_tuple(db))
-        }
         "_replace" | "__replace__" => {
             if name == "__replace__" && Program::get(db).python_version(db) < PythonVersion::PY313 {
                 return None;
@@ -5543,10 +5536,7 @@ impl<'db> DynamicNamedTupleLiteral<'db> {
         // For fallback members from NamedTupleFallback, apply type mapping to handle
         // `Self` types. The explicitly synthesized members (__new__, _fields, _replace,
         // __replace__) don't need this mapping.
-        if matches!(
-            name,
-            "__new__" | "_fields" | "_replace" | "__replace__" | "__slots__"
-        ) {
+        if matches!(name, "__new__" | "_fields" | "_replace" | "__replace__") {
             result
         } else {
             result.map(|ty| {

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

@@ -6565,6 +6565,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         // Infer keyword arguments.
         let mut default_types: Vec<Type<'db>> = vec![];
+        let mut defaults_kw: Option<&ast::Keyword> = None;
         let mut rename_type = None;
 
         for kw in keywords {
@@ -6575,6 +6576,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             };
             match arg.id.as_str() {
                 "defaults" if kind.is_collections() => {
+                    defaults_kw = Some(kw);
                     // Extract element types from AST literals (using already-inferred types)
                     // or fall back to the inferred tuple spec.
                     match &kw.value {
@@ -6779,16 +6781,60 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                     }
 
                     if let Some(mut field_names) = maybe_field_names {
-                        // TODO: When `rename` is false (or not specified), emit diagnostics for:
-                        // - Duplicate field names (e.g., `namedtuple("Foo", "x x")`)
-                        // - Field names starting with underscore (e.g., `namedtuple("Bar", "_x")`)
-                        // - Field names that are Python keywords (e.g., `namedtuple("Baz", "class")`)
-                        // - Field names that are not valid identifiers
-                        // These all raise ValueError at runtime. When `rename=True`, invalid names
-                        // are automatically replaced with `_0`, `_1`, etc., so no diagnostic is needed.
+                        // When `rename` is false (or not specified), emit diagnostics for invalid
+                        // field names. These all raise ValueError at runtime. When `rename=True`,
+                        // invalid names are automatically replaced with `_0`, `_1`, etc., so no
+                        // diagnostic is needed.
+                        if !rename {
+                            for (i, field_name) in field_names.iter().enumerate() {
+                                let name_str = field_name.as_str();
 
-                        // Apply rename logic.
-                        if rename {
+                                if field_names[..i].iter().any(|f| f.as_str() == name_str)
+                                    && let Some(builder) =
+                                        self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
+                                {
+                                    let mut diagnostic = builder.into_diagnostic(format_args!(
+                                        "Duplicate field name `{name_str}` in `namedtuple()`"
+                                    ));
+                                    diagnostic.set_primary_message(format_args!(
+                                        "Field `{name_str}` already defined; will raise `ValueError` at runtime"
+                                    ));
+                                }
+
+                                if name_str.starts_with('_')
+                                    && let Some(builder) =
+                                        self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
+                                {
+                                    let mut diagnostic = builder.into_diagnostic(format_args!(
+                                        "Field name `{name_str}` in `namedtuple()` cannot start with an underscore"
+                                    ));
+                                    diagnostic.set_primary_message(format_args!(
+                                        "Will raise `ValueError` at runtime"
+                                    ));
+                                } else if is_keyword(name_str)
+                                    && let Some(builder) =
+                                        self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
+                                {
+                                    let mut diagnostic = builder.into_diagnostic(format_args!(
+                                        "Field name `{name_str}` in `namedtuple()` cannot be a Python keyword"
+                                    ));
+                                    diagnostic.set_primary_message(format_args!(
+                                        "Will raise `ValueError` at runtime"
+                                    ));
+                                } else if !is_identifier(name_str)
+                                    && let Some(builder) =
+                                        self.context.report_lint(&INVALID_NAMED_TUPLE, fields_arg)
+                                {
+                                    let mut diagnostic = builder.into_diagnostic(format_args!(
+                                        "Field name `{name_str}` in `namedtuple()` is not a valid identifier"
+                                    ));
+                                    diagnostic.set_primary_message(format_args!(
+                                        "Will raise `ValueError` at runtime"
+                                    ));
+                                }
+                            }
+                        } else {
+                            // Apply rename logic.
                             let mut seen_names = FxHashSet::<&str>::default();
                             for (i, field_name) in field_names.iter_mut().enumerate() {
                                 let name_str = field_name.as_str();
@@ -6803,11 +6849,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                             }
                         }
 
-                        // Build fields with `Any` type and optional defaults.
-                        // TODO: emit a diagnostic when `defaults_count > num_fields` (which would
-                        // fail at runtime with `TypeError: Got more default values than field names`).
                         let num_fields = field_names.len();
-                        let defaults_count = default_types.len().min(num_fields);
+                        let defaults_count = default_types.len();
+
+                        if defaults_count > num_fields
+                            && let Some(defaults_kw) = defaults_kw
+                            && let Some(builder) =
+                                self.context.report_lint(&INVALID_NAMED_TUPLE, defaults_kw)
+                        {
+                            let mut diagnostic = builder.into_diagnostic(format_args!(
+                                "Too many defaults for `namedtuple()`"
+                            ));
+                            diagnostic.set_primary_message(format_args!(
+                                "Got {defaults_count} default values but only {num_fields} field names; \
+                                will raise `TypeError` at runtime"
+                            ));
+                        }
+
+                        let defaults_count = defaults_count.min(num_fields);
                         let fields = field_names
                             .iter()
                             .enumerate()

playground/ty/src/Editor/Chrome.tsx

@@ -163,13 +163,6 @@ export default function Chrome({
     [workspace, files.index, onRemoveFile],
   );
 
-  const handleChange = useCallback(
-    (content: string) => {
-      onChangeFile(workspace, content);
-    },
-    [onChangeFile, workspace],
-  );
-
   const { defaultLayout, onLayoutChange } = useDefaultLayout({
     groupId: "editor-diagnostics",
     storage: localStorage,
@@ -228,7 +221,7 @@ export default function Chrome({
                     diagnostics={checkResult.diagnostics}
                     workspace={workspace}
                     onMount={handleEditorMount}
-                    onChange={handleChange}
+                    onChange={(content) => onChangeFile(workspace, content)}
                     onOpenFile={onSelectFile}
                     onVendoredFileChange={onSelectVendoredFile}
                     onBackToUserFile={handleBackToUserFile}

playground/ty/src/Playground.tsx

@@ -58,7 +58,7 @@ export default function Playground() {
     }
   }, [files]);
 
-  const handleFileAdded = useCallback((workspace: Workspace, name: string) => {
+  const handleFileAdded = (workspace: Workspace, name: string) => {
     let handle = null;
 
     if (name === SETTINGS_FILE_NAME) {
@@ -68,74 +68,69 @@ export default function Playground() {
     }
 
     dispatchFiles({ type: "add", name, handle, content: "" });
-  }, []);
+  };
 
-  const handleFileChanged = useCallback(
-    (workspace: Workspace, content: string) => {
-      if (files.selected == null) {
-        return;
-      }
+  const handleFileChanged = (workspace: Workspace, content: string) => {
+    if (files.selected == null) {
+      return;
+    }
 
-      const handle = files.handles[files.selected];
+    dispatchFiles({
+      type: "change",
+      id: files.selected,
+      content,
+    });
 
-      if (handle != null) {
-        updateFile(workspace, handle, content, setError);
-      } else if (fileName === SETTINGS_FILE_NAME) {
-        updateOptions(workspace, content, setError);
-      }
+    const handle = files.handles[files.selected];
 
-      dispatchFiles({
-        type: "change",
-        id: files.selected,
-        content,
-      });
-    },
-    [fileName, files.handles, files.selected],
-  );
+    if (handle != null) {
+      updateFile(workspace, handle, content, setError);
+    } else if (fileName === SETTINGS_FILE_NAME) {
+      updateOptions(workspace, content, setError);
+    }
+  };
 
-  const handleFileRenamed = useCallback(
-    (workspace: Workspace, file: FileId, newName: string) => {
-      if (newName.startsWith("/")) {
-        setError("File names cannot start with '/'.");
-        return;
-      }
-      if (newName.startsWith("vendored:")) {
-        setError("File names cannot start with 'vendored:'.");
-        return;
-      }
+  const handleFileRenamed = (
+    workspace: Workspace,
+    file: FileId,
+    newName: string,
+  ) => {
+    if (newName.startsWith("/")) {
+      setError("File names cannot start with '/'.");
+      return;
+    }
+    if (newName.startsWith("vendored:")) {
+      setError("File names cannot start with 'vendored:'.");
+      return;
+    }
 
-      const handle = files.handles[file];
-      let newHandle: FileHandle | null = null;
-      if (handle == null) {
-        updateOptions(workspace, null, setError);
-      } else {
-        workspace.closeFile(handle);
-      }
+    const handle = files.handles[file];
+    let newHandle: FileHandle | null = null;
+    if (handle == null) {
+      updateOptions(workspace, null, setError);
+    } else {
+      workspace.closeFile(handle);
+    }
 
-      if (newName === SETTINGS_FILE_NAME) {
-        updateOptions(workspace, files.contents[file], setError);
-      } else {
-        newHandle = workspace.openFile(newName, files.contents[file]);
-      }
+    if (newName === SETTINGS_FILE_NAME) {
+      updateOptions(workspace, files.contents[file], setError);
+    } else {
+      newHandle = workspace.openFile(newName, files.contents[file]);
+    }
 
-      dispatchFiles({ type: "rename", id: file, to: newName, newHandle });
-    },
-    [files.contents, files.handles],
-  );
+    dispatchFiles({ type: "rename", id: file, to: newName, newHandle });
+  };
 
-  const handleFileRemoved = useCallback(
-    (workspace: Workspace, file: FileId) => {
-      const handle = files.handles[file];
-      if (handle == null) {
-        updateOptions(workspace, null, setError);
-      } else {
-        workspace.closeFile(handle);
-      }
+  const handleFileRemoved = (workspace: Workspace, file: FileId) => {
+    const handle = files.handles[file];
+    if (handle == null) {
+      updateOptions(workspace, null, setError);
+    } else {
+      workspace.closeFile(handle);
+    }
 
-      dispatchFiles({ type: "remove", id: file });
-    },
-    [files.handles],
-  );
+    dispatchFiles({ type: "remove", id: file });
+  };
 
   const handleFileSelected = useCallback((file: FileId) => {
     dispatchFiles({ type: "selectFile", id: file });