From 2d4fae45d9e33ee78f5a727001888301d58af9dc Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 22 Feb 2023 18:23:25 -0500 Subject: [PATCH] Avoid flagging unfixable `TypedDict` and `NamedTuple` definitions (#3148) --- .../test/fixtures/pyupgrade/UP013.py | 30 +-- .../test/fixtures/pyupgrade/UP014.py | 17 +- ...convert_named_tuple_functional_to_class.rs | 58 +++-- .../convert_typed_dict_functional_to_class.rs | 55 +++-- ...ff__rules__pyupgrade__tests__UP013.py.snap | 204 +++++++++--------- ...ff__rules__pyupgrade__tests__UP014.py.snap | 34 ++- 6 files changed, 202 insertions(+), 196 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP013.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP013.py index 66f041423c..8c187c1a44 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP013.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP013.py @@ -2,36 +2,36 @@ from typing import TypedDict, NotRequired, Literal import typing # dict literal -MyType1 = TypedDict("MyType1", {"a": int, "b": str}) +MyType = TypedDict("MyType", {"a": int, "b": str}) # dict call -MyType2 = TypedDict("MyType2", dict(a=int, b=str)) +MyType = TypedDict("MyType", dict(a=int, b=str)) # kwargs -MyType3 = TypedDict("MyType3", a=int, b=str) +MyType = TypedDict("MyType", a=int, b=str) # Empty TypedDict -MyType4 = TypedDict("MyType4") +MyType = TypedDict("MyType") # Literal values -MyType5 = TypedDict("MyType5", {"a": "hello"}) -MyType6 = TypedDict("MyType6", a="hello") +MyType = TypedDict("MyType", {"a": "hello"}) +MyType = TypedDict("MyType", a="hello") # NotRequired -MyType7 = TypedDict("MyType7", {"a": NotRequired[dict]}) +MyType = TypedDict("MyType", {"a": NotRequired[dict]}) # total -MyType8 = TypedDict("MyType8", {"x": int, "y": int}, total=False) - -# invalid identifiers -MyType9 = TypedDict("MyType9", {"in": int, "x-y": int}) +MyType = TypedDict("MyType", {"x": int, "y": int}, total=False) # using Literal type -MyType10 = TypedDict("MyType10", {"key": Literal["value"]}) +MyType = TypedDict("MyType", {"key": Literal["value"]}) # using namespace TypedDict -MyType11 = typing.TypedDict("MyType11", {"key": int}) +MyType = typing.TypedDict("MyType", {"key": int}) -# unpacking +# invalid identifiers (OK) +MyType = TypedDict("MyType", {"in": int, "x-y": int}) + +# unpacking (OK) c = {"c": float} -MyType12 = TypedDict("MyType1", {"a": int, "b": str, **c}) +MyType = TypedDict("MyType", {"a": int, "b": str, **c}) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py index 41abf7ff15..812f43e5e3 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py @@ -2,21 +2,24 @@ from typing import NamedTuple import typing # with complex annotations -NT1 = NamedTuple("NT1", [("a", int), ("b", tuple[str, ...])]) +MyType = NamedTuple("MyType", [("a", int), ("b", tuple[str, ...])]) # with default values as list -NT2 = NamedTuple( - "NT2", +MyType = NamedTuple( + "MyType", [("a", int), ("b", str), ("c", list[bool])], defaults=["foo", [True]], ) # with namespace -NT3 = typing.NamedTuple("NT3", [("a", int), ("b", str)]) +MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) -# with too many default values -NT4 = NamedTuple( - "NT4", +# too many default values (OK) +MyType = NamedTuple( + "MyType", [("a", int), ("b", str)], defaults=[1, "bar", "baz"], ) + +# invalid identifiers (OK) +MyType = NamedTuple("MyType", [("x-y", int), ("b", tuple[str, ...])]) diff --git a/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs b/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs index 61a2098276..7c46593ad0 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs @@ -1,9 +1,10 @@ use anyhow::{bail, Result}; use log::debug; +use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Keyword, Stmt, StmtKind}; + use ruff_macros::{define_violation, derive_message_formats}; use ruff_python::identifiers::is_identifier; use ruff_python::keyword::KWLIST; -use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Keyword, Stmt, StmtKind}; use crate::ast::helpers::{create_expr, create_stmt, unparse_stmt}; use crate::ast::types::Range; @@ -11,23 +12,29 @@ use crate::checkers::ast::Checker; use crate::fix::Fix; use crate::registry::Diagnostic; use crate::source_code::Stylist; -use crate::violation::AlwaysAutofixableViolation; +use crate::violation::{Availability, Violation}; +use crate::AutofixKind; define_violation!( pub struct ConvertNamedTupleFunctionalToClass { pub name: String, + pub fixable: bool, } ); -impl AlwaysAutofixableViolation for ConvertNamedTupleFunctionalToClass { +impl Violation for ConvertNamedTupleFunctionalToClass { + const AUTOFIX: Option = Some(AutofixKind::new(Availability::Sometimes)); + #[derive_message_formats] fn message(&self) -> String { - let ConvertNamedTupleFunctionalToClass { name } = self; + let ConvertNamedTupleFunctionalToClass { name, .. } = self; format!("Convert `{name}` from `NamedTuple` functional to class syntax") } - fn autofix_title(&self) -> String { - let ConvertNamedTupleFunctionalToClass { name } = self; - format!("Convert `{name}` to class syntax") + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|ConvertNamedTupleFunctionalToClass { name, .. }| { + format!("Convert `{name}` to class syntax") + }) } } @@ -172,28 +179,33 @@ pub fn convert_named_tuple_functional_to_class( { return; }; + + let properties = match match_defaults(keywords) + .and_then(|defaults| create_properties_from_args(args, defaults)) + { + Ok(properties) => properties, + Err(err) => { + debug!("Skipping `NamedTuple` \"{typename}\": {err}"); + return; + } + }; + // TODO(charlie): Preserve indentation, to remove the first-column requirement. + let fixable = stmt.location.column() == 0; let mut diagnostic = Diagnostic::new( ConvertNamedTupleFunctionalToClass { name: typename.to_string(), + fixable, }, Range::from_located(stmt), ); - // TODO(charlie): Preserve indentation, to remove the first-column requirement. - if checker.patch(diagnostic.kind.rule()) && stmt.location.column() == 0 { - match match_defaults(keywords) - .and_then(|defaults| create_properties_from_args(args, defaults)) - { - Ok(properties) => { - diagnostic.amend(convert_to_class( - stmt, - typename, - properties, - base_class, - checker.stylist, - )); - } - Err(err) => debug!("Skipping ineligible `NamedTuple` \"{typename}\": {err}"), - }; + if fixable && checker.patch(diagnostic.kind.rule()) { + diagnostic.amend(convert_to_class( + stmt, + typename, + properties, + base_class, + checker.stylist, + )); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs b/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs index 9dc2a6a314..f58a5e687e 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs @@ -1,9 +1,10 @@ use anyhow::{bail, Result}; use log::debug; +use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Keyword, Stmt, StmtKind}; + use ruff_macros::{define_violation, derive_message_formats}; use ruff_python::identifiers::is_identifier; use ruff_python::keyword::KWLIST; -use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Keyword, Stmt, StmtKind}; use crate::ast::helpers::{create_expr, create_stmt, unparse_stmt}; use crate::ast::types::Range; @@ -11,23 +12,29 @@ use crate::checkers::ast::Checker; use crate::fix::Fix; use crate::registry::Diagnostic; use crate::source_code::Stylist; -use crate::violation::AlwaysAutofixableViolation; +use crate::violation::{Availability, Violation}; +use crate::AutofixKind; define_violation!( pub struct ConvertTypedDictFunctionalToClass { pub name: String, + pub fixable: bool, } ); -impl AlwaysAutofixableViolation for ConvertTypedDictFunctionalToClass { +impl Violation for ConvertTypedDictFunctionalToClass { + const AUTOFIX: Option = Some(AutofixKind::new(Availability::Sometimes)); + #[derive_message_formats] fn message(&self) -> String { - let ConvertTypedDictFunctionalToClass { name } = self; + let ConvertTypedDictFunctionalToClass { name, .. } = self; format!("Convert `{name}` from `TypedDict` functional to class syntax") } - fn autofix_title(&self) -> String { - let ConvertTypedDictFunctionalToClass { name } = self; - format!("Convert `{name}` to class syntax") + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|ConvertTypedDictFunctionalToClass { name, .. }| { + format!("Convert `{name}` to class syntax") + }) } } @@ -219,27 +226,31 @@ pub fn convert_typed_dict_functional_to_class( return; }; + let (body, total_keyword) = match match_properties_and_total(args, keywords) { + Ok((body, total_keyword)) => (body, total_keyword), + Err(err) => { + debug!("Skipping ineligible `TypedDict` \"{class_name}\": {err}"); + return; + } + }; + // TODO(charlie): Preserve indentation, to remove the first-column requirement. + let fixable = stmt.location.column() == 0; let mut diagnostic = Diagnostic::new( ConvertTypedDictFunctionalToClass { name: class_name.to_string(), + fixable, }, Range::from_located(stmt), ); - // TODO(charlie): Preserve indentation, to remove the first-column requirement. - if checker.patch(diagnostic.kind.rule()) && stmt.location.column() == 0 { - match match_properties_and_total(args, keywords) { - Ok((body, total_keyword)) => { - diagnostic.amend(convert_to_class( - stmt, - class_name, - body, - total_keyword, - base_class, - checker.stylist, - )); - } - Err(err) => debug!("Skipping ineligible `TypedDict` \"{class_name}\": {err}"), - }; + if fixable && checker.patch(diagnostic.kind.rule()) { + diagnostic.amend(convert_to_class( + stmt, + class_name, + body, + total_keyword, + base_class, + checker.stylist, + )); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013.py.snap index 400bc3f598..6729d468f7 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013.py.snap @@ -4,204 +4,192 @@ expression: diagnostics --- - kind: ConvertTypedDictFunctionalToClass: - name: MyType1 + name: MyType + fixable: true location: row: 5 column: 0 end_location: row: 5 - column: 52 - fix: - content: "class MyType1(TypedDict):\n a: int\n b: str" - location: - row: 5 - column: 0 - end_location: - row: 5 - column: 52 - parent: ~ -- kind: - ConvertTypedDictFunctionalToClass: - name: MyType2 - location: - row: 8 - column: 0 - end_location: - row: 8 column: 50 fix: - content: "class MyType2(TypedDict):\n a: int\n b: str" + content: "class MyType(TypedDict):\n a: int\n b: str" location: - row: 8 + row: 5 column: 0 end_location: - row: 8 + row: 5 column: 50 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType3 + name: MyType + fixable: true + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 48 + fix: + content: "class MyType(TypedDict):\n a: int\n b: str" + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 48 + parent: ~ +- kind: + ConvertTypedDictFunctionalToClass: + name: MyType + fixable: true location: row: 11 column: 0 end_location: row: 11 - column: 44 + column: 42 fix: - content: "class MyType3(TypedDict):\n a: int\n b: str" + content: "class MyType(TypedDict):\n a: int\n b: str" location: row: 11 column: 0 end_location: row: 11 + column: 42 + parent: ~ +- kind: + ConvertTypedDictFunctionalToClass: + name: MyType + fixable: true + location: + row: 14 + column: 0 + end_location: + row: 14 + column: 28 + fix: + content: "class MyType(TypedDict):\n pass" + location: + row: 14 + column: 0 + end_location: + row: 14 + column: 28 + parent: ~ +- kind: + ConvertTypedDictFunctionalToClass: + name: MyType + fixable: true + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 44 + fix: + content: "class MyType(TypedDict):\n a: \"hello\"" + location: + row: 17 + column: 0 + end_location: + row: 17 column: 44 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType4 - location: - row: 14 - column: 0 - end_location: - row: 14 - column: 30 - fix: - content: "class MyType4(TypedDict):\n pass" - location: - row: 14 - column: 0 - end_location: - row: 14 - column: 30 - parent: ~ -- kind: - ConvertTypedDictFunctionalToClass: - name: MyType5 - location: - row: 17 - column: 0 - end_location: - row: 17 - column: 46 - fix: - content: "class MyType5(TypedDict):\n a: \"hello\"" - location: - row: 17 - column: 0 - end_location: - row: 17 - column: 46 - parent: ~ -- kind: - ConvertTypedDictFunctionalToClass: - name: MyType6 + name: MyType + fixable: true location: row: 18 column: 0 end_location: row: 18 - column: 41 + column: 39 fix: - content: "class MyType6(TypedDict):\n a: \"hello\"" + content: "class MyType(TypedDict):\n a: \"hello\"" location: row: 18 column: 0 end_location: row: 18 - column: 41 + column: 39 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType7 + name: MyType + fixable: true location: row: 21 column: 0 end_location: row: 21 - column: 56 + column: 54 fix: - content: "class MyType7(TypedDict):\n a: NotRequired[dict]" + content: "class MyType(TypedDict):\n a: NotRequired[dict]" location: row: 21 column: 0 end_location: row: 21 - column: 56 + column: 54 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType8 + name: MyType + fixable: true location: row: 24 column: 0 end_location: row: 24 - column: 65 + column: 63 fix: - content: "class MyType8(TypedDict, total=False):\n x: int\n y: int" + content: "class MyType(TypedDict, total=False):\n x: int\n y: int" location: row: 24 column: 0 end_location: row: 24 - column: 65 + column: 63 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType9 + name: MyType + fixable: true location: row: 27 column: 0 end_location: row: 27 column: 55 - fix: ~ + fix: + content: "class MyType(TypedDict):\n key: Literal[\"value\"]" + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 55 parent: ~ - kind: ConvertTypedDictFunctionalToClass: - name: MyType10 + name: MyType + fixable: true location: row: 30 column: 0 end_location: row: 30 - column: 59 + column: 49 fix: - content: "class MyType10(TypedDict):\n key: Literal[\"value\"]" + content: "class MyType(typing.TypedDict):\n key: int" location: row: 30 column: 0 end_location: row: 30 - column: 59 - parent: ~ -- kind: - ConvertTypedDictFunctionalToClass: - name: MyType11 - location: - row: 33 - column: 0 - end_location: - row: 33 - column: 53 - fix: - content: "class MyType11(typing.TypedDict):\n key: int" - location: - row: 33 - column: 0 - end_location: - row: 33 - column: 53 - parent: ~ -- kind: - ConvertTypedDictFunctionalToClass: - name: MyType12 - location: - row: 37 - column: 0 - end_location: - row: 37 - column: 58 - fix: ~ + column: 49 parent: ~ diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP014.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP014.py.snap index cd5dd8c884..fc620b0b16 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP014.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP014.py.snap @@ -4,25 +4,27 @@ expression: diagnostics --- - kind: ConvertNamedTupleFunctionalToClass: - name: NT1 + name: MyType + fixable: true location: row: 5 column: 0 end_location: row: 5 - column: 61 + column: 67 fix: - content: "class NT1(NamedTuple):\n a: int\n b: tuple[str, ...]" + content: "class MyType(NamedTuple):\n a: int\n b: tuple[str, ...]" location: row: 5 column: 0 end_location: row: 5 - column: 61 + column: 67 parent: ~ - kind: ConvertNamedTupleFunctionalToClass: - name: NT2 + name: MyType + fixable: true location: row: 8 column: 0 @@ -30,7 +32,7 @@ expression: diagnostics row: 12 column: 1 fix: - content: "class NT2(NamedTuple):\n a: int\n b: str = \"foo\"\n c: list[bool] = [True]" + content: "class MyType(NamedTuple):\n a: int\n b: str = \"foo\"\n c: list[bool] = [True]" location: row: 8 column: 0 @@ -40,31 +42,21 @@ expression: diagnostics parent: ~ - kind: ConvertNamedTupleFunctionalToClass: - name: NT3 + name: MyType + fixable: true location: row: 15 column: 0 end_location: row: 15 - column: 56 + column: 62 fix: - content: "class NT3(typing.NamedTuple):\n a: int\n b: str" + content: "class MyType(typing.NamedTuple):\n a: int\n b: str" location: row: 15 column: 0 end_location: row: 15 - column: 56 - parent: ~ -- kind: - ConvertNamedTupleFunctionalToClass: - name: NT4 - location: - row: 18 - column: 0 - end_location: - row: 22 - column: 1 - fix: ~ + column: 62 parent: ~