Upgrade RustPython to access ranged names (#5194)

## Summary In astral-sh/RustPython-Parser#8, we modified RustPython to include ranges for any identifiers that aren't `Expr::Name` (which already has an identifier). For example, the `e` in `except ValueError as e` was previously un-ranged. To extract its range, we had to do some lexing of our own. This change should improve performance and let us remove a bunch of code. ## Test Plan `cargo test`
astral-sh · Jun 20, 2023 · 6331598 · 6331598
1 parent 17f1ecd
commit 6331598
Show file tree

Hide file tree

Showing 24 changed files with 86 additions and 57 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -24,7 +24,6 @@ ignore = { version = "0.4.20" }
 insta = { version = "1.28.0" }
 is-macro = { version = "0.2.2" }
 itertools = { version = "0.10.5" }
-libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "80e4c1399f95e5beb532fdd1e209ad2dbb470438" }
 log = { version = "0.4.17" }
 memchr = "2.5.0"
 nohash-hasher = { version = "0.2.0" }
@@ -36,11 +35,6 @@ proc-macro2 = { version = "1.0.51" }
 quote = { version = "1.0.23" }
 regex = { version = "1.7.1" }
 rustc-hash = { version = "1.1.0" }
-ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8d74eee75031b68d2204219963fae54a3f31a394" }
-rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8d74eee75031b68d2204219963fae54a3f31a394" , default-features = false, features = ["all-nodes-with-ranges", "num-bigint"]}
-rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8d74eee75031b68d2204219963fae54a3f31a394", default-features = false, features = ["num-bigint"] }
-rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8d74eee75031b68d2204219963fae54a3f31a394" }
-rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8d74eee75031b68d2204219963fae54a3f31a394" , default-features = false, features = ["full-lexer", "all-nodes-with-ranges", "num-bigint"] }
 schemars = { version = "0.8.12" }
 serde = { version = "1.0.152", features = ["derive"] }
 serde_json = { version = "1.0.93" }
@@ -53,6 +47,19 @@ syn = { version = "2.0.15" }
 test-case = { version = "3.0.0" }
 toml = { version = "0.7.2" }
 
+# v0.0.1
+libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "80e4c1399f95e5beb532fdd1e209ad2dbb470438" }
+# v0.0.3
+ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "ed3b4eb72b6e497bbdb4d19dec6621074d724130" }
+# v0.0.3
+rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "ed3b4eb72b6e497bbdb4d19dec6621074d724130" , default-features = false, features = ["all-nodes-with-ranges", "num-bigint"]}
+# v0.0.3
+rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "ed3b4eb72b6e497bbdb4d19dec6621074d724130", default-features = false, features = ["num-bigint"] }
+# v0.0.3
+rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "ed3b4eb72b6e497bbdb4d19dec6621074d724130", default-features = false }
+# v0.0.3
+rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "ed3b4eb72b6e497bbdb4d19dec6621074d724130" , default-features = false, features = ["full-lexer", "all-nodes-with-ranges", "num-bigint"] }
+
 [profile.release]
 lto = "fat"
 codegen-units = 1

diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs
@@ -825,7 +825,7 @@ where
                         if alias
                             .asname
                             .as_ref()
-                            .map_or(false, |asname| asname == &alias.name)
+                            .map_or(false, |asname| asname.as_str() == alias.name.as_str())
                         {
                             flags |= BindingFlags::EXPLICIT_EXPORT;
                         }
@@ -1110,7 +1110,7 @@ where
                         if alias
                             .asname
                             .as_ref()
-                            .map_or(false, |asname| asname == &alias.name)
+                            .map_or(false, |asname| asname.as_str() == alias.name.as_str())
                         {
                             flags |= BindingFlags::EXPLICIT_EXPORT;
                         }

diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
@@ -1,5 +1,5 @@
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Ranged};
+use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Identifier, Ranged};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
@@ -27,7 +27,7 @@ impl AlwaysAutofixableViolation for GetAttrWithConstant {
 fn attribute(value: &Expr, attr: &str) -> Expr {
     ast::ExprAttribute {
         value: Box::new(value.clone()),
-        attr: attr.into(),
+        attr: Identifier::new(attr.to_string(), TextRange::default()),
         ctx: ExprContext::Load,
         range: TextRange::default(),
     }

diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs
@@ -1,5 +1,5 @@
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Ranged, Stmt};
+use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Identifier, Ranged, Stmt};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
@@ -30,7 +30,7 @@ fn assignment(obj: &Expr, name: &str, value: &Expr, generator: Generator) -> Str
     let stmt = Stmt::Assign(ast::StmtAssign {
         targets: vec![Expr::Attribute(ast::ExprAttribute {
             value: Box::new(obj.clone()),
-            attr: name.into(),
+            attr: Identifier::new(name.to_string(), TextRange::default()),
             ctx: ExprContext::Store,
             range: TextRange::default(),
         })],

diff --git a/crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs b/crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs
@@ -3,7 +3,7 @@ use rustpython_parser::ast::{self, Expr};
 /// Returns true if the [`Expr`] is an internationalization function call.
 pub(crate) fn is_gettext_func_call(func: &Expr, functions_names: &[String]) -> bool {
     if let Expr::Name(ast::ExprName { id, .. }) = func {
-        functions_names.contains(id.as_ref())
+        functions_names.contains(id)
     } else {
         false
     }

diff --git a/crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs b/crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs
@@ -5,7 +5,7 @@ use itertools::Either::{Left, Right};
 
 use ruff_text_size::TextRange;
 
-use rustpython_parser::ast::{self, BoolOp, Expr, ExprContext, Ranged};
+use rustpython_parser::ast::{self, BoolOp, Expr, ExprContext, Identifier, Ranged};
 
 use ruff_diagnostics::AlwaysAutofixableViolation;
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
@@ -140,7 +140,7 @@ pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) {
                 });
                 let node2 = Expr::Attribute(ast::ExprAttribute {
                     value: Box::new(node1),
-                    attr: attr_name.into(),
+                    attr: Identifier::new(attr_name.to_string(), TextRange::default()),
                     ctx: ExprContext::Load,
                     range: TextRange::default(),
                 });

diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs
@@ -1,4 +1,4 @@
-use rustpython_parser::ast::{self, Expr, Identifier, Keyword, Ranged, Stmt, WithItem};
+use rustpython_parser::ast::{self, Expr, Keyword, Ranged, Stmt, WithItem};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -74,9 +74,12 @@ pub(crate) fn raises_call(checker: &mut Checker, func: &Expr, args: &[Expr], key
         }
 
         if checker.enabled(Rule::PytestRaisesTooBroad) {
-            let match_keyword = keywords
-                .iter()
-                .find(|kw| kw.arg == Some(Identifier::new("match")));
+            let match_keyword = keywords.iter().find(|keyword| {
+                keyword
+                    .arg
+                    .as_ref()
+                    .map_or(false, |arg| arg.as_str() == "match")
+            });
 
             if let Some(exception) = args.first() {
                 if let Some(match_keyword) = match_keyword {

diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/unittest_assert.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/unittest_assert.rs
@@ -3,7 +3,9 @@ use std::hash::BuildHasherDefault;
 use anyhow::{anyhow, bail, Result};
 use ruff_text_size::TextRange;
 use rustc_hash::FxHashMap;
-use rustpython_parser::ast::{self, CmpOp, Constant, Expr, ExprContext, Keyword, Stmt, UnaryOp};
+use rustpython_parser::ast::{
+    self, CmpOp, Constant, Expr, ExprContext, Identifier, Keyword, Stmt, UnaryOp,
+};
 
 /// An enum to represent the different types of assertions present in the
 /// `unittest` module. Note: any variants that can't be replaced with plain
@@ -388,7 +390,7 @@ impl UnittestAssert {
                 };
                 let node1 = ast::ExprAttribute {
                     value: Box::new(node.into()),
-                    attr: "search".into(),
+                    attr: Identifier::new("search".to_string(), TextRange::default()),
                     ctx: ExprContext::Load,
                     range: TextRange::default(),
                 };

diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs
@@ -1,7 +1,7 @@
 use log::error;
 use ruff_text_size::TextRange;
 use rustc_hash::FxHashSet;
-use rustpython_parser::ast::{self, CmpOp, Constant, Expr, ExprContext, Ranged, Stmt};
+use rustpython_parser::ast::{self, CmpOp, Constant, Expr, ExprContext, Identifier, Ranged, Stmt};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -941,7 +941,7 @@ pub(crate) fn use_dict_get_with_default(
     let node1 = *test_key.clone();
     let node2 = ast::ExprAttribute {
         value: expected_subscript.clone(),
-        attr: "get".into(),
+        attr: Identifier::new("get".to_string(), TextRange::default()),
         ctx: ExprContext::Load,
         range: TextRange::default(),
     };

diff --git a/crates/ruff/src/rules/flake8_tidy_imports/rules/relative_imports.rs b/crates/ruff/src/rules/flake8_tidy_imports/rules/relative_imports.rs
@@ -1,5 +1,5 @@
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Int, Ranged, Stmt};
+use rustpython_parser::ast::{self, Identifier, Int, Ranged, Stmt};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -94,7 +94,10 @@ fn fix_banned_relative_import(
         panic!("Expected Stmt::ImportFrom");
     };
     let node = ast::StmtImportFrom {
-        module: Some(module_path.to_string().into()),
+        module: Some(Identifier::new(
+            module_path.to_string(),
+            TextRange::default(),
+        )),
         names: names.clone(),
         level: Some(Int::new(0)),
         range: TextRange::default(),

diff --git a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs
@@ -1,5 +1,7 @@
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Arg, ArgWithDefault, Arguments, Constant, Expr, Ranged, Stmt};
+use rustpython_parser::ast::{
+    self, Arg, ArgWithDefault, Arguments, Constant, Expr, Identifier, Ranged, Stmt,
+};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -201,7 +203,7 @@ fn function(
                 })
                 .collect::<Vec<_>>();
             let func = Stmt::FunctionDef(ast::StmtFunctionDef {
-                name: name.into(),
+                name: Identifier::new(name.to_string(), TextRange::default()),
                 args: Box::new(Arguments {
                     posonlyargs: new_posonlyargs,
                     args: new_args,
@@ -217,7 +219,7 @@ fn function(
         }
     }
     let func = Stmt::FunctionDef(ast::StmtFunctionDef {
-        name: name.into(),
+        name: Identifier::new(name.to_string(), TextRange::default()),
         args: Box::new(args.clone()),
         body: vec![body],
         decorator_list: vec![],

diff --git a/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs b/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs
@@ -1,7 +1,7 @@
 use std::hash::BuildHasherDefault;
 
 use rustc_hash::FxHashSet;
-use rustpython_parser::ast::{self, Expr, Identifier, Ranged};
+use rustpython_parser::ast::{self, Expr, Ranged};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -52,7 +52,7 @@ impl Violation for DuplicateBases {
 
 /// PLE0241
 pub(crate) fn duplicate_bases(checker: &mut Checker, name: &str, bases: &[Expr]) {
-    let mut seen: FxHashSet<&Identifier> =
+    let mut seen: FxHashSet<&str> =
         FxHashSet::with_capacity_and_hasher(bases.len(), BuildHasherDefault::default());
     for base in bases {
         if let Expr::Name(ast::ExprName { id, .. }) = base {

diff --git a/crates/ruff/src/rules/pylint/rules/manual_import_from.rs b/crates/ruff/src/rules/pylint/rules/manual_import_from.rs
@@ -1,5 +1,5 @@
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Alias, Int, Ranged, Stmt};
+use rustpython_parser::ast::{self, Alias, Identifier, Int, Ranged, Stmt};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -74,7 +74,7 @@ pub(crate) fn manual_from_import(
     if checker.patch(diagnostic.kind.rule()) {
         if names.len() == 1 {
             let node = ast::StmtImportFrom {
-                module: Some(module.into()),
+                module: Some(Identifier::new(module.to_string(), TextRange::default())),
                 names: vec![Alias {
                     name: asname.clone(),
                     asname: None,

diff --git a/crates/ruff/src/rules/pylint/rules/useless_import_alias.rs b/crates/ruff/src/rules/pylint/rules/useless_import_alias.rs
@@ -43,7 +43,7 @@ pub(crate) fn useless_import_alias(checker: &mut Checker, alias: &Alias) {
     if alias.name.contains('.') {
         return;
     }
-    if &alias.name != asname {
+    if alias.name.as_str() != asname.as_str() {
         return;
     }
 

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
@@ -1,7 +1,9 @@
 use anyhow::{bail, Result};
 use log::debug;
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Keyword, Ranged, Stmt};
+use rustpython_parser::ast::{
+    self, Constant, Expr, ExprContext, Identifier, Keyword, Ranged, Stmt,
+};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -185,7 +187,7 @@ fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result<Vec<S
 /// keywords.
 fn create_class_def_stmt(typename: &str, body: Vec<Stmt>, base_class: &Expr) -> Stmt {
     ast::StmtClassDef {
-        name: typename.into(),
+        name: Identifier::new(typename.to_string(), TextRange::default()),
         bases: vec![base_class.clone()],
         keywords: vec![],
         body,

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
@@ -1,7 +1,9 @@
 use anyhow::{bail, Result};
 use log::debug;
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Keyword, Ranged, Stmt};
+use rustpython_parser::ast::{
+    self, Constant, Expr, ExprContext, Identifier, Keyword, Ranged, Stmt,
+};
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -120,7 +122,7 @@ fn create_class_def_stmt(
         None => vec![],
     };
     ast::StmtClassDef {
-        name: class_name.into(),
+        name: Identifier::new(class_name.to_string(), TextRange::default()),
         bases: vec![base_class.clone()],
         keywords,
         body,

diff --git a/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs b/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs
@@ -129,7 +129,7 @@ pub(crate) fn super_call_with_parameters(
         return;
     };
 
-    if !(first_arg_id == parent_name && second_arg_id == parent_arg) {
+    if !(first_arg_id == parent_name.as_str() && second_arg_id == parent_arg.as_str()) {
         return;
     }
 

diff --git a/crates/ruff/src/rules/ruff/rules/implicit_optional.rs b/crates/ruff/src/rules/ruff/rules/implicit_optional.rs
@@ -289,7 +289,7 @@ fn generate_fix(checker: &Checker, conversion_type: ConversionType, expr: &Expr)
             let new_expr = Expr::Subscript(ast::ExprSubscript {
                 range: TextRange::default(),
                 value: Box::new(Expr::Name(ast::ExprName {
-                    id: binding.into(),
+                    id: binding,
                     ctx: ast::ExprContext::Store,
                     range: TextRange::default(),
                 })),

diff --git a/crates/ruff/src/rules/tryceratops/rules/useless_try_except.rs b/crates/ruff/src/rules/tryceratops/rules/useless_try_except.rs
@@ -50,7 +50,7 @@ pub(crate) fn useless_try_except(checker: &mut Checker, handlers: &[ExceptHandle
             if let Some(expr) = exc {
                 // E.g., `except ... as e: raise e`
                 if let Expr::Name(ast::ExprName { id, .. }) = expr.as_ref() {
-                    if Some(id) == name.as_ref() {
+                    if name.as_ref().map_or(false, |name| name.as_str() == id) {
                         return Some(Diagnostic::new(UselessTryExcept, handler.range()));
                     }
                 }

diff --git a/crates/ruff/src/rules/tryceratops/rules/verbose_log_message.rs b/crates/ruff/src/rules/tryceratops/rules/verbose_log_message.rs
@@ -86,7 +86,7 @@ pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandl
                         names
                     };
                     for expr in names {
-                        if expr.id == *target {
+                        if expr.id == target.as_str() {
                             checker
                                 .diagnostics
                                 .push(Diagnostic::new(VerboseLogMessage, expr.range()));
-Original file line number
+Diff line change
@@ Expand Up @@
         if alias.name.contains('.') {
             return;
         }
-        if &alias.name != asname {
+        if alias.name.as_str() != asname.as_str() {
             return;
         }
@@ Expand Down @@