From 7bdd8a8b6ebc206959c8dd61b5971d262a44796e Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 14:33:27 +0900 Subject: [PATCH 1/7] Do not fix NamedTuple calls containing both a list of fields and keywords --- ...convert_named_tuple_functional_to_class.rs | 59 ++++++------------- ...ff__rules__pyupgrade__tests__UP014.py.snap | 31 ---------- 2 files changed, 18 insertions(+), 72 deletions(-) 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 63298260cfab3..101350bbdc928 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 @@ -93,11 +93,7 @@ fn match_named_tuple_assign<'a>( /// Generate a `Stmt::AnnAssign` representing the provided property /// definition. -fn create_property_assignment_stmt( - property: &str, - annotation: &Expr, - value: Option<&Expr>, -) -> Stmt { +fn create_property_assignment_stmt(property: &str, annotation: &Expr) -> Stmt { ast::StmtAnnAssign { target: Box::new( ast::ExprName { @@ -108,34 +104,15 @@ fn create_property_assignment_stmt( .into(), ), annotation: Box::new(annotation.clone()), - value: value.map(|value| Box::new(value.clone())), + value: None, simple: true, range: TextRange::default(), } .into() } -/// Match the `defaults` keyword in a `NamedTuple(...)` call. -fn match_defaults(keywords: &[Keyword]) -> Result<&[Expr]> { - let defaults = keywords.iter().find(|keyword| { - if let Some(arg) = &keyword.arg { - arg == "defaults" - } else { - false - } - }); - match defaults { - Some(defaults) => match &defaults.value { - Expr::List(ast::ExprList { elts, .. }) => Ok(elts), - Expr::Tuple(ast::ExprTuple { elts, .. }) => Ok(elts), - _ => bail!("Expected defaults to be `Expr::List` | `Expr::Tuple`"), - }, - None => Ok(&[]), - } -} - /// Create a list of property assignments from the `NamedTuple` arguments. -fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result> { +fn create_properties_from_args(args: &[Expr]) -> Result> { let Some(fields) = args.get(1) else { let node = Stmt::Pass(ast::StmtPass { range: TextRange::default(), @@ -151,16 +128,8 @@ fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result= defaults.len() { - std::iter::repeat(None) - .take(elts.len() - defaults.len()) - .chain(defaults.iter().map(Some)) - } else { - bail!("Defaults must be `None` or an iterable of at least the number of fields") - }; elts.iter() - .zip(padded_defaults) - .map(|(field, default)| { + .map(|field| { let Expr::Tuple(ast::ExprTuple { elts, .. }) = &field else { bail!("Expected `field` to be `Expr::Tuple`") }; @@ -180,9 +149,7 @@ fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result 1 && !keywords.is_empty() { + return; + } + + let properties = match create_properties_from_args(args) { Ok(properties) => properties, Err(err) => { debug!("Skipping `NamedTuple` \"{typename}\": {err}"); 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 0ee3cb2e098a9..2f0e8b4a0e3d6 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 @@ -23,37 +23,6 @@ UP014.py:5:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class s 7 9 | # with default values as list 8 10 | MyType = NamedTuple( -UP014.py:8:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax - | - 7 | # with default values as list - 8 | / MyType = NamedTuple( - 9 | | "MyType", -10 | | [("a", int), ("b", str), ("c", list[bool])], -11 | | defaults=["foo", [True]], -12 | | ) - | |_^ UP014 -13 | -14 | # with namespace - | - = help: Convert `MyType` to class syntax - -ℹ Suggested fix -5 5 | MyType = NamedTuple("MyType", [("a", int), ("b", tuple[str, ...])]) -6 6 | -7 7 | # with default values as list -8 |-MyType = NamedTuple( -9 |- "MyType", -10 |- [("a", int), ("b", str), ("c", list[bool])], -11 |- defaults=["foo", [True]], -12 |-) - 8 |+class MyType(NamedTuple): - 9 |+ a: int - 10 |+ b: str = "foo" - 11 |+ c: list[bool] = [True] -13 12 | -14 13 | # with namespace -15 14 | MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) - UP014.py:15:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax | 14 | # with namespace From 37b969603289b4c847fa2e7c554d31e25a603095 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 21:26:21 +0900 Subject: [PATCH 2/7] Keywords example --- crates/ruff/resources/test/fixtures/pyupgrade/UP014.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py index 1df6e8d035b6a..aaae7b9d384e1 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py @@ -8,7 +8,6 @@ MyType = NamedTuple( "MyType", [("a", int), ("b", str), ("c", list[bool])], - defaults=["foo", [True]], ) # with namespace @@ -18,7 +17,6 @@ MyType = NamedTuple( "MyType", [("a", int), ("b", str)], - defaults=[1, "bar", "baz"], ) # invalid identifiers (OK) @@ -29,3 +27,6 @@ # empty fields MyType = typing.NamedTuple("MyType", []) + +# keywords +MyType = typing.NamedTuple("MyType", a=int, b=tuple[str, ...]) From 25294a5da25c0d321b2614c39fd638f931dd1aca Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 23:19:27 +0900 Subject: [PATCH 3/7] Fix test cases --- .../resources/test/fixtures/pyupgrade/UP014.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py index aaae7b9d384e1..144e6fef716b3 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py @@ -4,21 +4,9 @@ # with complex annotations MyType = NamedTuple("MyType", [("a", int), ("b", tuple[str, ...])]) -# with default values as list -MyType = NamedTuple( - "MyType", - [("a", int), ("b", str), ("c", list[bool])], -) - # with namespace MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) -# too many default values (OK) -MyType = NamedTuple( - "MyType", - [("a", int), ("b", str)], -) - # invalid identifiers (OK) MyType = NamedTuple("MyType", [("x-y", int), ("b", tuple[str, ...])]) @@ -30,3 +18,7 @@ # keywords MyType = typing.NamedTuple("MyType", a=int, b=tuple[str, ...]) + +# unfixable +MyType = typing.NamedTuple("MyType", [("a", int)], [("b", str)]) +MyType = typing.NamedTuple("MyType", [("a", int)], b=str) From b136b4616bb70b39dd2cdf055282d5bcc6435e3b Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 23:19:36 +0900 Subject: [PATCH 4/7] Support keywords --- ...convert_named_tuple_functional_to_class.rs | 57 ++++++++++++------- 1 file changed, 35 insertions(+), 22 deletions(-) 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 101350bbdc928..ded4d3b77335a 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 @@ -111,14 +111,8 @@ fn create_property_assignment_stmt(property: &str, annotation: &Expr) -> Stmt { .into() } -/// Create a list of property assignments from the `NamedTuple` arguments. -fn create_properties_from_args(args: &[Expr]) -> Result> { - let Some(fields) = args.get(1) else { - let node = Stmt::Pass(ast::StmtPass { - range: TextRange::default(), - }); - return Ok(vec![node]); - }; +/// Create a list of property assignments from the `NamedTuple` positional arguments. +fn create_properties_from_fields_arg(fields: &Expr) -> Result> { let Expr::List(ast::ExprList { elts, .. }) = &fields else { bail!("Expected argument to be `Expr::List`"); }; @@ -154,6 +148,20 @@ fn create_properties_from_args(args: &[Expr]) -> Result> { .collect() } +/// Create a list of property assignments from the `NamedTuple` keyword arguments. +fn create_properties_from_keywords(keywords: &[Keyword]) -> Result> { + keywords + .iter() + .map(|keyword| { + let Keyword { arg, value, .. } = keyword; + let Some(arg) = arg else { + bail!("Expected `keyword` to have an `arg`") + }; + Ok(create_property_assignment_stmt(arg.as_str(), value)) + }) + .collect() +} + /// Generate a `StmtKind:ClassDef` statement based on the provided body and /// keywords. fn create_class_def_stmt(typename: &str, body: Vec, base_class: &Expr) -> Stmt { @@ -195,26 +203,31 @@ pub(crate) fn convert_named_tuple_functional_to_class( return; }; - // Ignore `NamedTuple` calls containing both a list of fields and keywords: - // ``` - // NamedTuple( - // "MyType", - // [("a", int), ("b", str)], - // defaults=[0, ""], - // ) - // ``` - if args.len() > 1 && !keywords.is_empty() { - return; - } - - let properties = match create_properties_from_args(args) { + let properties = match (&args[1..], keywords) { + // Ex) NamedTuple("MyType") + ([], []) => { + let node = Stmt::Pass(ast::StmtPass { + range: TextRange::default(), + }); + Ok(vec![node]) + } + // Ex) NamedTuple("MyType", [("a", int), ("b", str)]) + ([fields], []) => create_properties_from_fields_arg(fields), + // Ex) NamedTuple("MyType", a=int, b=str) + ([], keywords) => create_properties_from_keywords(keywords), + // Unfixable + _ => { + debug!("Skipping `NamedTuple` \"{typename}\": unfixable"); + return; + } + }; + let properties = match properties { Ok(properties) => properties, Err(err) => { debug!("Skipping `NamedTuple` \"{typename}\": {err}"); return; } }; - let mut diagnostic = Diagnostic::new( ConvertNamedTupleFunctionalToClass { name: typename.to_string(), From 451d7a645b33e0fe68b22c9508cbf11971dcdccf Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 23:21:30 +0900 Subject: [PATCH 5/7] Fix comment --- .../pyupgrade/rules/convert_named_tuple_functional_to_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ded4d3b77335a..33ae4c5095995 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 @@ -111,7 +111,7 @@ fn create_property_assignment_stmt(property: &str, annotation: &Expr) -> Stmt { .into() } -/// Create a list of property assignments from the `NamedTuple` positional arguments. +/// Create a list of property assignments from the `NamedTuple` fields argument. fn create_properties_from_fields_arg(fields: &Expr) -> Result> { let Expr::List(ast::ExprList { elts, .. }) = &fields else { bail!("Expected argument to be `Expr::List`"); From a036ec7876528fafece4789c712fe33369483908 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 16 Jul 2023 23:55:56 +0900 Subject: [PATCH 6/7] Update snapshot --- ...ff__rules__pyupgrade__tests__UP014.py.snap | 109 +++++++++++------- 1 file changed, 68 insertions(+), 41 deletions(-) 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 2f0e8b4a0e3d6..ee8d7ca89fd71 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 @@ -7,7 +7,7 @@ UP014.py:5:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class s 5 | MyType = NamedTuple("MyType", [("a", int), ("b", tuple[str, ...])]) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP014 6 | -7 | # with default values as list +7 | # with namespace | = help: Convert `MyType` to class syntax @@ -20,66 +20,93 @@ UP014.py:5:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class s 6 |+ a: int 7 |+ b: tuple[str, ...] 6 8 | -7 9 | # with default values as list -8 10 | MyType = NamedTuple( +7 9 | # with namespace +8 10 | MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) -UP014.py:15:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax +UP014.py:8:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax | -14 | # with namespace -15 | MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) + 7 | # with namespace + 8 | MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP014 -16 | -17 | # too many default values (OK) + 9 | +10 | # invalid identifiers (OK) | = help: Convert `MyType` to class syntax ℹ Suggested fix -12 12 | ) -13 13 | -14 14 | # with namespace -15 |-MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) - 15 |+class MyType(typing.NamedTuple): - 16 |+ a: int - 17 |+ b: str -16 18 | -17 19 | # too many default values (OK) -18 20 | MyType = NamedTuple( +5 5 | MyType = NamedTuple("MyType", [("a", int), ("b", tuple[str, ...])]) +6 6 | +7 7 | # with namespace +8 |-MyType = typing.NamedTuple("MyType", [("a", int), ("b", str)]) + 8 |+class MyType(typing.NamedTuple): + 9 |+ a: int + 10 |+ b: str +9 11 | +10 12 | # invalid identifiers (OK) +11 13 | MyType = NamedTuple("MyType", [("x-y", int), ("b", tuple[str, ...])]) -UP014.py:28:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax +UP014.py:14:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax | -27 | # no fields -28 | MyType = typing.NamedTuple("MyType") +13 | # no fields +14 | MyType = typing.NamedTuple("MyType") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP014 -29 | -30 | # empty fields +15 | +16 | # empty fields | = help: Convert `MyType` to class syntax ℹ Suggested fix -25 25 | MyType = NamedTuple("MyType", [("x-y", int), ("b", tuple[str, ...])]) -26 26 | -27 27 | # no fields -28 |-MyType = typing.NamedTuple("MyType") - 28 |+class MyType(typing.NamedTuple): - 29 |+ pass -29 30 | -30 31 | # empty fields -31 32 | MyType = typing.NamedTuple("MyType", []) +11 11 | MyType = NamedTuple("MyType", [("x-y", int), ("b", tuple[str, ...])]) +12 12 | +13 13 | # no fields +14 |-MyType = typing.NamedTuple("MyType") + 14 |+class MyType(typing.NamedTuple): + 15 |+ pass +15 16 | +16 17 | # empty fields +17 18 | MyType = typing.NamedTuple("MyType", []) -UP014.py:31:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax +UP014.py:17:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax | -30 | # empty fields -31 | MyType = typing.NamedTuple("MyType", []) +16 | # empty fields +17 | MyType = typing.NamedTuple("MyType", []) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP014 +18 | +19 | # keywords | = help: Convert `MyType` to class syntax ℹ Suggested fix -28 28 | MyType = typing.NamedTuple("MyType") -29 29 | -30 30 | # empty fields -31 |-MyType = typing.NamedTuple("MyType", []) - 31 |+class MyType(typing.NamedTuple): - 32 |+ pass +14 14 | MyType = typing.NamedTuple("MyType") +15 15 | +16 16 | # empty fields +17 |-MyType = typing.NamedTuple("MyType", []) + 17 |+class MyType(typing.NamedTuple): + 18 |+ pass +18 19 | +19 20 | # keywords +20 21 | MyType = typing.NamedTuple("MyType", a=int, b=tuple[str, ...]) + +UP014.py:20:1: UP014 [*] Convert `MyType` from `NamedTuple` functional to class syntax + | +19 | # keywords +20 | MyType = typing.NamedTuple("MyType", a=int, b=tuple[str, ...]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP014 +21 | +22 | # unfixable + | + = help: Convert `MyType` to class syntax + +ℹ Suggested fix +17 17 | MyType = typing.NamedTuple("MyType", []) +18 18 | +19 19 | # keywords +20 |-MyType = typing.NamedTuple("MyType", a=int, b=tuple[str, ...]) + 20 |+class MyType(typing.NamedTuple): + 21 |+ a: int + 22 |+ b: tuple[str, ...] +21 23 | +22 24 | # unfixable +23 25 | MyType = typing.NamedTuple("MyType", [("a", int)], [("b", str)]) From add4f62f3e547d3786cef2047a258cce2fd877cb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 16 Jul 2023 21:22:06 -0400 Subject: [PATCH 7/7] Truncate --- ...convert_named_tuple_functional_to_class.rs | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) 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 33ae4c5095995..2d59a3df801c4 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 @@ -155,8 +155,8 @@ fn create_properties_from_keywords(keywords: &[Keyword]) -> Result> { .map(|keyword| { let Keyword { arg, value, .. } = keyword; let Some(arg) = arg else { - bail!("Expected `keyword` to have an `arg`") - }; + bail!("Expected `keyword` to have an `arg`") + }; Ok(create_property_assignment_stmt(arg.as_str(), value)) }) .collect() @@ -205,29 +205,34 @@ pub(crate) fn convert_named_tuple_functional_to_class( let properties = match (&args[1..], keywords) { // Ex) NamedTuple("MyType") - ([], []) => { - let node = Stmt::Pass(ast::StmtPass { - range: TextRange::default(), - }); - Ok(vec![node]) - } + ([], []) => vec![Stmt::Pass(ast::StmtPass { + range: TextRange::default(), + })], // Ex) NamedTuple("MyType", [("a", int), ("b", str)]) - ([fields], []) => create_properties_from_fields_arg(fields), + ([fields], []) => { + if let Ok(properties) = create_properties_from_fields_arg(fields) { + properties + } else { + debug!("Skipping `NamedTuple` \"{typename}\": unable to parse fields"); + return; + } + } // Ex) NamedTuple("MyType", a=int, b=str) - ([], keywords) => create_properties_from_keywords(keywords), + ([], keywords) => { + if let Ok(properties) = create_properties_from_keywords(keywords) { + properties + } else { + debug!("Skipping `NamedTuple` \"{typename}\": unable to parse keywords"); + return; + } + } // Unfixable _ => { - debug!("Skipping `NamedTuple` \"{typename}\": unfixable"); - return; - } - }; - let properties = match properties { - Ok(properties) => properties, - Err(err) => { - debug!("Skipping `NamedTuple` \"{typename}\": {err}"); + debug!("Skipping `NamedTuple` \"{typename}\": mixed fields and keywords"); return; } }; + let mut diagnostic = Diagnostic::new( ConvertNamedTupleFunctionalToClass { name: typename.to_string(),