Fix ranges for global usages (#6917)

## Summary

The range of the usage from `Globals` should be the range of the
identifier, not the range of the full `global pandas` statement.

Closes https://github.com/astral-sh/ruff/issues/6914.
This commit is contained in:
Charlie Marsh 2023-08-27 11:27:07 -04:00 committed by GitHub
parent 381fc5b2a8
commit d0b051e447
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 77 additions and 30 deletions

View File

@ -0,0 +1,9 @@
"""Test cases for difficult renames."""
def rename_global():
try:
global pandas
import pandas
except ImportError:
return False

View File

@ -1895,6 +1895,8 @@ impl<'a> Checker<'a> {
for (name, range) in exports { for (name, range) in exports {
if let Some(binding_id) = self.semantic.global_scope().get(name) { if let Some(binding_id) = self.semantic.global_scope().get(name) {
// Mark anything referenced in `__all__` as used. // 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); self.semantic.add_global_reference(binding_id, range);
} else { } else {
if self.semantic.global_scope().uses_star_imports() { if self.semantic.global_scope().uses_star_imports() {

View File

@ -20,7 +20,7 @@ mod tests {
Path::new("flake8_import_conventions/defaults.py"), Path::new("flake8_import_conventions/defaults.py"),
&Settings::for_rule(Rule::UnconventionalImportAlias), &Settings::for_rule(Rule::UnconventionalImportAlias),
)?; )?;
assert_messages!("defaults", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -42,7 +42,7 @@ mod tests {
..Settings::for_rule(Rule::UnconventionalImportAlias) ..Settings::for_rule(Rule::UnconventionalImportAlias)
}, },
)?; )?;
assert_messages!("custom", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -75,7 +75,7 @@ mod tests {
..Settings::for_rule(Rule::BannedImportAlias) ..Settings::for_rule(Rule::BannedImportAlias)
}, },
)?; )?;
assert_messages!("custom_banned", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -98,7 +98,7 @@ mod tests {
..Settings::for_rule(Rule::BannedImportFrom) ..Settings::for_rule(Rule::BannedImportFrom)
}, },
)?; )?;
assert_messages!("custom_banned_from", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -122,7 +122,7 @@ mod tests {
..Settings::for_rule(Rule::UnconventionalImportAlias) ..Settings::for_rule(Rule::UnconventionalImportAlias)
}, },
)?; )?;
assert_messages!("remove_default", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -144,7 +144,7 @@ mod tests {
..Settings::for_rule(Rule::UnconventionalImportAlias) ..Settings::for_rule(Rule::UnconventionalImportAlias)
}, },
)?; )?;
assert_messages!("override_default", diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
@ -169,7 +169,17 @@ mod tests {
..Settings::for_rule(Rule::UnconventionalImportAlias) ..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(()) Ok(())
} }
} }

View File

@ -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

View File

@ -1,100 +1,100 @@
--- ---
source: crates/ruff/src/rules/pylint/mod.rs 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): 15 | def fix_constant(value):
16 | """All this is ok, but try not to use `global` ;)""" 16 | """All this is ok, but try not to use `global` ;)"""
17 | global CONSTANT # [global-statement] 17 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
18 | print(CONSTANT) 18 | print(CONSTANT)
19 | CONSTANT = value 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(): 22 | def global_with_import():
23 | """Should only warn for global-statement when using `Import` node""" 23 | """Should only warn for global-statement when using `Import` node"""
24 | global sys # [global-statement] 24 | global sys # [global-statement]
| ^^^^^^^^^^ PLW0603 | ^^^ PLW0603
25 | import sys 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(): 28 | def global_with_import_from():
29 | """Should only warn for global-statement when using `ImportFrom` node""" 29 | """Should only warn for global-statement when using `ImportFrom` node"""
30 | global namedtuple # [global-statement] 30 | global namedtuple # [global-statement]
| ^^^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^^^ PLW0603
31 | from collections import namedtuple 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(): 34 | def global_del():
35 | """Deleting the global name prevents `global-variable-not-assigned`""" 35 | """Deleting the global name prevents `global-variable-not-assigned`"""
36 | global CONSTANT # [global-statement] 36 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
37 | print(CONSTANT) 37 | print(CONSTANT)
38 | del 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(): 41 | def global_operator_assign():
42 | """Operator assigns should only throw a global statement error""" 42 | """Operator assigns should only throw a global statement error"""
43 | global CONSTANT # [global-statement] 43 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
44 | print(CONSTANT) 44 | print(CONSTANT)
45 | CONSTANT += 1 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(): 48 | def global_function_assign():
49 | """Function assigns should only throw a global statement error""" 49 | """Function assigns should only throw a global statement error"""
50 | global CONSTANT # [global-statement] 50 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
51 | 51 |
52 | def CONSTANT(): 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(): 58 | def override_func():
59 | """Overriding a function should only throw a global statement error""" 59 | """Overriding a function should only throw a global statement error"""
60 | global FUNC # [global-statement] 60 | global FUNC # [global-statement]
| ^^^^^^^^^^^ PLW0603 | ^^^^ PLW0603
61 | 61 |
62 | def FUNC(): 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(): 68 | def override_class():
69 | """Overriding a class should only throw a global statement error""" 69 | """Overriding a class should only throw a global statement error"""
70 | global CLASS # [global-statement] 70 | global CLASS # [global-statement]
| ^^^^^^^^^^^^ PLW0603 | ^^^^^ PLW0603
71 | 71 |
72 | class CLASS: 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(): 78 | def multiple_assignment():
79 | """Should warn on every assignment.""" 79 | """Should warn on every assignment."""
80 | global CONSTANT # [global-statement] 80 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
81 | CONSTANT = 1 81 | CONSTANT = 1
82 | CONSTANT = 2 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(): 78 | def multiple_assignment():
79 | """Should warn on every assignment.""" 79 | """Should warn on every assignment."""
80 | global CONSTANT # [global-statement] 80 | global CONSTANT # [global-statement]
| ^^^^^^^^^^^^^^^ PLW0603 | ^^^^^^^^ PLW0603
81 | CONSTANT = 1 81 | CONSTANT = 1
82 | CONSTANT = 2 82 | CONSTANT = 2
| |

View File

@ -5,7 +5,7 @@
use std::ops::Index; use std::ops::Index;
use ruff_python_ast as ast; use ruff_python_ast as ast;
use ruff_python_ast::Stmt; use ruff_python_ast::{Ranged, Stmt};
use ruff_text_size::TextRange; use ruff_text_size::TextRange;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
@ -75,9 +75,9 @@ impl<'a> GlobalsVisitor<'a> {
impl<'a> StatementVisitor<'a> for GlobalsVisitor<'a> { impl<'a> StatementVisitor<'a> for GlobalsVisitor<'a> {
fn visit_stmt(&mut self, stmt: &'a Stmt) { fn visit_stmt(&mut self, stmt: &'a Stmt) {
match stmt { match stmt {
Stmt::Global(ast::StmtGlobal { names, range }) => { Stmt::Global(ast::StmtGlobal { names, range: _ }) => {
for name in names { for name in names {
self.0.insert(name.as_str(), *range); self.0.insert(name.as_str(), name.range());
} }
} }
Stmt::FunctionDef(_) | Stmt::ClassDef(_) => { Stmt::FunctionDef(_) | Stmt::ClassDef(_) => {