Update RustPython to fix Dict.keys type (#2086)

This PR upgrades RustPython to fix the type of `Dict.keys` to `Vec<Option<Expr>>` (see RustPython/RustPython#4449 for why this change was needed) and unblock #1884.
astral-sh · Jan 22, 2023 · a7ce862 · a7ce862
1 parent 36fb8f7
commit a7ce862
Show file tree

Hide file tree

Showing 18 changed files with 156 additions and 97 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -295,7 +295,7 @@ pub enum ComparableExpr<'a> {
         orelse: Box<ComparableExpr<'a>>,
     },
     Dict {
-        keys: Vec<ComparableExpr<'a>>,
+        keys: Vec<Option<ComparableExpr<'a>>>,
         values: Vec<ComparableExpr<'a>>,
     },
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -26,16 +26,19 @@ fn into_dictionary_key(expr: &Expr) -> Option<DictionaryKey> {
 }
 
 /// F601, F602
-pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) {
+pub fn repeated_keys(checker: &mut Checker, keys: &[Option<Expr>], values: &[Expr]) {
     // Generate a map from key to (index, value).
     let mut seen: FxHashMap<DictionaryKey, FxHashSet<ComparableExpr>> =
         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
@@ -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
@@ -78,14 +78,18 @@ fn create_class_def_stmt(
     })
 }
 
-fn properties_from_dict_literal(keys: &[Expr], values: &[Expr]) -> Result<Vec<Stmt>> {
+fn properties_from_dict_literal(keys: &[Option<Expr>], values: &[Expr]) -> Result<Vec<Stmt>> {
     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
@@ -156,41 +156,47 @@ fn clean_params_dictionary(checker: &mut Checker, right: &Expr) -> Option<String
         let mut seen: Vec<&str> = 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
@@ -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
@@ -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: ~