Remove identifier lexing in favor of parser ranges (#5195)

## Summary Now that all identifiers include ranges (#5194), we can remove a ton of this "custom lexing" code that we have to sketchily extract identifier ranges from source. ## Test Plan `cargo test`
astral-sh · Jun 20, 2023 · 7bc33a8 · 7bc33a8
1 parent 6331598
commit 7bc33a8
Show file tree

Hide file tree

Showing 40 changed files with 136 additions and 430 deletions.
diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs
@@ -366,9 +366,7 @@ where
                 }
                 if self.enabled(Rule::AmbiguousFunctionName) {
                     if let Some(diagnostic) =
-                        pycodestyle::rules::ambiguous_function_name(name, || {
-                            stmt.identifier(self.locator)
-                        })
+                        pycodestyle::rules::ambiguous_function_name(name, || stmt.identifier())
                     {
                         self.diagnostics.push(diagnostic);
                     }
@@ -383,7 +381,6 @@ where
                         decorator_list,
                         &self.settings.pep8_naming.ignore_names,
                         &self.semantic,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -452,7 +449,6 @@ where
                         stmt,
                         name,
                         &self.settings.pep8_naming.ignore_names,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -503,7 +499,6 @@ where
                         name,
                         body,
                         self.settings.mccabe.max_complexity,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -523,7 +518,6 @@ where
                         stmt,
                         body,
                         self.settings.pylint.max_returns,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -533,7 +527,6 @@ where
                         stmt,
                         body,
                         self.settings.pylint.max_branches,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -543,7 +536,6 @@ where
                         stmt,
                         body,
                         self.settings.pylint.max_statements,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -605,7 +597,6 @@ where
                         name,
                         decorator_list,
                         args,
-                        self.locator,
                     );
                 }
                 if self.enabled(Rule::FStringDocstring) {
@@ -693,9 +684,9 @@ where
                     pyupgrade::rules::unnecessary_class_parentheses(self, class_def, stmt);
                 }
                 if self.enabled(Rule::AmbiguousClassName) {
-                    if let Some(diagnostic) = pycodestyle::rules::ambiguous_class_name(name, || {
-                        stmt.identifier(self.locator)
-                    }) {
+                    if let Some(diagnostic) =
+                        pycodestyle::rules::ambiguous_class_name(name, || stmt.identifier())
+                    {
                         self.diagnostics.push(diagnostic);
                     }
                 }
@@ -704,7 +695,6 @@ where
                         stmt,
                         name,
                         &self.settings.pep8_naming.ignore_names,
-                        self.locator,
                     ) {
                         self.diagnostics.push(diagnostic);
                     }
@@ -714,7 +704,6 @@ where
                         stmt,
                         bases,
                         name,
-                        self.locator,
                         &self.settings.pep8_naming.ignore_names,
                     ) {
                         self.diagnostics.push(diagnostic);
@@ -813,7 +802,7 @@ where
                         let qualified_name = &alias.name;
                         self.add_binding(
                             name,
-                            alias.identifier(self.locator),
+                            alias.identifier(),
                             BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }),
                             BindingFlags::EXTERNAL,
                         );
@@ -834,7 +823,7 @@ where
                         let qualified_name = &alias.name;
                         self.add_binding(
                             name,
-                            alias.identifier(self.locator),
+                            alias.identifier(),
                             BindingKind::Import(Import { qualified_name }),
                             flags,
                         );
@@ -1052,7 +1041,7 @@ where
 
                         self.add_binding(
                             name,
-                            alias.identifier(self.locator),
+                            alias.identifier(),
                             BindingKind::FutureImport,
                             BindingFlags::empty(),
                         );
@@ -1123,7 +1112,7 @@ where
                             helpers::format_import_from_member(level, module, &alias.name);
                         self.add_binding(
                             name,
-                            alias.identifier(self.locator),
+                            alias.identifier(),
                             BindingKind::FromImport(FromImport { qualified_name }),
                             flags,
                         );
@@ -1804,7 +1793,7 @@ where
 
                 self.add_binding(
                     name,
-                    stmt.identifier(self.locator),
+                    stmt.identifier(),
                     BindingKind::FunctionDefinition,
                     BindingFlags::empty(),
                 );
@@ -2029,7 +2018,7 @@ where
                 self.semantic.pop_definition();
                 self.add_binding(
                     name,
-                    stmt.identifier(self.locator),
+                    stmt.identifier(),
                     BindingKind::ClassDefinition,
                     BindingFlags::empty(),
                 );
