From 87aa5d6b664233af8fb5600ba28bbb035dc23e9e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 28 Aug 2023 17:03:18 -0400 Subject: [PATCH] Avoid panic when `typename` is provided as a keyword argument (#6955) ## Summary The `typename` argument to `NamedTuple` and `TypedDict` is a required positional argument. We assumed as much, but panicked if it was provided as a keyword argument or otherwise omitted. This PR handles the case gracefully. Closes https://github.com/astral-sh/ruff/issues/6953. --- .../test/fixtures/pyupgrade/UP014.py | 1 + ...convert_named_tuple_functional_to_class.rs | 11 ++-- .../convert_typed_dict_functional_to_class.rs | 51 ++++++++++--------- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py index 144e6fef71..83f4d0727b 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP014.py @@ -22,3 +22,4 @@ 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) +MyType = typing.NamedTuple(typename="MyType", a=int, b=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 08619c6de2..9b63bad3f2 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 @@ -72,8 +72,7 @@ fn match_named_tuple_assign<'a>( value: &'a Expr, semantic: &SemanticModel, ) -> Option<(&'a str, &'a [Expr], &'a [Keyword], &'a Expr)> { - let target = targets.get(0)?; - let Expr::Name(ast::ExprName { id: typename, .. }) = target else { + let [Expr::Name(ast::ExprName { id: typename, .. })] = targets else { return None; }; let Expr::Call(ast::ExprCall { @@ -209,13 +208,13 @@ pub(crate) fn convert_named_tuple_functional_to_class( return; }; - let properties = match (&args[1..], keywords) { + let properties = match (args, keywords) { // Ex) NamedTuple("MyType") - ([], []) => vec![Stmt::Pass(ast::StmtPass { + ([_typename], []) => vec![Stmt::Pass(ast::StmtPass { range: TextRange::default(), })], // Ex) NamedTuple("MyType", [("a", int), ("b", str)]) - ([fields], []) => { + ([_typename, fields], []) => { if let Ok(properties) = create_properties_from_fields_arg(fields) { properties } else { @@ -224,7 +223,7 @@ pub(crate) fn convert_named_tuple_functional_to_class( } } // Ex) NamedTuple("MyType", a=int, b=str) - ([], keywords) => { + ([_typename], keywords) => { if let Ok(properties) = create_properties_from_keywords(keywords) { properties } else { 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 b0b4602fb3..5012d36c89 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 @@ -71,8 +71,7 @@ fn match_typed_dict_assign<'a>( value: &'a Expr, semantic: &SemanticModel, ) -> Option<(&'a str, &'a Arguments, &'a Expr)> { - let target = targets.get(0)?; - let Expr::Name(ast::ExprName { id: class_name, .. }) = target else { + let [Expr::Name(ast::ExprName { id: class_name, .. })] = targets else { return None; }; let Expr::Call(ast::ExprCall { @@ -210,28 +209,34 @@ fn match_properties_and_total(arguments: &Arguments) -> Result<(Vec, Optio // ``` // MyType = TypedDict('MyType', {'a': int, 'b': str}, a=int, b=str) // ``` - if let Some(dict) = arguments.args.get(1) { - let total = arguments.find_keyword("total"); - match dict { - Expr::Dict(ast::ExprDict { - keys, - values, - range: _, - }) => Ok((properties_from_dict_literal(keys, values)?, total)), - Expr::Call(ast::ExprCall { - func, - arguments: Arguments { keywords, .. }, - .. - }) => Ok((properties_from_dict_call(func, keywords)?, total)), - _ => bail!("Expected `arg` to be `Expr::Dict` or `Expr::Call`"), + match (arguments.args.as_slice(), arguments.keywords.as_slice()) { + // Ex) `TypedDict("MyType", {"a": int, "b": str})` + ([_typename, fields], [..]) => { + let total = arguments.find_keyword("total"); + match fields { + Expr::Dict(ast::ExprDict { + keys, + values, + range: _, + }) => Ok((properties_from_dict_literal(keys, values)?, total)), + Expr::Call(ast::ExprCall { + func, + arguments: Arguments { keywords, .. }, + range: _, + }) => Ok((properties_from_dict_call(func, keywords)?, total)), + _ => bail!("Expected `arg` to be `Expr::Dict` or `Expr::Call`"), + } } - } else if !arguments.keywords.is_empty() { - Ok((properties_from_keywords(&arguments.keywords)?, None)) - } else { - let node = Stmt::Pass(ast::StmtPass { - range: TextRange::default(), - }); - Ok((vec![node], None)) + // Ex) `TypedDict("MyType")` + ([_typename], []) => { + let node = Stmt::Pass(ast::StmtPass { + range: TextRange::default(), + }); + Ok((vec![node], None)) + } + // Ex) `TypedDict("MyType", a=int, b=str)` + ([_typename], fields) => Ok((properties_from_keywords(fields)?, None)), + _ => bail!("Expected `args` to have exactly one or two elements"), } }