[pyupgrade] Avoid reporting __future__ features as unnecessary when they are used (UP010) (#19769)

IDrokin117 · Igor Drokin · web-flow · commit 7b75aee21d45 · 2025-08-20T15:22:03.000-04:00
## 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>
diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_0.py
diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_1.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP010_1.py
@@ -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"]
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
@@ -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);
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -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);
             }
diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs
@@ -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"))]
diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs
@@ -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;
-        }
-
-        if is_import_required_by_isort(
-            &checker.settings().isort.required_imports,
-            stmt.into(),
-            alias,
-        ) {
-            continue;
-        }
-
-        if PY33_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str())
-            || PY37_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str())
-        {
-            unused_imports.push(alias);
+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;
+                };
+
+                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 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(),
-            )),
-        )
-    });
 }
diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010_0.py.snap
@@ -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
diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010_1.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010_1.py.snap

Original file line number	Diff line number	Diff line change
`@@ -728,13 +728,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {`
`728`	`728`	`pylint::rules::non_ascii_module_import(checker, alias);`
`729`	`729`	`}`
`730`	`730`	`}`
`731`		`- if checker.is_rule_enabled(Rule::UnnecessaryFutureImport) {`
`732`		`- if checker.target_version() >= PythonVersion::PY37 {`
`733`		`- if let Some("__future__") = module {`
`734`		`- pyupgrade::rules::unnecessary_future_import(checker, stmt, names);`
`735`		`- }`
`736`		`- }`
`737`		`- }`
`738`	`731`	`if checker.is_rule_enabled(Rule::DeprecatedMockImport) {`
`739`	`732`	`pyupgrade::rules::deprecated_mock_import(checker, stmt);`
`740`	`733`	`}`