@@ -3846,7 +3835,7 @@ where
                 }
                 match name {
                     Some(name) => {
-                        let range = except_handler.try_identifier(self.locator).unwrap();
+                        let range = except_handler.try_identifier().unwrap();
 
                         if self.enabled(Rule::AmbiguousVariableName) {
                             if let Some(diagnostic) =
@@ -3961,7 +3950,7 @@ where
         // upstream.
         self.add_binding(
             &arg.arg,
-            arg.identifier(self.locator),
+            arg.identifier(),
             BindingKind::Argument,
             BindingFlags::empty(),
         );
@@ -4001,7 +3990,7 @@ where
         {
             self.add_binding(
                 name,
-                pattern.try_identifier(self.locator).unwrap(),
+                pattern.try_identifier().unwrap(),
                 BindingKind::Assignment,
                 BindingFlags::empty(),
             );

diff --git a/crates/ruff/src/rules/flake8_annotations/rules/definition.rs b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs
@@ -653,7 +653,7 @@ pub(crate) fn definition(
                     MissingReturnTypeClassMethod {
                         name: name.to_string(),
                     },
-                    stmt.identifier(checker.locator),
+                    stmt.identifier(),
                 ));
             }
         } else if is_method
@@ -664,7 +664,7 @@ pub(crate) fn definition(
                     MissingReturnTypeStaticMethod {
                         name: name.to_string(),
                     },
-                    stmt.identifier(checker.locator),
+                    stmt.identifier(),
                 ));
             }
         } else if is_method && visibility::is_init(name) {
@@ -676,7 +676,7 @@ pub(crate) fn definition(
                         MissingReturnTypeSpecialMethod {
                             name: name.to_string(),
                         },
-                        stmt.identifier(checker.locator),
+                        stmt.identifier(),
                     );
                     if checker.patch(diagnostic.kind.rule()) {
                         diagnostic.try_set_fix(|| {
@@ -693,7 +693,7 @@ pub(crate) fn definition(
                     MissingReturnTypeSpecialMethod {
                         name: name.to_string(),
                     },
-                    stmt.identifier(checker.locator),
+                    stmt.identifier(),
                 );
                 if checker.patch(diagnostic.kind.rule()) {
                     if let Some(return_type) = simple_magic_return_type(name) {
@@ -713,7 +713,7 @@ pub(crate) fn definition(
                             MissingReturnTypeUndocumentedPublicFunction {
                                 name: name.to_string(),
                             },
-                            stmt.identifier(checker.locator),
+                            stmt.identifier(),
                         ));
                     }
                 }
@@ -723,7 +723,7 @@ pub(crate) fn definition(
                             MissingReturnTypePrivateFunction {
                                 name: name.to_string(),
                             },
-                            stmt.identifier(checker.locator),
+                            stmt.identifier(),
                         ));
                     }
                 }

diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/abstract_base_class.rs b/crates/ruff/src/rules/flake8_bugbear/rules/abstract_base_class.rs
@@ -134,7 +134,7 @@ pub(crate) fn abstract_base_class(
                 AbstractBaseClassWithoutAbstractMethod {
                     name: name.to_string(),
                 },
-                stmt.identifier(checker.locator),
+                stmt.identifier(),
             ));
         }
     }

diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/f_string_docstring.rs b/crates/ruff/src/rules/flake8_bugbear/rules/f_string_docstring.rs
@@ -29,8 +29,7 @@ pub(crate) fn f_string_docstring(checker: &mut Checker, body: &[Stmt]) {
     let Expr::JoinedStr ( _) = value.as_ref() else {
         return;
     };
-    checker.diagnostics.push(Diagnostic::new(
-        FStringDocstring,
-        stmt.identifier(checker.locator),
-    ));
+    checker
+        .diagnostics
+        .push(Diagnostic::new(FStringDocstring, stmt.identifier()));
 }
diff --git a/crates/ruff/src/rules/flake8_builtins/helpers.rs b/crates/ruff/src/rules/flake8_builtins/helpers.rs
@@ -1,8 +1,7 @@
 use ruff_text_size::TextRange;
 use rustpython_parser::ast::{ExceptHandler, Expr, Ranged, Stmt};
 
