[ty] Emit diagnostic for functional TypedDict with non-literal name#24331
[ty] Emit diagnostic for functional TypedDict with non-literal name#24331charliermarsh merged 2 commits intomainfrom
Conversation
|
|
||
| def g(x: str) -> None: | ||
| # error: [invalid-argument-type] "The first argument to `TypedDict` must be a string literal" | ||
| TypedDict(x, {}) |
There was a problem hiding this comment.
I could see a case for not raising this...?
There was a problem hiding this comment.
Yeah, I'd be inclined to avoid the diagnostic on this one and also simplify the code a little, e.g.
diff --git a/crates/ty_python_semantic/resources/mdtest/typed_dict.md b/crates/ty_python_semantic/resources/mdtest/typed_dict.md
index 7a7de94f98..ff0b5dce9a 100644
--- a/crates/ty_python_semantic/resources/mdtest/typed_dict.md
+++ b/crates/ty_python_semantic/resources/mdtest/typed_dict.md
@@ -2529,19 +2529,18 @@ Movie2 = TypedDict("Movie2", name=str, year=int)
```py
from typing_extensions import TypedDict
-# error: [invalid-argument-type] "Invalid argument to parameter `typename` of `TypedDict()`"
+# error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "Bad1", got variable of type `Literal[123]`"
Bad1 = TypedDict(123, {"name": str})
-# error: [invalid-argument-type] "The name of a `TypedDict` (`WrongName`) must match the name of the variable it is assigned to (`BadTypedDict3`)"
+# error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "BadTypedDict3", got "WrongName""
BadTypedDict3 = TypedDict("WrongName", {"name": str})
def f(x: str) -> None:
- # error: [invalid-argument-type] "The first argument to `TypedDict` must be the string literal `Y`"
+ # error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "Y", got variable of type `str`"
Y = TypedDict(x, {})
def g(x: str) -> None:
- # error: [invalid-argument-type] "The first argument to `TypedDict` must be a string literal"
- TypedDict(x, {})
+ TypedDict(x, {}) # fine
name = "GoodTypedDict"
GoodTypedDict = TypedDict(name, {"name": str})
diff --git a/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs b/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
index c22fd8367e..e7bff0d29a 100644
--- a/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
+++ b/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
@@ -158,45 +158,30 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
);
}
- let name = if let Some(literal) = name_type.as_string_literal() {
- let name = literal.value(db);
-
- if let Some(assigned_name) = definition.and_then(|definition| definition.name(db))
- && name != assigned_name
- && let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg)
- {
- builder.into_diagnostic(format_args!(
- "The name of a `TypedDict` (`{name}`) must match \
- the name of the variable it is assigned to (`{assigned_name}`)"
+ let name = name_type
+ .as_string_literal()
+ .map(|literal| Name::new(literal.value(db)));
+
+ if let Some(definition) = definition
+ && let Some(assigned_name) = definition.name(db)
+ && Some(&*assigned_name) != name.as_deref()
+ && let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg)
+ {
+ let mut diagnostic =
+ builder.into_diagnostic("TypedDict name must match the variable it is assigned to");
+ if let Some(name) = name.as_deref() {
+ diagnostic.set_primary_message(format_args!(
+ "Expected \"{assigned_name}\", got \"{name}\""
+ ));
+ } else {
+ diagnostic.set_primary_message(format_args!(
+ "Expected \"{assigned_name}\", got variable of type `{}`",
+ name_type.display(db)
));
}
+ }
- Name::new(name)
- } else {
- let is_str = name_type.is_assignable_to(db, KnownClass::Str.to_instance(db));
- if let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg) {
- if let Some(assigned_name) = definition.and_then(|definition| definition.name(db))
- && is_str
- {
- builder.into_diagnostic(format_args!(
- "The first argument to `TypedDict` must be the string literal `{assigned_name}`"
- ));
- } else if is_str {
- builder.into_diagnostic(
- "The first argument to `TypedDict` must be a string literal",
- );
- } else {
- let mut diagnostic = builder.into_diagnostic(format_args!(
- "Invalid argument to parameter `typename` of `TypedDict()`"
- ));
- diagnostic.set_primary_message(format_args!(
- "Expected `str`, found `{}`",
- name_type.display(db)
- ));
- }
- }
- Name::new_static("<unknown>")
- };
+ let name = name.unwrap_or_else(|| Name::new_static("<unknown>"));
if let Some(definition) = definition {
self.deferred.insert(definition);
Typing conformance resultsThe percentage of diagnostics emitted that were expected errors held steady at 86.61%. The percentage of expected errors that received a diagnostic held steady at 81.56%. The number of fully passing files held steady at 70/132. SummaryHow are test cases classified?Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (
True positives changed (1)1 diagnostic
|
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
0 | 40 | 0 |
invalid-return-type |
0 | 1 | 0 |
| Total | 0 | 41 | 0 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
|
|
||
| def g(x: str) -> None: | ||
| # error: [invalid-argument-type] "The first argument to `TypedDict` must be a string literal" | ||
| TypedDict(x, {}) |
There was a problem hiding this comment.
Yeah, I'd be inclined to avoid the diagnostic on this one and also simplify the code a little, e.g.
diff --git a/crates/ty_python_semantic/resources/mdtest/typed_dict.md b/crates/ty_python_semantic/resources/mdtest/typed_dict.md
index 7a7de94f98..ff0b5dce9a 100644
--- a/crates/ty_python_semantic/resources/mdtest/typed_dict.md
+++ b/crates/ty_python_semantic/resources/mdtest/typed_dict.md
@@ -2529,19 +2529,18 @@ Movie2 = TypedDict("Movie2", name=str, year=int)
```py
from typing_extensions import TypedDict
-# error: [invalid-argument-type] "Invalid argument to parameter `typename` of `TypedDict()`"
+# error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "Bad1", got variable of type `Literal[123]`"
Bad1 = TypedDict(123, {"name": str})
-# error: [invalid-argument-type] "The name of a `TypedDict` (`WrongName`) must match the name of the variable it is assigned to (`BadTypedDict3`)"
+# error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "BadTypedDict3", got "WrongName""
BadTypedDict3 = TypedDict("WrongName", {"name": str})
def f(x: str) -> None:
- # error: [invalid-argument-type] "The first argument to `TypedDict` must be the string literal `Y`"
+ # error: [invalid-argument-type] "TypedDict name must match the variable it is assigned to: Expected "Y", got variable of type `str`"
Y = TypedDict(x, {})
def g(x: str) -> None:
- # error: [invalid-argument-type] "The first argument to `TypedDict` must be a string literal"
- TypedDict(x, {})
+ TypedDict(x, {}) # fine
name = "GoodTypedDict"
GoodTypedDict = TypedDict(name, {"name": str})
diff --git a/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs b/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
index c22fd8367e..e7bff0d29a 100644
--- a/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
+++ b/crates/ty_python_semantic/src/types/infer/builder/typed_dict.rs
@@ -158,45 +158,30 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
);
}
- let name = if let Some(literal) = name_type.as_string_literal() {
- let name = literal.value(db);
-
- if let Some(assigned_name) = definition.and_then(|definition| definition.name(db))
- && name != assigned_name
- && let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg)
- {
- builder.into_diagnostic(format_args!(
- "The name of a `TypedDict` (`{name}`) must match \
- the name of the variable it is assigned to (`{assigned_name}`)"
+ let name = name_type
+ .as_string_literal()
+ .map(|literal| Name::new(literal.value(db)));
+
+ if let Some(definition) = definition
+ && let Some(assigned_name) = definition.name(db)
+ && Some(&*assigned_name) != name.as_deref()
+ && let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg)
+ {
+ let mut diagnostic =
+ builder.into_diagnostic("TypedDict name must match the variable it is assigned to");
+ if let Some(name) = name.as_deref() {
+ diagnostic.set_primary_message(format_args!(
+ "Expected \"{assigned_name}\", got \"{name}\""
+ ));
+ } else {
+ diagnostic.set_primary_message(format_args!(
+ "Expected \"{assigned_name}\", got variable of type `{}`",
+ name_type.display(db)
));
}
+ }
- Name::new(name)
- } else {
- let is_str = name_type.is_assignable_to(db, KnownClass::Str.to_instance(db));
- if let Some(builder) = self.context.report_lint(&INVALID_ARGUMENT_TYPE, name_arg) {
- if let Some(assigned_name) = definition.and_then(|definition| definition.name(db))
- && is_str
- {
- builder.into_diagnostic(format_args!(
- "The first argument to `TypedDict` must be the string literal `{assigned_name}`"
- ));
- } else if is_str {
- builder.into_diagnostic(
- "The first argument to `TypedDict` must be a string literal",
- );
- } else {
- let mut diagnostic = builder.into_diagnostic(format_args!(
- "Invalid argument to parameter `typename` of `TypedDict()`"
- ));
- diagnostic.set_primary_message(format_args!(
- "Expected `str`, found `{}`",
- name_type.display(db)
- ));
- }
- }
- Name::new_static("<unknown>")
- };
+ let name = name.unwrap_or_else(|| Name::new_static("<unknown>"));
if let Some(definition) = definition {
self.deferred.insert(definition);* main: [ty] Add missing test case for inline functional TypedDict with an invalid type passed to the `name` parameter (#24334) [ty] Use `_cls` as argument name for `collections.namedtuple` (#24333) [ty] Emit diagnostic for functional TypedDict with non-literal name (#24331) Add `nested-string-quote-style` formatting option (#24312) `RUF010`: Mark fix as unsafe when it deletes a comment [ty] Fix semantic token classification for properties accessed on instances (#24065) publish installers to `/installers/ruff/latest` on the mirror (#24247)
Summary
See: #24295 (comment).