diff --git a/crates/ruff/resources/test/fixtures/flake8_import_conventions/tricky.py b/crates/ruff/resources/test/fixtures/flake8_import_conventions/tricky.py new file mode 100644 index 0000000000..7360a7485a --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_import_conventions/tricky.py @@ -0,0 +1,9 @@ +"""Test cases for difficult renames.""" + + +def rename_global(): + try: + global pandas + import pandas + except ImportError: + return False diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index d19d212b19..a22111f266 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1895,6 +1895,8 @@ impl<'a> Checker<'a> { for (name, range) in exports { if let Some(binding_id) = self.semantic.global_scope().get(name) { // Mark anything referenced in `__all__` as used. + // TODO(charlie): `range` here should be the range of the name in `__all__`, not + // the range of `__all__` itself. self.semantic.add_global_reference(binding_id, range); } else { if self.semantic.global_scope().uses_star_imports() { diff --git a/crates/ruff/src/rules/flake8_import_conventions/mod.rs b/crates/ruff/src/rules/flake8_import_conventions/mod.rs index 3df13cbea4..46fc75fd89 100644 --- a/crates/ruff/src/rules/flake8_import_conventions/mod.rs +++ b/crates/ruff/src/rules/flake8_import_conventions/mod.rs @@ -20,7 +20,7 @@ mod tests { Path::new("flake8_import_conventions/defaults.py"), &Settings::for_rule(Rule::UnconventionalImportAlias), )?; - assert_messages!("defaults", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -42,7 +42,7 @@ mod tests { ..Settings::for_rule(Rule::UnconventionalImportAlias) }, )?; - assert_messages!("custom", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -75,7 +75,7 @@ mod tests { ..Settings::for_rule(Rule::BannedImportAlias) }, )?; - assert_messages!("custom_banned", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -98,7 +98,7 @@ mod tests { ..Settings::for_rule(Rule::BannedImportFrom) }, )?; - assert_messages!("custom_banned_from", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -122,7 +122,7 @@ mod tests { ..Settings::for_rule(Rule::UnconventionalImportAlias) }, )?; - assert_messages!("remove_default", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -144,7 +144,7 @@ mod tests { ..Settings::for_rule(Rule::UnconventionalImportAlias) }, )?; - assert_messages!("override_default", diagnostics); + assert_messages!(diagnostics); Ok(()) } @@ -169,7 +169,17 @@ mod tests { ..Settings::for_rule(Rule::UnconventionalImportAlias) }, )?; - assert_messages!("from_imports", diagnostics); + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn tricky() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_import_conventions/tricky.py"), + &Settings::for_rule(Rule::UnconventionalImportAlias), + )?; + assert_messages!(diagnostics); Ok(()) } } diff --git a/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__override_default.snap b/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__override_defaults.snap similarity index 100% rename from crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__override_default.snap rename to crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__override_defaults.snap diff --git a/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__remove_default.snap b/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__remove_defaults.snap similarity index 100% rename from crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__remove_default.snap rename to crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__remove_defaults.snap diff --git a/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__tricky.snap b/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__tricky.snap new file mode 100644 index 0000000000..1d93608266 --- /dev/null +++ b/crates/ruff/src/rules/flake8_import_conventions/snapshots/ruff__rules__flake8_import_conventions__tests__tricky.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/src/rules/flake8_import_conventions/mod.rs +--- +tricky.py:7:16: ICN001 [*] `pandas` should be imported as `pd` + | +5 | try: +6 | global pandas +7 | import pandas + | ^^^^^^ ICN001 +8 | except ImportError: +9 | return False + | + = help: Alias `pandas` to `pd` + +ℹ Suggested fix +3 3 | +4 4 | def rename_global(): +5 5 | try: +6 |- global pandas +7 |- import pandas + 6 |+ global pd + 7 |+ import pandas as pd +8 8 | except ImportError: +9 9 | return False + + diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0603_global_statement.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0603_global_statement.py.snap index 910f5d31fc..c84a187a2d 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0603_global_statement.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0603_global_statement.py.snap @@ -1,100 +1,100 @@ --- source: crates/ruff/src/rules/pylint/mod.rs --- -global_statement.py:17:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:17:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 15 | def fix_constant(value): 16 | """All this is ok, but try not to use `global` ;)""" 17 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 18 | print(CONSTANT) 19 | CONSTANT = value | -global_statement.py:24:5: PLW0603 Using the global statement to update `sys` is discouraged +global_statement.py:24:12: PLW0603 Using the global statement to update `sys` is discouraged | 22 | def global_with_import(): 23 | """Should only warn for global-statement when using `Import` node""" 24 | global sys # [global-statement] - | ^^^^^^^^^^ PLW0603 + | ^^^ PLW0603 25 | import sys | -global_statement.py:30:5: PLW0603 Using the global statement to update `namedtuple` is discouraged +global_statement.py:30:12: PLW0603 Using the global statement to update `namedtuple` is discouraged | 28 | def global_with_import_from(): 29 | """Should only warn for global-statement when using `ImportFrom` node""" 30 | global namedtuple # [global-statement] - | ^^^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^^^ PLW0603 31 | from collections import namedtuple | -global_statement.py:36:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:36:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 34 | def global_del(): 35 | """Deleting the global name prevents `global-variable-not-assigned`""" 36 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 37 | print(CONSTANT) 38 | del CONSTANT | -global_statement.py:43:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:43:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 41 | def global_operator_assign(): 42 | """Operator assigns should only throw a global statement error""" 43 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 44 | print(CONSTANT) 45 | CONSTANT += 1 | -global_statement.py:50:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:50:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 48 | def global_function_assign(): 49 | """Function assigns should only throw a global statement error""" 50 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 51 | 52 | def CONSTANT(): | -global_statement.py:60:5: PLW0603 Using the global statement to update `FUNC` is discouraged +global_statement.py:60:12: PLW0603 Using the global statement to update `FUNC` is discouraged | 58 | def override_func(): 59 | """Overriding a function should only throw a global statement error""" 60 | global FUNC # [global-statement] - | ^^^^^^^^^^^ PLW0603 + | ^^^^ PLW0603 61 | 62 | def FUNC(): | -global_statement.py:70:5: PLW0603 Using the global statement to update `CLASS` is discouraged +global_statement.py:70:12: PLW0603 Using the global statement to update `CLASS` is discouraged | 68 | def override_class(): 69 | """Overriding a class should only throw a global statement error""" 70 | global CLASS # [global-statement] - | ^^^^^^^^^^^^ PLW0603 + | ^^^^^ PLW0603 71 | 72 | class CLASS: | -global_statement.py:80:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:80:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 78 | def multiple_assignment(): 79 | """Should warn on every assignment.""" 80 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 81 | CONSTANT = 1 82 | CONSTANT = 2 | -global_statement.py:80:5: PLW0603 Using the global statement to update `CONSTANT` is discouraged +global_statement.py:80:12: PLW0603 Using the global statement to update `CONSTANT` is discouraged | 78 | def multiple_assignment(): 79 | """Should warn on every assignment.""" 80 | global CONSTANT # [global-statement] - | ^^^^^^^^^^^^^^^ PLW0603 + | ^^^^^^^^ PLW0603 81 | CONSTANT = 1 82 | CONSTANT = 2 | diff --git a/crates/ruff_python_semantic/src/globals.rs b/crates/ruff_python_semantic/src/globals.rs index ffaf1b16f9..03cdb687ce 100644 --- a/crates/ruff_python_semantic/src/globals.rs +++ b/crates/ruff_python_semantic/src/globals.rs @@ -5,7 +5,7 @@ use std::ops::Index; use ruff_python_ast as ast; -use ruff_python_ast::Stmt; +use ruff_python_ast::{Ranged, Stmt}; use ruff_text_size::TextRange; use rustc_hash::FxHashMap; @@ -75,9 +75,9 @@ impl<'a> GlobalsVisitor<'a> { impl<'a> StatementVisitor<'a> for GlobalsVisitor<'a> { fn visit_stmt(&mut self, stmt: &'a Stmt) { match stmt { - Stmt::Global(ast::StmtGlobal { names, range }) => { + Stmt::Global(ast::StmtGlobal { names, range: _ }) => { for name in names { - self.0.insert(name.as_str(), *range); + self.0.insert(name.as_str(), name.range()); } } Stmt::FunctionDef(_) | Stmt::ClassDef(_) => {