-use ruff_python_ast::identifier::Identifier;
-use ruff_python_ast::source_code::Locator;
+use ruff_python_ast::identifier::{Identifier, TryIdentifier};
 use ruff_python_stdlib::builtins::BUILTINS;
 
 pub(super) fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool {
@@ -16,12 +15,12 @@ pub(crate) enum AnyShadowing<'a> {
     ExceptHandler(&'a ExceptHandler),
 }
 
-impl AnyShadowing<'_> {
-    pub(crate) fn range(self, locator: &Locator) -> TextRange {
+impl Identifier for AnyShadowing<'_> {
+    fn identifier(&self) -> TextRange {
         match self {
             AnyShadowing::Expression(expr) => expr.range(),
-            AnyShadowing::Statement(stmt) => stmt.identifier(locator),
-            AnyShadowing::ExceptHandler(handler) => handler.range(),
+            AnyShadowing::Statement(stmt) => stmt.identifier(),
+            AnyShadowing::ExceptHandler(handler) => handler.try_identifier().unwrap(),
         }
     }
 }

diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs
@@ -1,6 +1,7 @@
 use ruff_diagnostics::Diagnostic;
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::identifier::Identifier;
 use rustpython_parser::ast;
 
 use crate::checkers::ast::Checker;
@@ -83,7 +84,7 @@ pub(crate) fn builtin_attribute_shadowing(
             BuiltinAttributeShadowing {
                 name: name.to_string(),
             },
-            shadowing.range(checker.locator),
+            shadowing.identifier(),
         ));
     }
 }
diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs
@@ -1,6 +1,7 @@
 use ruff_diagnostics::Diagnostic;
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::identifier::Identifier;
 
 use crate::checkers::ast::Checker;
 
@@ -68,7 +69,7 @@ pub(crate) fn builtin_variable_shadowing(
             BuiltinVariableShadowing {
                 name: name.to_string(),
             },
-            shadowing.range(checker.locator),
+            shadowing.identifier(),
         ));
     }
 }
diff --git a/...rc/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap b/...rc/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap
@@ -122,15 +122,13 @@ A001.py:16:7: A001 Variable `slice` is shadowing a Python builtin
 17 |     pass
    |
 
-A001.py:21:1: A001 Variable `ValueError` is shadowing a Python builtin
-   |
-19 |   try:
-20 |       ...
-21 | / except ImportError as ValueError:
-22 | |     ...
-   | |_______^ A001
-23 |   
-24 |   for memoryview, *bytearray in []:
+A001.py:21:23: A001 Variable `ValueError` is shadowing a Python builtin
+   |
+19 | try:
+20 |     ...
+21 | except ImportError as ValueError:
+   |                       ^^^^^^^^^^ A001
+22 |     ...
    |
 
 A001.py:24:5: A001 Variable `memoryview` is shadowing a Python builtin

diff --git a/...tins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap b/...tins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap
@@ -102,15 +102,13 @@ A001.py:16:7: A001 Variable `slice` is shadowing a Python builtin
 17 |     pass
    |
 
-A001.py:21:1: A001 Variable `ValueError` is shadowing a Python builtin
-   |
-19 |   try:
-20 |       ...
-21 | / except ImportError as ValueError:
-22 | |     ...
-   | |_______^ A001
-23 |   
-24 |   for memoryview, *bytearray in []:
+A001.py:21:23: A001 Variable `ValueError` is shadowing a Python builtin
+   |
+19 | try:
+20 |     ...
+21 | except ImportError as ValueError:
+   |                       ^^^^^^^^^^ A001
+22 |     ...
    |
 
 A001.py:24:5: A001 Variable `memoryview` is shadowing a Python builtin

diff --git a/...8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__custom.snap b/...8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__custom.snap
@@ -16,7 +16,7 @@ custom.py:4:8: ICN001 `dask.array` should be imported as `da`
   |
 3 | import altair  # unconventional
 4 | import dask.array  # unconventional
-  |        ^^^^ ICN001
+  |        ^^^^^^^^^^ ICN001
 5 | import dask.dataframe  # unconventional
 6 | import matplotlib.pyplot  # unconventional
   |
@@ -27,7 +27,7 @@ custom.py:5:8: ICN001 `dask.dataframe` should be imported as `dd`
 3 | import altair  # unconventional
 4 | import dask.array  # unconventional
 5 | import dask.dataframe  # unconventional
-  |        ^^^^ ICN001
+  |        ^^^^^^^^^^^^^^ ICN001
 6 | import matplotlib.pyplot  # unconventional
 7 | import numpy  # unconventional
   |
@@ -38,7 +38,7 @@ custom.py:6:8: ICN001 `matplotlib.pyplot` should be imported as `plt`
 4 | import dask.array  # unconventional
 5 | import dask.dataframe  # unconventional
 6 | import matplotlib.pyplot  # unconventional
-  |        ^^^^^^^^^^ ICN001
+  |        ^^^^^^^^^^^^^^^^^ ICN001
 7 | import numpy  # unconventional
 8 | import pandas  # unconventional
   |
@@ -115,7 +115,7 @@ custom.py:13:8: ICN001 `plotly.express` should be imported as `px`
 11 | import holoviews  # unconventional
 12 | import panel  # unconventional
 13 | import plotly.express  # unconventional
-   |        ^^^^^^ ICN001
+   |        ^^^^^^^^^^^^^^ ICN001
 14 | import matplotlib  # unconventional
 15 | import polars  # unconventional
    |