From a7ce8621a9c43bae200bdea60aa6803ef497ede1 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Mon, 23 Jan 2023 03:24:00 +0900 Subject: [PATCH] Update RustPython to fix `Dict.keys` type (#2086) This PR upgrades RustPython to fix the type of `Dict.keys` to `Vec>` (see https://github.com/RustPython/RustPython/pull/4449 for why this change was needed) and unblock #1884. --- Cargo.lock | 8 +-- Cargo.toml | 6 +- resources/test/fixtures/pyupgrade/UP013.py | 4 ++ resources/test/fixtures/pyupgrade/UP031_0.py | 6 ++ ruff_dev/Cargo.toml | 6 +- src/ast/comparable.rs | 7 +- src/ast/helpers.rs | 2 +- src/ast/relocate.rs | 2 +- src/ast/visitor.rs | 2 +- src/checkers/ast.rs | 2 +- src/rules/pyflakes/rules/repeated_keys.rs | 23 +++--- src/rules/pyflakes/rules/strings.rs | 23 +++--- .../convert_typed_dict_functional_to_class.rs | 14 ++-- .../rules/printf_string_formatting.rs | 72 ++++++++++--------- .../rules/unpack_list_comprehension.rs | 6 +- ...les__pyupgrade__tests__UP013_UP013.py.snap | 10 +++ ...s__pyupgrade__tests__UP031_UP031_0.py.snap | 41 +++++++---- src/source_code/generator.rs | 19 +++-- 18 files changed, 156 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82a278d4c1d39..51e181e8cb452 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1994,7 +1994,7 @@ dependencies = [ [[package]] name = "rustpython-ast" version = "0.2.0" -source = "git+https://github.com/RustPython/RustPython.git?rev=62aa942bf506ea3d41ed0503b947b84141fdaa3c#62aa942bf506ea3d41ed0503b947b84141fdaa3c" +source = "git+https://github.com/RustPython/RustPython.git?rev=4f38cb68e4a97aeea9eb19673803a0bd5f655383#4f38cb68e4a97aeea9eb19673803a0bd5f655383" dependencies = [ "num-bigint", "rustpython-common", @@ -2004,7 +2004,7 @@ dependencies = [ [[package]] name = "rustpython-common" version = "0.2.0" -source = "git+https://github.com/RustPython/RustPython.git?rev=62aa942bf506ea3d41ed0503b947b84141fdaa3c#62aa942bf506ea3d41ed0503b947b84141fdaa3c" +source = "git+https://github.com/RustPython/RustPython.git?rev=4f38cb68e4a97aeea9eb19673803a0bd5f655383#4f38cb68e4a97aeea9eb19673803a0bd5f655383" dependencies = [ "ascii", "bitflags", @@ -2029,7 +2029,7 @@ dependencies = [ [[package]] name = "rustpython-compiler-core" version = "0.2.0" -source = "git+https://github.com/RustPython/RustPython.git?rev=62aa942bf506ea3d41ed0503b947b84141fdaa3c#62aa942bf506ea3d41ed0503b947b84141fdaa3c" +source = "git+https://github.com/RustPython/RustPython.git?rev=4f38cb68e4a97aeea9eb19673803a0bd5f655383#4f38cb68e4a97aeea9eb19673803a0bd5f655383" dependencies = [ "bincode", "bitflags", @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "rustpython-parser" version = "0.2.0" -source = "git+https://github.com/RustPython/RustPython.git?rev=62aa942bf506ea3d41ed0503b947b84141fdaa3c#62aa942bf506ea3d41ed0503b947b84141fdaa3c" +source = "git+https://github.com/RustPython/RustPython.git?rev=4f38cb68e4a97aeea9eb19673803a0bd5f655383#4f38cb68e4a97aeea9eb19673803a0bd5f655383" dependencies = [ "ahash", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 021cdec50d08b..e7925900102db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,9 +48,9 @@ path-absolutize = { version = "3.0.14", features = ["once_cell_cache", "use_unix regex = { version = "1.6.0" } ruff_macros = { version = "0.0.229", path = "ruff_macros" } rustc-hash = { version = "1.1.0" } -rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } -rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } -rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } +rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } +rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } +rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } schemars = { version = "0.8.11" } semver = { version = "1.0.16" } serde = { version = "1.0.147", features = ["derive"] } diff --git a/resources/test/fixtures/pyupgrade/UP013.py b/resources/test/fixtures/pyupgrade/UP013.py index 28c983017fdde..66f041423c172 100644 --- a/resources/test/fixtures/pyupgrade/UP013.py +++ b/resources/test/fixtures/pyupgrade/UP013.py @@ -31,3 +31,7 @@ # using namespace TypedDict MyType11 = typing.TypedDict("MyType11", {"key": int}) + +# unpacking +c = {"c": float} +MyType12 = TypedDict("MyType1", {"a": int, "b": str, **c}) diff --git a/resources/test/fixtures/pyupgrade/UP031_0.py b/resources/test/fixtures/pyupgrade/UP031_0.py index 7ca4dfc39b7cb..9a67469656d5a 100644 --- a/resources/test/fixtures/pyupgrade/UP031_0.py +++ b/resources/test/fixtures/pyupgrade/UP031_0.py @@ -62,6 +62,12 @@ "bar %(bar)s" % {"foo": x, "bar": y} ) +bar = {"bar": y} +print( + "foo %(foo)s " + "bar %(bar)s" % {"foo": x, **bar} +) + print("%s \N{snowman}" % (a,)) print("%(foo)s \N{snowman}" % {"foo": 1}) diff --git a/ruff_dev/Cargo.toml b/ruff_dev/Cargo.toml index ee35ac1573154..a1e807b7a1030 100644 --- a/ruff_dev/Cargo.toml +++ b/ruff_dev/Cargo.toml @@ -11,9 +11,9 @@ libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "f2f0b7a487a87 once_cell = { version = "1.16.0" } ruff = { path = ".." } ruff_cli = { path = "../ruff_cli" } -rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } -rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } -rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" } +rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } +rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } +rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "4f38cb68e4a97aeea9eb19673803a0bd5f655383" } schemars = { version = "0.8.11" } serde_json = {version="1.0.91"} strum = { version = "0.24.1", features = ["strum_macros"] } diff --git a/src/ast/comparable.rs b/src/ast/comparable.rs index 40f9dc492bb00..54d57e5f0fd0f 100644 --- a/src/ast/comparable.rs +++ b/src/ast/comparable.rs @@ -295,7 +295,7 @@ pub enum ComparableExpr<'a> { orelse: Box>, }, Dict { - keys: Vec>, + keys: Vec>>, values: Vec>, }, Set { @@ -424,7 +424,10 @@ impl<'a> From<&'a Expr> for ComparableExpr<'a> { orelse: orelse.into(), }, ExprKind::Dict { keys, values } => Self::Dict { - keys: keys.iter().map(std::convert::Into::into).collect(), + keys: keys + .iter() + .map(|expr| expr.as_ref().map(std::convert::Into::into)) + .collect(), values: values.iter().map(std::convert::Into::into).collect(), }, ExprKind::Set { elts } => Self::Set { diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index 0dd6208418e9f..71ce1296a6c50 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -161,7 +161,7 @@ where } ExprKind::Dict { keys, values } => values .iter() - .chain(keys.iter()) + .chain(keys.iter().flatten()) .any(|expr| any_over_expr(expr, func)), ExprKind::Set { elts } | ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } => { elts.iter().any(|expr| any_over_expr(expr, func)) diff --git a/src/ast/relocate.rs b/src/ast/relocate.rs index 2dd294fafdc1a..71e043225a188 100644 --- a/src/ast/relocate.rs +++ b/src/ast/relocate.rs @@ -39,7 +39,7 @@ pub fn relocate_expr(expr: &mut Expr, location: Range) { relocate_expr(orelse, location); } ExprKind::Dict { keys, values } => { - for expr in keys { + for expr in keys.iter_mut().flatten() { relocate_expr(expr, location); } for expr in values { diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index 6db3b3f3b60ca..bcaafa8045d0d 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -288,7 +288,7 @@ pub fn walk_expr<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) { visitor.visit_expr(orelse); } ExprKind::Dict { keys, values } => { - for expr in keys { + for expr in keys.iter().flatten() { visitor.visit_expr(expr); } for expr in values { diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 047fd5613644f..dcd40a2901a8a 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -3155,7 +3155,7 @@ where // Ex) TypedDict("a", {"a": int}) if args.len() > 1 { if let ExprKind::Dict { keys, values } = &args[1].node { - for key in keys { + for key in keys.iter().flatten() { self.in_type_definition = false; self.visit_expr(key); self.in_type_definition = prev_in_type_definition; diff --git a/src/rules/pyflakes/rules/repeated_keys.rs b/src/rules/pyflakes/rules/repeated_keys.rs index 1d8ccfa7c67f2..219ad72d91b7b 100644 --- a/src/rules/pyflakes/rules/repeated_keys.rs +++ b/src/rules/pyflakes/rules/repeated_keys.rs @@ -26,16 +26,19 @@ fn into_dictionary_key(expr: &Expr) -> Option { } /// F601, F602 -pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { +pub fn repeated_keys(checker: &mut Checker, keys: &[Option], values: &[Expr]) { // Generate a map from key to (index, value). let mut seen: FxHashMap> = FxHashMap::with_capacity_and_hasher(keys.len(), BuildHasherDefault::default()); // Detect duplicate keys. for (i, key) in keys.iter().enumerate() { - if let Some(key) = into_dictionary_key(key) { - if let Some(seen_values) = seen.get_mut(&key) { - match key { + let Some(key) = key else { + continue; + }; + if let Some(dict_key) = into_dictionary_key(key) { + if let Some(seen_values) = seen.get_mut(&dict_key) { + match dict_key { DictionaryKey::Constant(..) => { if checker .settings @@ -46,10 +49,10 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { let is_duplicate_value = seen_values.contains(&comparable_value); let mut diagnostic = Diagnostic::new( violations::MultiValueRepeatedKeyLiteral( - unparse_expr(&keys[i], checker.stylist), + unparse_expr(key, checker.stylist), is_duplicate_value, ), - Range::from_located(&keys[i]), + Range::from_located(key), ); if is_duplicate_value { if checker.patch(&Rule::MultiValueRepeatedKeyLiteral) { @@ -64,7 +67,7 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { checker.diagnostics.push(diagnostic); } } - DictionaryKey::Variable(key) => { + DictionaryKey::Variable(dict_key) => { if checker .settings .rules @@ -74,10 +77,10 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { let is_duplicate_value = seen_values.contains(&comparable_value); let mut diagnostic = Diagnostic::new( violations::MultiValueRepeatedKeyVariable( - key.to_string(), + dict_key.to_string(), is_duplicate_value, ), - Range::from_located(&keys[i]), + Range::from_located(key), ); if is_duplicate_value { if checker.patch(&Rule::MultiValueRepeatedKeyVariable) { @@ -94,7 +97,7 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { } } } else { - seen.insert(key, FxHashSet::from_iter([(&values[i]).into()])); + seen.insert(dict_key, FxHashSet::from_iter([(&values[i]).into()])); } } } diff --git a/src/rules/pyflakes/rules/strings.rs b/src/rules/pyflakes/rules/strings.rs index 0e0e6a0b0516e..49b4df4e27bb3 100644 --- a/src/rules/pyflakes/rules/strings.rs +++ b/src/rules/pyflakes/rules/strings.rs @@ -81,21 +81,24 @@ pub(crate) fn percent_format_extra_named_arguments( if summary.num_positional > 0 { return; } - let ExprKind::Dict { keys, values } = &right.node else { + let ExprKind::Dict { keys, .. } = &right.node else { return; }; - if values.len() > keys.len() { + if keys.iter().any(std::option::Option::is_none) { return; // contains **x splat } let missing: Vec<&str> = keys .iter() - .filter_map(|k| match &k.node { - // We can only check that string literals exist - ExprKind::Constant { - value: Constant::Str(value), + .filter_map(|k| match k { + Some(Expr { + node: + ExprKind::Constant { + value: Constant::Str(value), + .. + }, .. - } => { + }) => { if summary.keywords.contains(value) { None } else { @@ -138,13 +141,13 @@ pub(crate) fn percent_format_missing_arguments( return; } - if let ExprKind::Dict { keys, values } = &right.node { - if values.len() > keys.len() { + if let ExprKind::Dict { keys, .. } = &right.node { + if keys.iter().any(std::option::Option::is_none) { return; // contains **x splat } let mut keywords = FxHashSet::default(); - for key in keys { + for key in keys.iter().flatten() { match &key.node { ExprKind::Constant { value: Constant::Str(value), diff --git a/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs b/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs index 6ab70cea21eb5..85432cfb9d785 100644 --- a/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs +++ b/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs @@ -78,14 +78,18 @@ fn create_class_def_stmt( }) } -fn properties_from_dict_literal(keys: &[Expr], values: &[Expr]) -> Result> { +fn properties_from_dict_literal(keys: &[Option], values: &[Expr]) -> Result> { keys.iter() .zip(values.iter()) - .map(|(key, value)| match &key.node { - ExprKind::Constant { - value: Constant::Str(property), + .map(|(key, value)| match key { + Some(Expr { + node: + ExprKind::Constant { + value: Constant::Str(property), + .. + }, .. - } => { + }) => { if is_identifier(property) && !KWLIST.contains(&property.as_str()) { Ok(create_property_assignment_stmt(property, &value.node)) } else { diff --git a/src/rules/pyupgrade/rules/printf_string_formatting.rs b/src/rules/pyupgrade/rules/printf_string_formatting.rs index 8b21151b045ae..f9a91a839dabe 100644 --- a/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -156,41 +156,47 @@ fn clean_params_dictionary(checker: &mut Checker, right: &Expr) -> Option = vec![]; let mut indent = None; for (key, value) in keys.iter().zip(values.iter()) { - if let ExprKind::Constant { - value: Constant::Str(key_string), - .. - } = &key.node - { - // If the dictionary key is not a valid variable name, abort. - if !is_identifier(key_string) { - return None; - } - // If the key is a Python keyword, abort. - if KWLIST.contains(&key_string.as_str()) { - return None; - } - // If there are multiple entries of the same key, abort. - if seen.contains(&key_string.as_str()) { - return None; - } - seen.push(key_string); - let mut contents = String::new(); - if is_multi_line { - if indent.is_none() { - indent = indentation(checker.locator, key); + match key { + Some(key) => { + if let ExprKind::Constant { + value: Constant::Str(key_string), + .. + } = &key.node + { + // If the dictionary key is not a valid variable name, abort. + if !is_identifier(key_string) { + return None; + } + // If the key is a Python keyword, abort. + if KWLIST.contains(&key_string.as_str()) { + return None; + } + // If there are multiple entries of the same key, abort. + if seen.contains(&key_string.as_str()) { + return None; + } + seen.push(key_string); + if is_multi_line { + if indent.is_none() { + indent = indentation(checker.locator, key); + } + } + + let value_string = checker + .locator + .slice_source_code_range(&Range::from_located(value)); + arguments.push(format!("{key_string}={value_string}")); + } else { + // If there are any non-string keys, abort. + return None; } } - - let value_string = checker - .locator - .slice_source_code_range(&Range::from_located(value)); - contents.push_str(key_string); - contents.push('='); - contents.push_str(value_string); - arguments.push(contents); - } else { - // If there are any non-string keys, abort. - return None; + None => { + let value_string = checker + .locator + .slice_source_code_range(&Range::from_located(value)); + arguments.push(format!("**{value_string}")); + } } } // If we couldn't parse out key values, abort. diff --git a/src/rules/pyupgrade/rules/unpack_list_comprehension.rs b/src/rules/pyupgrade/rules/unpack_list_comprehension.rs index f6fa2fd765164..1f8582f385cd2 100644 --- a/src/rules/pyupgrade/rules/unpack_list_comprehension.rs +++ b/src/rules/pyupgrade/rules/unpack_list_comprehension.rs @@ -18,7 +18,11 @@ fn contains_await(expr: &Expr) -> bool { ExprKind::IfExp { test, body, orelse } => { contains_await(test) || contains_await(body) || contains_await(orelse) } - ExprKind::Dict { keys, values } => keys.iter().chain(values.iter()).any(contains_await), + ExprKind::Dict { keys, values } => keys + .iter() + .flatten() + .chain(values.iter()) + .any(contains_await), ExprKind::Set { elts } => elts.iter().any(contains_await), ExprKind::ListComp { elt, .. } => contains_await(elt), ExprKind::SetComp { elt, .. } => contains_await(elt), diff --git a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013_UP013.py.snap b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013_UP013.py.snap index a44e192dc8e70..2e63422bdceee 100644 --- a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013_UP013.py.snap +++ b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP013_UP013.py.snap @@ -182,4 +182,14 @@ expression: diagnostics row: 33 column: 53 parent: ~ +- kind: + ConvertTypedDictFunctionalToClass: MyType12 + location: + row: 37 + column: 0 + end_location: + row: 37 + column: 58 + fix: ~ + parent: ~ diff --git a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_UP031_0.py.snap b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_UP031_0.py.snap index 17e6e2b60e887..1cfee20ed9a71 100644 --- a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_UP031_0.py.snap +++ b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_UP031_0.py.snap @@ -430,52 +430,69 @@ expression: diagnostics - kind: PrintfStringFormatting: ~ location: - row: 65 + row: 67 + column: 4 + end_location: + row: 68 + column: 37 + fix: + content: "\"foo {foo} \"\n \"bar {bar}\".format(foo=x, **bar)" + location: + row: 67 + column: 4 + end_location: + row: 68 + column: 37 + parent: ~ +- kind: + PrintfStringFormatting: ~ + location: + row: 71 column: 6 end_location: - row: 65 + row: 71 column: 29 fix: content: "\"{} \\N{snowman}\".format(a)" location: - row: 65 + row: 71 column: 6 end_location: - row: 65 + row: 71 column: 29 parent: ~ - kind: PrintfStringFormatting: ~ location: - row: 67 + row: 73 column: 6 end_location: - row: 67 + row: 73 column: 40 fix: content: "\"{foo} \\N{snowman}\".format(foo=1)" location: - row: 67 + row: 73 column: 6 end_location: - row: 67 + row: 73 column: 40 parent: ~ - kind: PrintfStringFormatting: ~ location: - row: 69 + row: 75 column: 6 end_location: - row: 69 + row: 75 column: 35 fix: content: "(\"foo {} \" \"bar {}\").format(x, y)" location: - row: 69 + row: 75 column: 6 end_location: - row: 69 + row: 75 column: 35 parent: ~ diff --git a/src/source_code/generator.rs b/src/source_code/generator.rs index 6779f56daf6cc..e93e4506d197d 100644 --- a/src/source_code/generator.rs +++ b/src/source_code/generator.rs @@ -678,17 +678,16 @@ impl<'a> Generator<'a> { ExprKind::Dict { keys, values } => { self.p("{"); let mut first = true; - let (packed, unpacked) = values.split_at(keys.len()); - for (k, v) in keys.iter().zip(packed) { + for (k, v) in keys.iter().zip(values) { self.p_delim(&mut first, ", "); - self.unparse_expr(k, precedence::TEST); - self.p(": "); - self.unparse_expr(v, precedence::TEST); - } - for d in unpacked { - self.p_delim(&mut first, ", "); - self.p("**"); - self.unparse_expr(d, precedence::EXPR); + if let Some(k) = k { + self.unparse_expr(k, precedence::TEST); + self.p(": "); + self.unparse_expr(v, precedence::TEST); + } else { + self.p("**"); + self.unparse_expr(v, precedence::EXPR); + } } self.p("}"); }