mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-02 14:51:25 +00:00
[pyupgrade
] Avoid reporting __future__
features as unnecessary when they are used (UP010
) (#19769)
## Summary Resolves #19561 Fixes the [unnecessary-future-import (UP010)](https://docs.astral.sh/ruff/rules/unnecessary-future-import/) rule to correctly identify when imported __future__ modules are actually used in the code, preventing false positives. I assume there is no way to check usage in `analyze::statements`, because we don't have any usage bindings for imports. To determine unused imports, we have to fully scan the file to create bindings and then check usage, similar to [unused-import (F401)](https://docs.astral.sh/ruff/rules/unused-import/#unused-import-f401). So, `Rule::UnnecessaryFutureImport` was moved from the `analyze::statements` to the `analyze::deferred_scopes` stage. This caused the need to change the logic of future import handling to a bindings-based approach. Also, the diagnostic report was changed. Before ``` | 1 | from __future__ import nested_scopes, generators | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010 ``` after ``` | 1 | from __future__ import nested_scopes, generators | ^^^^^^^^^^^^^ UP010 ``` I believe this is the correct way, because `generators` may be used, but `nested_scopes` is not. ### Special case I've found out about some specific case. ```python from __future__ import nested_scopes nested_scopes = 1 ``` Here we can treat `nested_scopes` as an unused import because the variable `nested_scopes` shadows it and we can safely remove the future import (my fix does it). But [F401](https://docs.astral.sh/ruff/rules/unused-import/#unused-import-f401) not triggered for such case ([sandbox](https://play.ruff.rs/296d9c7e-0f02-4659-b0c0-78cc21f3de76)) ``` from foo import print_function print_function = 1 ``` In my mind, `print_function` here is an unused import and should be deleted (my IDE highlight it). What do you think? ## Test Plan Added test cases and snapshots: - Split test file into separate _0 and _1 files for appropriate checks. - Added test cases to verify fixes when future module are used. --------- Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
This commit is contained in:
parent
d04dcd991b
commit
7b75aee21d
8 changed files with 213 additions and 81 deletions
18
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_1.py
vendored
Normal file
18
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_1.py
vendored
Normal file
|
@ -0,0 +1,18 @@
|
|||
from __future__ import nested_scopes, generators
|
||||
from __future__ import with_statement, unicode_literals
|
||||
|
||||
from __future__ import absolute_import, division
|
||||
from __future__ import generator_stop
|
||||
from __future__ import print_function, nested_scopes, generator_stop
|
||||
|
||||
print(with_statement)
|
||||
generators = 1
|
||||
|
||||
|
||||
class Foo():
|
||||
|
||||
def boo(self):
|
||||
print(division)
|
||||
|
||||
|
||||
__all__ = ["print_function", "generator_stop"]
|
|
@ -1,10 +1,11 @@
|
|||
use ruff_python_ast::PythonVersion;
|
||||
use ruff_python_semantic::{Binding, ScopeKind};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::codes::Rule;
|
||||
use crate::rules::{
|
||||
flake8_builtins, flake8_pyi, flake8_type_checking, flake8_unused_arguments, pep8_naming,
|
||||
pyflakes, pylint, ruff,
|
||||
pyflakes, pylint, pyupgrade, ruff,
|
||||
};
|
||||
|
||||
/// Run lint rules over all deferred scopes in the [`SemanticModel`].
|
||||
|
@ -45,6 +46,7 @@ pub(crate) fn deferred_scopes(checker: &Checker) {
|
|||
Rule::UnusedStaticMethodArgument,
|
||||
Rule::UnusedUnpackedVariable,
|
||||
Rule::UnusedVariable,
|
||||
Rule::UnnecessaryFutureImport,
|
||||
]) {
|
||||
return;
|
||||
}
|
||||
|
@ -224,6 +226,11 @@ pub(crate) fn deferred_scopes(checker: &Checker) {
|
|||
if checker.is_rule_enabled(Rule::UnusedImport) {
|
||||
pyflakes::rules::unused_import(checker, scope);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::UnnecessaryFutureImport) {
|
||||
if checker.target_version() >= PythonVersion::PY37 {
|
||||
pyupgrade::rules::unnecessary_future_import(checker, scope);
|
||||
}
|
||||
}
|
||||
|
||||
if checker.is_rule_enabled(Rule::ImportPrivateName) {
|
||||
pylint::rules::import_private_name(checker, scope);
|
||||
|
|
|
@ -728,13 +728,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
pylint::rules::non_ascii_module_import(checker, alias);
|
||||
}
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::UnnecessaryFutureImport) {
|
||||
if checker.target_version() >= PythonVersion::PY37 {
|
||||
if let Some("__future__") = module {
|
||||
pyupgrade::rules::unnecessary_future_import(checker, stmt, names);
|
||||
}
|
||||
}
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::DeprecatedMockImport) {
|
||||
pyupgrade::rules::deprecated_mock_import(checker, stmt);
|
||||
}
|
||||
|
|
|
@ -101,7 +101,8 @@ mod tests {
|
|||
#[test_case(Rule::UnnecessaryClassParentheses, Path::new("UP039.py"))]
|
||||
#[test_case(Rule::UnnecessaryDefaultTypeArgs, Path::new("UP043.py"))]
|
||||
#[test_case(Rule::UnnecessaryEncodeUTF8, Path::new("UP012.py"))]
|
||||
#[test_case(Rule::UnnecessaryFutureImport, Path::new("UP010.py"))]
|
||||
#[test_case(Rule::UnnecessaryFutureImport, Path::new("UP010_0.py"))]
|
||||
#[test_case(Rule::UnnecessaryFutureImport, Path::new("UP010_1.py"))]
|
||||
#[test_case(Rule::UselessMetaclassType, Path::new("UP001.py"))]
|
||||
#[test_case(Rule::UselessObjectInheritance, Path::new("UP004.py"))]
|
||||
#[test_case(Rule::YieldInForLoop, Path::new("UP028_0.py"))]
|
||||
|
|
|
@ -1,10 +1,11 @@
|
|||
use std::collections::BTreeSet;
|
||||
use std::collections::{BTreeSet, HashMap};
|
||||
|
||||
use itertools::Itertools;
|
||||
use itertools::{Itertools, chain};
|
||||
use ruff_python_semantic::NodeId;
|
||||
|
||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef};
|
||||
use ruff_python_semantic::NameImport;
|
||||
use ruff_python_semantic::{NameImport, Scope};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
@ -111,68 +112,81 @@ pub(crate) fn is_import_required_by_isort(
|
|||
}
|
||||
|
||||
/// UP010
|
||||
pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: &[Alias]) {
|
||||
let mut unused_imports: Vec<&Alias> = vec![];
|
||||
for alias in names {
|
||||
if alias.asname.is_some() {
|
||||
continue;
|
||||
}
|
||||
pub(crate) fn unnecessary_future_import(checker: &Checker, scope: &Scope) {
|
||||
let mut unused_imports: HashMap<NodeId, Vec<&Alias>> = HashMap::new();
|
||||
for future_name in chain(PY33_PLUS_REMOVE_FUTURES, PY37_PLUS_REMOVE_FUTURES).unique() {
|
||||
for binding_id in scope.get_all(future_name) {
|
||||
let binding = checker.semantic().binding(binding_id);
|
||||
if binding.kind.is_future_import() && binding.is_unused() {
|
||||
let Some(node_id) = binding.source else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if is_import_required_by_isort(
|
||||
&checker.settings().isort.required_imports,
|
||||
stmt.into(),
|
||||
alias,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
let stmt = checker.semantic().statement(node_id);
|
||||
if let Stmt::ImportFrom(ast::StmtImportFrom { names, .. }) = stmt {
|
||||
let Some(alias) = names
|
||||
.iter()
|
||||
.find(|alias| alias.name.as_str() == binding.name(checker.source()))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if PY33_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str())
|
||||
|| PY37_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str())
|
||||
{
|
||||
unused_imports.push(alias);
|
||||
if alias.asname.is_some() {
|
||||
continue;
|
||||
}
|
||||
|
||||
if is_import_required_by_isort(
|
||||
&checker.settings().isort.required_imports,
|
||||
stmt.into(),
|
||||
alias,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
unused_imports.entry(node_id).or_default().push(alias);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if unused_imports.is_empty() {
|
||||
return;
|
||||
for (node_id, unused_aliases) in unused_imports {
|
||||
let mut diagnostic = checker.report_diagnostic(
|
||||
UnnecessaryFutureImport {
|
||||
names: unused_aliases
|
||||
.iter()
|
||||
.map(|alias| alias.name.to_string())
|
||||
.sorted()
|
||||
.collect(),
|
||||
},
|
||||
checker.semantic().statement(node_id).range(),
|
||||
);
|
||||
|
||||
diagnostic.try_set_fix(|| {
|
||||
let statement = checker.semantic().statement(node_id);
|
||||
let parent = checker.semantic().parent_statement(node_id);
|
||||
let edit = fix::edits::remove_unused_imports(
|
||||
unused_aliases
|
||||
.iter()
|
||||
.map(|alias| &alias.name)
|
||||
.map(ast::Identifier::as_str),
|
||||
statement,
|
||||
parent,
|
||||
checker.locator(),
|
||||
checker.stylist(),
|
||||
checker.indexer(),
|
||||
)?;
|
||||
|
||||
let range = edit.range();
|
||||
let applicability = if checker.comment_ranges().intersects(range) {
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
Applicability::Safe
|
||||
};
|
||||
|
||||
Ok(
|
||||
Fix::applicable_edit(edit, applicability).isolate(Checker::isolation(
|
||||
checker.semantic().current_statement_parent_id(),
|
||||
)),
|
||||
)
|
||||
});
|
||||
}
|
||||
let mut diagnostic = checker.report_diagnostic(
|
||||
UnnecessaryFutureImport {
|
||||
names: unused_imports
|
||||
.iter()
|
||||
.map(|alias| alias.name.to_string())
|
||||
.sorted()
|
||||
.collect(),
|
||||
},
|
||||
stmt.range(),
|
||||
);
|
||||
|
||||
diagnostic.try_set_fix(|| {
|
||||
let statement = checker.semantic().current_statement();
|
||||
let parent = checker.semantic().current_statement_parent();
|
||||
let edit = fix::edits::remove_unused_imports(
|
||||
unused_imports
|
||||
.iter()
|
||||
.map(|alias| &alias.name)
|
||||
.map(ast::Identifier::as_str),
|
||||
statement,
|
||||
parent,
|
||||
checker.locator(),
|
||||
checker.stylist(),
|
||||
checker.indexer(),
|
||||
)?;
|
||||
|
||||
let range = edit.range();
|
||||
let applicability = if checker.comment_ranges().intersects(range) {
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
Applicability::Safe
|
||||
};
|
||||
|
||||
Ok(
|
||||
Fix::applicable_edit(edit, applicability).isolate(Checker::isolation(
|
||||
checker.semantic().current_statement_parent_id(),
|
||||
)),
|
||||
)
|
||||
});
|
||||
}
|
||||
|
|
|
@ -2,7 +2,7 @@
|
|||
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
|
||||
---
|
||||
UP010 [*] Unnecessary `__future__` imports `generators`, `nested_scopes` for target Python version
|
||||
--> UP010.py:1:1
|
||||
--> UP010_0.py:1:1
|
||||
|
|
||||
1 | from __future__ import nested_scopes, generators
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
@ -18,7 +18,7 @@ help: Remove unnecessary `__future__` import
|
|||
4 3 | from __future__ import generator_stop
|
||||
|
||||
UP010 [*] Unnecessary `__future__` imports `unicode_literals`, `with_statement` for target Python version
|
||||
--> UP010.py:2:1
|
||||
--> UP010_0.py:2:1
|
||||
|
|
||||
1 | from __future__ import nested_scopes, generators
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
|
@ -36,7 +36,7 @@ help: Remove unnecessary `__future__` import
|
|||
5 4 | from __future__ import print_function, generator_stop
|
||||
|
||||
UP010 [*] Unnecessary `__future__` imports `absolute_import`, `division` for target Python version
|
||||
--> UP010.py:3:1
|
||||
--> UP010_0.py:3:1
|
||||
|
|
||||
1 | from __future__ import nested_scopes, generators
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
|
@ -56,7 +56,7 @@ help: Remove unnecessary `__future__` import
|
|||
6 5 | from __future__ import invalid_module, generators
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version
|
||||
--> UP010.py:4:1
|
||||
--> UP010_0.py:4:1
|
||||
|
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
3 | from __future__ import absolute_import, division
|
||||
|
@ -77,7 +77,7 @@ help: Remove unnecessary `__future__` import
|
|||
7 6 |
|
||||
|
||||
UP010 [*] Unnecessary `__future__` imports `generator_stop`, `print_function` for target Python version
|
||||
--> UP010.py:5:1
|
||||
--> UP010_0.py:5:1
|
||||
|
|
||||
3 | from __future__ import absolute_import, division
|
||||
4 | from __future__ import generator_stop
|
||||
|
@ -97,7 +97,7 @@ help: Remove unnecessary `__future__` import
|
|||
8 7 | if True:
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generators` for target Python version
|
||||
--> UP010.py:6:1
|
||||
--> UP010_0.py:6:1
|
||||
|
|
||||
4 | from __future__ import generator_stop
|
||||
5 | from __future__ import print_function, generator_stop
|
||||
|
@ -119,7 +119,7 @@ help: Remove unnecessary `__future__` import
|
|||
9 9 | from __future__ import generator_stop
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version
|
||||
--> UP010.py:9:5
|
||||
--> UP010_0.py:9:5
|
||||
|
|
||||
8 | if True:
|
||||
9 | from __future__ import generator_stop
|
||||
|
@ -138,7 +138,7 @@ help: Remove unnecessary `__future__` import
|
|||
12 11 | if True:
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generators` for target Python version
|
||||
--> UP010.py:10:5
|
||||
--> UP010_0.py:10:5
|
||||
|
|
||||
8 | if True:
|
||||
9 | from __future__ import generator_stop
|
||||
|
@ -159,7 +159,7 @@ help: Remove unnecessary `__future__` import
|
|||
13 12 | from __future__ import generator_stop
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version
|
||||
--> UP010.py:13:5
|
||||
--> UP010_0.py:13:5
|
||||
|
|
||||
12 | if True:
|
||||
13 | from __future__ import generator_stop
|
||||
|
@ -178,7 +178,7 @@ help: Remove unnecessary `__future__` import
|
|||
15 14 | from __future__ import generators # comment
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generators` for target Python version
|
||||
--> UP010.py:14:5
|
||||
--> UP010_0.py:14:5
|
||||
|
|
||||
12 | if True:
|
||||
13 | from __future__ import generator_stop
|
||||
|
@ -197,7 +197,7 @@ help: Remove unnecessary `__future__` import
|
|||
15 15 | from __future__ import generators # comment
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generators` for target Python version
|
||||
--> UP010.py:15:5
|
||||
--> UP010_0.py:15:5
|
||||
|
|
||||
13 | from __future__ import generator_stop
|
||||
14 | from __future__ import invalid_module, generators
|
|
@ -0,0 +1,99 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
|
||||
---
|
||||
UP010 [*] Unnecessary `__future__` imports `generators`, `nested_scopes` for target Python version
|
||||
--> UP010_1.py:1:1
|
||||
|
|
||||
1 | from __future__ import nested_scopes, generators
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
|
|
||||
help: Remove unnecessary `__future__` import
|
||||
|
||||
ℹ Safe fix
|
||||
1 |-from __future__ import nested_scopes, generators
|
||||
2 1 | from __future__ import with_statement, unicode_literals
|
||||
3 2 |
|
||||
4 3 | from __future__ import absolute_import, division
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `unicode_literals` for target Python version
|
||||
--> UP010_1.py:2:1
|
||||
|
|
||||
1 | from __future__ import nested_scopes, generators
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
3 |
|
||||
4 | from __future__ import absolute_import, division
|
||||
|
|
||||
help: Remove unnecessary `__future__` import
|
||||
|
||||
ℹ Safe fix
|
||||
1 1 | from __future__ import nested_scopes, generators
|
||||
2 |-from __future__ import with_statement, unicode_literals
|
||||
2 |+from __future__ import with_statement
|
||||
3 3 |
|
||||
4 4 | from __future__ import absolute_import, division
|
||||
5 5 | from __future__ import generator_stop
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `absolute_import` for target Python version
|
||||
--> UP010_1.py:4:1
|
||||
|
|
||||
2 | from __future__ import with_statement, unicode_literals
|
||||
3 |
|
||||
4 | from __future__ import absolute_import, division
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
5 | from __future__ import generator_stop
|
||||
6 | from __future__ import print_function, nested_scopes, generator_stop
|
||||
|
|
||||
help: Remove unnecessary `__future__` import
|
||||
|
||||
ℹ Safe fix
|
||||
1 1 | from __future__ import nested_scopes, generators
|
||||
2 2 | from __future__ import with_statement, unicode_literals
|
||||
3 3 |
|
||||
4 |-from __future__ import absolute_import, division
|
||||
4 |+from __future__ import division
|
||||
5 5 | from __future__ import generator_stop
|
||||
6 6 | from __future__ import print_function, nested_scopes, generator_stop
|
||||
7 7 |
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version
|
||||
--> UP010_1.py:5:1
|
||||
|
|
||||
4 | from __future__ import absolute_import, division
|
||||
5 | from __future__ import generator_stop
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
6 | from __future__ import print_function, nested_scopes, generator_stop
|
||||
|
|
||||
help: Remove unnecessary `__future__` import
|
||||
|
||||
ℹ Safe fix
|
||||
2 2 | from __future__ import with_statement, unicode_literals
|
||||
3 3 |
|
||||
4 4 | from __future__ import absolute_import, division
|
||||
5 |-from __future__ import generator_stop
|
||||
6 5 | from __future__ import print_function, nested_scopes, generator_stop
|
||||
7 6 |
|
||||
8 7 | print(with_statement)
|
||||
|
||||
UP010 [*] Unnecessary `__future__` import `nested_scopes` for target Python version
|
||||
--> UP010_1.py:6:1
|
||||
|
|
||||
4 | from __future__ import absolute_import, division
|
||||
5 | from __future__ import generator_stop
|
||||
6 | from __future__ import print_function, nested_scopes, generator_stop
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
7 |
|
||||
8 | print(with_statement)
|
||||
|
|
||||
help: Remove unnecessary `__future__` import
|
||||
|
||||
ℹ Safe fix
|
||||
3 3 |
|
||||
4 4 | from __future__ import absolute_import, division
|
||||
5 5 | from __future__ import generator_stop
|
||||
6 |-from __future__ import print_function, nested_scopes, generator_stop
|
||||
6 |+from __future__ import print_function, generator_stop
|
||||
7 7 |
|
||||
8 8 | print(with_statement)
|
||||
9 9 | generators = 1
|
Loading…
Add table
Add a link
Reference in a new issue