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

IDrokin117 · 2025-08-05T18:48:53Z

Summary

Resolves #19561

Fixes the unnecessary-future-import (UP010) 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). 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.

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 not triggered for such case (sandbox)

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.

github-actions · 2025-08-05T19:07:45Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

MichaReiser · 2025-08-11T08:13:26Z

Also, the diagnostic report was changed.

Note: This requires gating behind preview because it changes the rule's suppression ranges.

ntBre · 2025-08-12T15:56:48Z

Thanks Micha, that makes sense. Hopefully the preview-gating isn't too involved, so I think I'll still take a look at this. Your special case is also very interesting. Apparently assigning to __future__.print_function will override it but not assigning to print_function alone:

>>> from __future__ import print_function
>>> import __future__
>>> __future__.print_function
_Feature((2, 6, 0, 'alpha', 2), (3, 0, 0, 'alpha', 0), 1048576)
>>> print_function = 1
>>> __future__.print_function
_Feature((2, 6, 0, 'alpha', 2), (3, 0, 0, 'alpha', 0), 1048576)
>>> __future__.print_function = 1
>>> __future__.print_function
1
>>> from __future__ import print_function
>>> print_function
1

I'm not sure if we need this to be covered by F401, though, since F811 flags it as redefined-while-unused.

ntBre

Thanks, this looks good to me in general. I just had a few minor suggestions on the code itself.

More broadly, I think we should probably split this into two PRs. Converting to a binding-based rule and checking for uses of the __future__ imports is a bug fix that resolves the linked issue. But emitting separate diagnostics for each unused import and updating the ranges accordingly are conceptually separate and need to be preview-gated as Micha said. I think they're separate enough that it would be easier to review them in a separate PR.

I do think these changes are nice and worth having, just a better fit for a separate patch, in my opinion.

ntBre · 2025-08-12T16:03:36Z

crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs

-                .iter()
-                .map(|alias| &alias.name)
-                .map(ast::Identifier::as_str),
+            vec![alias.name.as_str()].into_iter(),


I think we could use std::iter::once here to avoid allocating a Vec just to call into_iter.

Noticed. Replaced with std::iter::once

crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs

ntBre · 2025-08-12T16:12:50Z

crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs

-    if unused_imports.is_empty() {
+/// Process unused future import statements and generate diagnostics with fixes.
+/// Filters out required isort imports and creates removal fixes for unused aliases.
+fn process_unused_future_import(checker: &Checker, stmt: &Stmt, alias: &Alias, node_id: NodeId) {


I'm not totally sold on splitting off this helper function. I think with judicious use of let-else we could put this back since it won't be too indented. But I don't feel too strongly either way.

Removed helper function.

moved UnnecessaryFutureImport rule from `statement.rs` to `deferred_scopes.rs` reworked unused future rules determination, now checks bindings added appropriate test cases and snapshots

IDrokin117 · 2025-08-13T16:01:56Z

@ntBre Thanks for your review.
I haven't put the rule behind a preview yet. Is setting RuleGroup::Preview sufficient?

More broadly, I think we should probably split this into two PRs. Converting to a binding-based rule and checking for uses of the __future__ imports is a bug fix that resolves the linked issue. But emitting separate diagnostics for each unused import and updating the ranges accordingly are conceptually separate and need to be preview-gated as Micha said. I think they're separate enough that it would be easier to review them in a separate PR.

Sorry, but I don't understand how to split the PR into two independent PRs. If I fix the bug which implies determining used names from the __future__ module, then I need to have the ability to remove only unused ones. Otherwise, it will introduce a bug. Previously, the fix removes the entire import statement, because there were no exceptions, but now it has to remove only the dedicated one. So, here:

from __future__ import absolute_import, division

print(division)

only absolute_import must be removed, not the entire line. Please let me know what I'm missing.

ntBre · 2025-08-13T16:49:27Z

I think you can update the range of the fix without updating the range of the diagnostic itself. The diagnostic range is used for the noqa code and needs to be modified only in preview, but updating the fix to remove only the unused import is a bug fix. Does that help? The old code also had support for removing only part of an import line without emitting multiple diagnostics, as you can see in this snapshot that didn't change on this branch:

ruff/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap

Lines 109 to 116 in f0b03c3

    
           help: Remove unnecessary `__future__` import 
        
           ℹ Safe fix 
        
           3 3 | from __future__ import absolute_import, division 
        
           4 4 | from __future__ import generator_stop 
        
           5 5 | from __future__ import print_function, generator_stop 
        
           6   |-from __future__ import invalid_module, generators 
        
             6 |+from __future__ import invalid_module

For the bug fix, I think you'd need to preserve the old behavior of gathering all of the unused imports in the same import line and emitting a single diagnostic. This is work we have to do one way or another because even if we keep the changes in the same PR, we have to preserve the old behavior when preview is disabled.

One way of implementing this might be grouping the diagnostics by parent statement, which you're already computing. Hopefully it's not too much trouble.

changed RuleGroup to Preview

codspeed-hq · 2025-08-13T17:20:37Z

CodSpeed Instrumentation Performance Report

Merging #19769 will not alter performance

_{Comparing IDrokin117:fix/ruff-19561 (e85d3d1) with main (df0648a)}

Summary

✅ 42 untouched benchmarks

revert diagnosis report changes

IDrokin117 · 2025-08-13T18:30:22Z

@ntBre

I think you can update the range of the fix without updating the range of the diagnostic itself. The diagnostic range is used for the noqa code and needs to be modified only in preview, but updating the fix to remove only the unused import is a bug fix. Does that help?

It was very helpful, thanks. I believe I implemented what you suggested. So

I reverted diagnosis changes -> previous test cases didn't change (except filename).
Current implementation aggregates aliases and applies one diagnosis per stmt (as before).

I will create a second PR with Preview changes when you approve the current PR.

ntBre

Excellent, thank you!

I'll look forward to the preview PR (if you're still interested, no pressure!). The ranges highlighting only the specific unused import were really nice :) but this neatly avoids a breaking change for now.

* main: (29 commits) [ty] add docstrings to completions based on type (#20008) [`pyupgrade`] Avoid reporting `__future__` features as unnecessary when they are used (`UP010`) (#19769) [`flake8-use-pathlib`] Add fixes for `PTH102` and `PTH103` (#19514) [ty] correctly ignore field specifiers when not specified (#20002) `Option::unwrap` is now const (#20007) [ty] Re-arrange "list modules" implementation for Salsa caching [ty] Test "list modules" versus "resolve module" in every mdtest [ty] Wire up "list modules" API to make module completions work [ty] Tweak some completion tests [ty] Add "list modules" implementation [ty] Lightly expose `FileModule` and `NamespacePackage` fields [ty] Add some more helper routines to `ModulePath` [ty] Fix a bug when converting `ModulePath` to `ModuleName` [ty] Split out another constructor for `ModuleName` [ty] Add stub-file tests to existing module resolver [ty] Expose some routines in the module resolver [ty] Add more path helper functions [`flake8-annotations`] Remove unused import in example (`ANN401`) (#20000) [ty] distinguish base conda from child conda (#19990) [ty] Fix server hang (#19991) ...

…en they are used (`UP010`) (astral-sh#19769) ## Summary Resolves astral-sh#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>

IDrokin117 force-pushed the fix/ruff-19561 branch from f11b18d to 31bbfe8 Compare August 5, 2025 18:57

ntBre self-requested a review August 6, 2025 18:21

ntBre added bug Something isn't working rule Implementing or modifying a lint rule labels Aug 6, 2025

IDrokin117 force-pushed the fix/ruff-19561 branch 2 times, most recently from 190d13b to c243b8d Compare August 9, 2025 20:40

ntBre reviewed Aug 12, 2025

View reviewed changes

IDrokin117 and others added 3 commits August 13, 2025 18:01

fix(UPO10): ignore rule if imported future module used

4628e65

moved UnnecessaryFutureImport rule from `statement.rs` to `deferred_scopes.rs` reworked unused future rules determination, now checks bindings added appropriate test cases and snapshots

refactor(UPO10): apply clippy

4e8eb59

fix(UP010): resolve snapshot conflicts

f2b99b9

IDrokin117 force-pushed the fix/ruff-19561 branch from c243b8d to f2b99b9 Compare August 13, 2025 15:51

feat(UP010): inline process_unused_future_import

497c3eb

changed RuleGroup to Preview

feat(UP010): group unused import for fix

e85d3d1

revert diagnosis report changes

IDrokin117 requested a review from ntBre August 17, 2025 10:22

ntBre approved these changes Aug 20, 2025

View reviewed changes

ntBre changed the title ~~[pyupgrade] Reworked to not report future features as unnecessary when they are used (UP010)~~ [pyupgrade] Avoid reporting __future__ features as unnecessary when they are used (UP010) Aug 20, 2025

ntBre merged commit 7b75aee into astral-sh:main Aug 20, 2025
35 checks passed

IDrokin117 mentioned this pull request Aug 21, 2025

[pyupgrade] Gated behind a preview of separate diagnostics (UP010) #20020

Closed

BrewTestBot mentioned this pull request Aug 21, 2025

ruff 0.12.10 Homebrew/homebrew-core#234408

Merged

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

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

Uh oh!

Conversation

IDrokin117 commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Special case

Test Plan

Uh oh!

github-actions bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

MichaReiser commented Aug 11, 2025

Uh oh!

ntBre commented Aug 12, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

IDrokin117 Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ntBre Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

IDrokin117 Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

IDrokin117 commented Aug 13, 2025

Uh oh!

ntBre commented Aug 13, 2025

Uh oh!

codspeed-hq bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #19769 will not alter performance

Summary

Uh oh!

IDrokin117 commented Aug 13, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

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

IDrokin117 commented Aug 5, 2025 •

edited

Loading

github-actions bot commented Aug 5, 2025 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Aug 13, 2025 •

edited

Loading