From 9bb09bfcc4637229eef383ffc9288bd821e4485e Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 17 Jul 2025 16:58:48 -0400 Subject: [PATCH 01/10] fix-18729 --- .../rules/unnecessary_future_import.rs | 39 +++++++++++++++++++ ...er__rules__pyupgrade__tests__UP010.py.snap | 22 +++++++++++ 2 files changed, 61 insertions(+) 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 index c713bdf718935..3ba77e0328ec6 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs @@ -7,6 +7,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::fix; use crate::{AlwaysFixableViolation, Applicability, Fix}; +use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport}; /// ## What it does /// Checks for unnecessary `__future__` imports. @@ -91,6 +92,44 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: & if alias.asname.is_some() { continue; } + let is_required = match stmt { + ruff_python_ast::Stmt::ImportFrom(ruff_python_ast::StmtImportFrom { + module, + level, + .. + }) => { + let name_import = NameImport::ImportFrom(MemberNameImport { + module: module.as_ref().map(std::string::ToString::to_string), + name: ruff_python_semantic::Alias { + name: alias.name.to_string(), + as_name: None, + }, + level: *level, + }); + checker + .settings() + .isort + .required_imports + .contains(&name_import) + } + ruff_python_ast::Stmt::Import(_) => { + let name_import = NameImport::Import(ModuleNameImport { + name: ruff_python_semantic::Alias { + name: alias.name.to_string(), + as_name: None, + }, + }); + checker + .settings() + .isort + .required_imports + .contains(&name_import) + } + _ => false, + }; + if is_required { + continue; + } if PY33_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str()) || PY37_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str()) { diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap index 4696822fe94e7..73fda83f38d08 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap @@ -167,6 +167,7 @@ UP010.py:13:5: UP010 [*] Unnecessary `__future__` import `generator_stop` for ta 13 |- from __future__ import generator_stop 14 13 | from __future__ import invalid_module, generators 15 14 | from __future__ import generators # comment +16 15 | UP010.py:14:5: UP010 [*] Unnecessary `__future__` import `generators` for target Python version | @@ -185,6 +186,8 @@ UP010.py:14:5: UP010 [*] Unnecessary `__future__` import `generators` for target 14 |- from __future__ import invalid_module, generators 14 |+ from __future__ import invalid_module 15 15 | from __future__ import generators # comment +16 16 | +17 17 | from __future__ import generator_stop # required by config UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target Python version | @@ -192,6 +195,8 @@ UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target 14 | from __future__ import invalid_module, generators 15 | from __future__ import generators # comment | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010 +16 | +17 | from __future__ import generator_stop # required by config | = help: Remove unnecessary `__future__` import @@ -200,3 +205,20 @@ UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target 13 13 | from __future__ import generator_stop 14 14 | from __future__ import invalid_module, generators 15 |- from __future__ import generators # comment +16 15 | +17 16 | from __future__ import generator_stop # required by config + +UP010.py:17:1: UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version + | +15 | from __future__ import generators # comment +16 | +17 | from __future__ import generator_stop # required by config + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010 + | + = help: Remove unnecessary `__future__` import + +ℹ Unsafe fix +14 14 | from __future__ import invalid_module, generators +15 15 | from __future__ import generators # comment +16 16 | +17 |-from __future__ import generator_stop # required by config From d2f40c9e7eeb472dc4f615a39f95b090fa78933e Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 17 Jul 2025 17:03:25 -0400 Subject: [PATCH 02/10] remove duplicate test --- ...er__rules__pyupgrade__tests__UP010.py.snap | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap index 73fda83f38d08..4696822fe94e7 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP010.py.snap @@ -167,7 +167,6 @@ UP010.py:13:5: UP010 [*] Unnecessary `__future__` import `generator_stop` for ta 13 |- from __future__ import generator_stop 14 13 | from __future__ import invalid_module, generators 15 14 | from __future__ import generators # comment -16 15 | UP010.py:14:5: UP010 [*] Unnecessary `__future__` import `generators` for target Python version | @@ -186,8 +185,6 @@ UP010.py:14:5: UP010 [*] Unnecessary `__future__` import `generators` for target 14 |- from __future__ import invalid_module, generators 14 |+ from __future__ import invalid_module 15 15 | from __future__ import generators # comment -16 16 | -17 17 | from __future__ import generator_stop # required by config UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target Python version | @@ -195,8 +192,6 @@ UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target 14 | from __future__ import invalid_module, generators 15 | from __future__ import generators # comment | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010 -16 | -17 | from __future__ import generator_stop # required by config | = help: Remove unnecessary `__future__` import @@ -205,20 +200,3 @@ UP010.py:15:5: UP010 [*] Unnecessary `__future__` import `generators` for target 13 13 | from __future__ import generator_stop 14 14 | from __future__ import invalid_module, generators 15 |- from __future__ import generators # comment -16 15 | -17 16 | from __future__ import generator_stop # required by config - -UP010.py:17:1: UP010 [*] Unnecessary `__future__` import `generator_stop` for target Python version - | -15 | from __future__ import generators # comment -16 | -17 | from __future__ import generator_stop # required by config - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010 - | - = help: Remove unnecessary `__future__` import - -ℹ Unsafe fix -14 14 | from __future__ import invalid_module, generators -15 15 | from __future__ import generators # comment -16 16 | -17 |-from __future__ import generator_stop # required by config From e8f7e8768c325b76b725ef38bcb2e449026962bd Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 28 Jul 2025 19:32:56 -0400 Subject: [PATCH 03/10] extract function --- .../pyupgrade/rules/deprecated_import.rs | 10 ++- .../rules/unnecessary_future_import.rs | 69 +++++++++---------- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs index 022db3909737c..bc10227e54eb1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs @@ -2,7 +2,7 @@ use itertools::Itertools; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::whitespace::indentation; -use ruff_python_ast::{Alias, StmtImportFrom}; +use ruff_python_ast::{Alias, Stmt, StmtImportFrom}; use ruff_python_codegen::Stylist; use ruff_python_parser::Tokens; use ruff_text_size::Ranged; @@ -10,6 +10,7 @@ use ruff_text_size::Ranged; use crate::Locator; use crate::checkers::ast::Checker; use crate::rules::pyupgrade::fixes; +use crate::rules::pyupgrade::rules::unnecessary_future_import::is_import_required_by_isort; use crate::{Edit, Fix, FixAvailability, Violation}; use ruff_python_ast::PythonVersion; @@ -719,6 +720,13 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport return; } + let stmt = Stmt::ImportFrom(import_from_stmt.clone()); + for alias in &import_from_stmt.names { + if is_import_required_by_isort(checker, &stmt, alias) { + return; + } + } + let fixer = ImportReplacer::new( import_from_stmt, module, 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 index 3ba77e0328ec6..56c9db8194462 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs @@ -85,6 +85,36 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ "generator_stop", ]; +pub(crate) fn is_import_required_by_isort(checker: &Checker, stmt: &Stmt, alias: &Alias) -> bool { + let name_import = match stmt { + ruff_python_ast::Stmt::ImportFrom(ruff_python_ast::StmtImportFrom { + module, + level, + .. + }) => NameImport::ImportFrom(MemberNameImport { + module: module.as_ref().map(std::string::ToString::to_string), + name: ruff_python_semantic::Alias { + name: alias.name.to_string(), + as_name: None, + }, + level: *level, + }), + ruff_python_ast::Stmt::Import(_) => NameImport::Import(ModuleNameImport { + name: ruff_python_semantic::Alias { + name: alias.name.to_string(), + as_name: None, + }, + }), + _ => return false, + }; + + checker + .settings() + .isort + .required_imports + .contains(&name_import) +} + /// UP010 pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: &[Alias]) { let mut unused_imports: Vec<&Alias> = vec![]; @@ -92,44 +122,11 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: & if alias.asname.is_some() { continue; } - let is_required = match stmt { - ruff_python_ast::Stmt::ImportFrom(ruff_python_ast::StmtImportFrom { - module, - level, - .. - }) => { - let name_import = NameImport::ImportFrom(MemberNameImport { - module: module.as_ref().map(std::string::ToString::to_string), - name: ruff_python_semantic::Alias { - name: alias.name.to_string(), - as_name: None, - }, - level: *level, - }); - checker - .settings() - .isort - .required_imports - .contains(&name_import) - } - ruff_python_ast::Stmt::Import(_) => { - let name_import = NameImport::Import(ModuleNameImport { - name: ruff_python_semantic::Alias { - name: alias.name.to_string(), - as_name: None, - }, - }); - checker - .settings() - .isort - .required_imports - .contains(&name_import) - } - _ => false, - }; - if is_required { + + if is_import_required_by_isort(checker, stmt, alias) { continue; } + if PY33_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str()) || PY37_PLUS_REMOVE_FUTURES.contains(&alias.name.as_str()) { From aa8ca16f503306921998028decd8999be974ec9c Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 30 Jul 2025 11:20:31 -0400 Subject: [PATCH 04/10] pass `StmtRef` to avoid cloning --- .../pyupgrade/rules/deprecated_import.rs | 5 ++- .../rules/unnecessary_future_import.rs | 34 ++++++++++--------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs index bc10227e54eb1..5f0009dc5ec3b 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs @@ -2,7 +2,7 @@ use itertools::Itertools; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::whitespace::indentation; -use ruff_python_ast::{Alias, Stmt, StmtImportFrom}; +use ruff_python_ast::{Alias, StmtImportFrom, StmtRef}; use ruff_python_codegen::Stylist; use ruff_python_parser::Tokens; use ruff_text_size::Ranged; @@ -720,9 +720,8 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport return; } - let stmt = Stmt::ImportFrom(import_from_stmt.clone()); for alias in &import_from_stmt.names { - if is_import_required_by_isort(checker, &stmt, alias) { + if is_import_required_by_isort(checker, StmtRef::ImportFrom(import_from_stmt), alias) { return; } } 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 index 56c9db8194462..de5c201b51204 100644 --- 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,5 +1,5 @@ use itertools::Itertools; -use ruff_python_ast::{Alias, Stmt}; +use ruff_python_ast::{Alias, Stmt, StmtRef}; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_text_size::Ranged; @@ -85,21 +85,23 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ "generator_stop", ]; -pub(crate) fn is_import_required_by_isort(checker: &Checker, stmt: &Stmt, alias: &Alias) -> bool { +pub(crate) fn is_import_required_by_isort( + checker: &Checker, + stmt: StmtRef<'_>, + alias: &Alias, +) -> bool { let name_import = match stmt { - ruff_python_ast::Stmt::ImportFrom(ruff_python_ast::StmtImportFrom { - module, - level, - .. - }) => NameImport::ImportFrom(MemberNameImport { - module: module.as_ref().map(std::string::ToString::to_string), - name: ruff_python_semantic::Alias { - name: alias.name.to_string(), - as_name: None, - }, - level: *level, - }), - ruff_python_ast::Stmt::Import(_) => NameImport::Import(ModuleNameImport { + StmtRef::ImportFrom(ruff_python_ast::StmtImportFrom { module, level, .. }) => { + NameImport::ImportFrom(MemberNameImport { + module: module.as_ref().map(std::string::ToString::to_string), + name: ruff_python_semantic::Alias { + name: alias.name.to_string(), + as_name: None, + }, + level: *level, + }) + } + StmtRef::Import(_) => NameImport::Import(ModuleNameImport { name: ruff_python_semantic::Alias { name: alias.name.to_string(), as_name: None, @@ -123,7 +125,7 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: & continue; } - if is_import_required_by_isort(checker, stmt, alias) { + if is_import_required_by_isort(checker, stmt.into(), alias) { continue; } From ff6072d7f43335d6103f712a0c8e14176811e48c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 30 Jul 2025 16:16:18 -0400 Subject: [PATCH 05/10] apply test patch --- crates/ruff_linter/src/rules/pyupgrade/mod.rs | 46 ++++++++++++++++++- crates/ruff_linter/src/test.rs | 2 +- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 3f64cb323fd4f..919958e48ec0a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -7,16 +7,18 @@ pub(crate) mod types; #[cfg(test)] mod tests { + use std::collections::BTreeSet; use std::path::Path; use anyhow::Result; use ruff_python_ast::PythonVersion; + use ruff_python_semantic::{MemberNameImport, NameImport}; use test_case::test_case; use crate::registry::Rule; - use crate::rules::pyupgrade; + use crate::rules::{isort, pyupgrade}; use crate::settings::types::PreviewMode; - use crate::test::test_path; + use crate::test::{test_path, test_snippet}; use crate::{assert_diagnostics, settings}; #[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))] @@ -294,4 +296,44 @@ mod tests { assert_diagnostics!(diagnostics); Ok(()) } + + #[test] + fn i002_conflict() { + let diagnostics = test_snippet( + "1", + &settings::LinterSettings { + isort: isort::settings::Settings { + required_imports: BTreeSet::from_iter([ + // https://github.com/astral-sh/ruff/issues/18729 + NameImport::ImportFrom(MemberNameImport::member( + "__future__".to_string(), + "generator_stop".to_string(), + )), + // https://github.com/astral-sh/ruff/issues/16802 + NameImport::ImportFrom(MemberNameImport::member( + "collections".to_string(), + "Sequence".to_string(), + )), + ]), + ..Default::default() + }, + ..settings::LinterSettings::for_rules([ + Rule::MissingRequiredImport, + Rule::UnnecessaryFutureImport, + Rule::DeprecatedImport, + ]) + }, + ); + assert_diagnostics!(diagnostics, @r" + :1:1: I002 [*] Missing required import: `from __future__ import generator_stop` + ℹ Safe fix + 1 |+from __future__ import generator_stop + 1 2 | 1 + + :1:1: I002 [*] Missing required import: `from collections import Sequence` + ℹ Safe fix + 1 |+from collections import Sequence + 1 2 | 1 + "); + } } diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index bab73eb86ec3f..90494ee72a306 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -380,7 +380,7 @@ macro_rules! assert_diagnostics { }}; ($value:expr, @$snapshot:literal) => {{ insta::with_settings!({ omit_expression => true }, { - insta::assert_snapshot!($crate::test::print_messages(&$value), $snapshot); + insta::assert_snapshot!($crate::test::print_messages(&$value), @$snapshot); }); }}; ($name:expr, $value:expr) => {{ From 99de9433cbfdabf586ffe6f28aa4d543c02c9073 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 30 Jul 2025 16:25:29 -0400 Subject: [PATCH 06/10] tidy up --- .../pyupgrade/rules/unnecessary_future_import.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) 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 index de5c201b51204..09de7b0f4f940 100644 --- 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,13 +1,13 @@ use itertools::Itertools; -use ruff_python_ast::{Alias, Stmt, StmtRef}; use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef}; +use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::fix; use crate::{AlwaysFixableViolation, Applicability, Fix}; -use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport}; /// ## What it does /// Checks for unnecessary `__future__` imports. @@ -85,13 +85,9 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ "generator_stop", ]; -pub(crate) fn is_import_required_by_isort( - checker: &Checker, - stmt: StmtRef<'_>, - alias: &Alias, -) -> bool { +pub(crate) fn is_import_required_by_isort(checker: &Checker, stmt: StmtRef, alias: &Alias) -> bool { let name_import = match stmt { - StmtRef::ImportFrom(ruff_python_ast::StmtImportFrom { module, level, .. }) => { + StmtRef::ImportFrom(ast::StmtImportFrom { module, level, .. }) => { NameImport::ImportFrom(MemberNameImport { module: module.as_ref().map(std::string::ToString::to_string), name: ruff_python_semantic::Alias { @@ -157,7 +153,7 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: & unused_imports .iter() .map(|alias| &alias.name) - .map(ruff_python_ast::Identifier::as_str), + .map(ast::Identifier::as_str), statement, parent, checker.locator(), From 8d72fe37dfba1cd70b00ada1e62181b3fb4f57c9 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 30 Jul 2025 20:00:50 -0400 Subject: [PATCH 07/10] use `continue` instead of return for isort required imports --- crates/ruff_linter/src/rules/pyupgrade/mod.rs | 58 +++++++++++++++++++ .../pyupgrade/rules/deprecated_import.rs | 13 ++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 919958e48ec0a..f986cac769a01 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -336,4 +336,62 @@ mod tests { 1 2 | 1 "); } + + #[test] + fn up035_partial_required_import() { + let diagnostics = test_snippet( + "from pipes import quote, Template", + &settings::LinterSettings { + isort: isort::settings::Settings { + required_imports: BTreeSet::from_iter([ + // https://github.com/astral-sh/ruff/issues/18729 + NameImport::ImportFrom(MemberNameImport::member( + "__future__".to_string(), + "generator_stop".to_string(), + )), + // https://github.com/astral-sh/ruff/issues/16802 + NameImport::ImportFrom(MemberNameImport::member( + "collections".to_string(), + "Sequence".to_string(), + )), + // Only bail out if _all_ the names in UP035 are required. `pipes.Template` + // isn't flagged by UP035, so requiring it shouldn't prevent `pipes.quote` + // from getting a diagnostic. + NameImport::ImportFrom(MemberNameImport::member( + "pipes".to_string(), + "Template".to_string(), + )), + ]), + ..Default::default() + }, + ..settings::LinterSettings::for_rules([ + Rule::MissingRequiredImport, + Rule::UnnecessaryFutureImport, + Rule::DeprecatedImport, + ]) + }, + ); + assert_diagnostics!(diagnostics, @r" + :1:1: UP035 [*] Import from `shlex` instead: `quote` + | + 1 | from pipes import quote, Template + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP035 + | + = help: Import from `shlex` + + ℹ Safe fix + 1 |-from pipes import quote, Template + 1 |+from shlex import quote + + :1:1: I002 [*] Missing required import: `from __future__ import generator_stop` + ℹ Safe fix + 1 |+from __future__ import generator_stop + 1 2 | from pipes import quote, Template + + :1:1: I002 [*] Missing required import: `from collections import Sequence` + ℹ Safe fix + 1 |+from collections import Sequence + 1 2 | from pipes import quote, Template + "); + } } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs index 5f0009dc5ec3b..f6e86cf3620db 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs @@ -411,6 +411,7 @@ struct ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, + skip_aliases: std::collections::HashSet<&'a str>, } impl<'a> ImportReplacer<'a> { @@ -421,6 +422,7 @@ impl<'a> ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, + skip_aliases: std::collections::HashSet<&'a str>, ) -> Self { Self { import_from_stmt, @@ -429,6 +431,7 @@ impl<'a> ImportReplacer<'a> { stylist, tokens, version, + skip_aliases, } } @@ -438,6 +441,9 @@ impl<'a> ImportReplacer<'a> { if self.module == "typing" { if self.version >= PythonVersion::PY39 { for member in &self.import_from_stmt.names { + if self.skip_aliases.contains(&member.name.as_str()) { + continue; + } if let Some(target) = TYPING_TO_RENAME_PY39.iter().find_map(|(name, target)| { if &member.name == *name { Some(*target) @@ -674,6 +680,9 @@ impl<'a> ImportReplacer<'a> { let mut matched_names = vec![]; let mut unmatched_names = vec![]; for name in &self.import_from_stmt.names { + if self.skip_aliases.contains(&name.name.as_str()) { + continue; + } if candidates.contains(&name.name.as_str()) { matched_names.push(name); } else { @@ -720,9 +729,10 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport return; } + let mut skip_aliases = std::collections::HashSet::new(); for alias in &import_from_stmt.names { if is_import_required_by_isort(checker, StmtRef::ImportFrom(import_from_stmt), alias) { - return; + skip_aliases.insert(alias.name.as_str()); } } @@ -733,6 +743,7 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport checker.stylist(), checker.tokens(), checker.target_version(), + skip_aliases, ); for (operation, fix) in fixer.without_renames() { From d9768bb21a9dbdb588ea4ec27deede340c37afd2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 31 Jul 2025 14:11:51 -0400 Subject: [PATCH 08/10] avoid collecting a HashSet, check imports when needed --- .../pyupgrade/rules/deprecated_import.rs | 34 ++++++++++--------- .../rules/unnecessary_future_import.rs | 22 ++++++++---- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs index f6e86cf3620db..c3428ef94514a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_import.rs @@ -14,6 +14,8 @@ use crate::rules::pyupgrade::rules::unnecessary_future_import::is_import_require use crate::{Edit, Fix, FixAvailability, Violation}; use ruff_python_ast::PythonVersion; +use super::RequiredImports; + /// An import was moved and renamed as part of a deprecation. /// For example, `typing.AbstractSet` was moved to `collections.abc.Set`. #[derive(Debug, PartialEq, Eq)] @@ -411,7 +413,7 @@ struct ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, - skip_aliases: std::collections::HashSet<&'a str>, + required_imports: &'a RequiredImports, } impl<'a> ImportReplacer<'a> { @@ -422,7 +424,7 @@ impl<'a> ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, - skip_aliases: std::collections::HashSet<&'a str>, + required_imports: &'a RequiredImports, ) -> Self { Self { import_from_stmt, @@ -431,7 +433,7 @@ impl<'a> ImportReplacer<'a> { stylist, tokens, version, - skip_aliases, + required_imports, } } @@ -441,7 +443,11 @@ impl<'a> ImportReplacer<'a> { if self.module == "typing" { if self.version >= PythonVersion::PY39 { for member in &self.import_from_stmt.names { - if self.skip_aliases.contains(&member.name.as_str()) { + if is_import_required_by_isort( + self.required_imports, + StmtRef::ImportFrom(self.import_from_stmt), + member, + ) { continue; } if let Some(target) = TYPING_TO_RENAME_PY39.iter().find_map(|(name, target)| { @@ -680,10 +686,13 @@ impl<'a> ImportReplacer<'a> { let mut matched_names = vec![]; let mut unmatched_names = vec![]; for name in &self.import_from_stmt.names { - if self.skip_aliases.contains(&name.name.as_str()) { - continue; - } - if candidates.contains(&name.name.as_str()) { + if is_import_required_by_isort( + self.required_imports, + StmtRef::ImportFrom(self.import_from_stmt), + name, + ) { + unmatched_names.push(name); + } else if candidates.contains(&name.name.as_str()) { matched_names.push(name); } else { unmatched_names.push(name); @@ -729,13 +738,6 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport return; } - let mut skip_aliases = std::collections::HashSet::new(); - for alias in &import_from_stmt.names { - if is_import_required_by_isort(checker, StmtRef::ImportFrom(import_from_stmt), alias) { - skip_aliases.insert(alias.name.as_str()); - } - } - let fixer = ImportReplacer::new( import_from_stmt, module, @@ -743,7 +745,7 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport checker.stylist(), checker.tokens(), checker.target_version(), - skip_aliases, + &checker.settings().isort.required_imports, ); for (operation, fix) in fixer.without_renames() { 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 index 09de7b0f4f940..a3076b73d015c 100644 --- 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,3 +1,5 @@ +use std::collections::BTreeSet; + use itertools::Itertools; use ruff_macros::{ViolationMetadata, derive_message_formats}; @@ -85,7 +87,13 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ "generator_stop", ]; -pub(crate) fn is_import_required_by_isort(checker: &Checker, stmt: StmtRef, alias: &Alias) -> bool { +pub(crate) type RequiredImports = BTreeSet; + +pub(crate) fn is_import_required_by_isort( + required_imports: &RequiredImports, + stmt: StmtRef, + alias: &Alias, +) -> bool { let name_import = match stmt { StmtRef::ImportFrom(ast::StmtImportFrom { module, level, .. }) => { NameImport::ImportFrom(MemberNameImport { @@ -106,11 +114,7 @@ pub(crate) fn is_import_required_by_isort(checker: &Checker, stmt: StmtRef, alia _ => return false, }; - checker - .settings() - .isort - .required_imports - .contains(&name_import) + required_imports.contains(&name_import) } /// UP010 @@ -121,7 +125,11 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: & continue; } - if is_import_required_by_isort(checker, stmt.into(), alias) { + if is_import_required_by_isort( + &checker.settings().isort.required_imports, + stmt.into(), + alias, + ) { continue; } From 199f49f36941e120da53d1781d7107fbea7c9b0c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 31 Jul 2025 14:12:13 -0400 Subject: [PATCH 09/10] combine tests, fix snapshot --- crates/ruff_linter/src/rules/pyupgrade/mod.rs | 43 +------------------ 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index f986cac769a01..65e0313f2d2cb 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -299,46 +299,6 @@ mod tests { #[test] fn i002_conflict() { - let diagnostics = test_snippet( - "1", - &settings::LinterSettings { - isort: isort::settings::Settings { - required_imports: BTreeSet::from_iter([ - // https://github.com/astral-sh/ruff/issues/18729 - NameImport::ImportFrom(MemberNameImport::member( - "__future__".to_string(), - "generator_stop".to_string(), - )), - // https://github.com/astral-sh/ruff/issues/16802 - NameImport::ImportFrom(MemberNameImport::member( - "collections".to_string(), - "Sequence".to_string(), - )), - ]), - ..Default::default() - }, - ..settings::LinterSettings::for_rules([ - Rule::MissingRequiredImport, - Rule::UnnecessaryFutureImport, - Rule::DeprecatedImport, - ]) - }, - ); - assert_diagnostics!(diagnostics, @r" - :1:1: I002 [*] Missing required import: `from __future__ import generator_stop` - ℹ Safe fix - 1 |+from __future__ import generator_stop - 1 2 | 1 - - :1:1: I002 [*] Missing required import: `from collections import Sequence` - ℹ Safe fix - 1 |+from collections import Sequence - 1 2 | 1 - "); - } - - #[test] - fn up035_partial_required_import() { let diagnostics = test_snippet( "from pipes import quote, Template", &settings::LinterSettings { @@ -381,7 +341,8 @@ mod tests { ℹ Safe fix 1 |-from pipes import quote, Template - 1 |+from shlex import quote + 1 |+from pipes import Template + 2 |+from shlex import quote :1:1: I002 [*] Missing required import: `from __future__ import generator_stop` ℹ Safe fix From 4378e8edb2851beb32ca40dbfb74bc09bdcd4eb0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 31 Jul 2025 14:34:15 -0400 Subject: [PATCH 10/10] simplify is_import_required_by_isort (?) --- .../rules/unnecessary_future_import.rs | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) 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 index a3076b73d015c..1809cc3e1629d 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_future_import.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef}; -use ruff_python_semantic::{MemberNameImport, ModuleNameImport, NameImport}; +use ruff_python_semantic::NameImport; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -94,27 +94,20 @@ pub(crate) fn is_import_required_by_isort( stmt: StmtRef, alias: &Alias, ) -> bool { - let name_import = match stmt { - StmtRef::ImportFrom(ast::StmtImportFrom { module, level, .. }) => { - NameImport::ImportFrom(MemberNameImport { - module: module.as_ref().map(std::string::ToString::to_string), - name: ruff_python_semantic::Alias { - name: alias.name.to_string(), - as_name: None, - }, - level: *level, - }) + let segments: &[&str] = match stmt { + StmtRef::ImportFrom(ast::StmtImportFrom { + module: Some(module), + .. + }) => &[module.as_str(), alias.name.as_str()], + StmtRef::ImportFrom(ast::StmtImportFrom { module: None, .. }) | StmtRef::Import(_) => { + &[alias.name.as_str()] } - StmtRef::Import(_) => NameImport::Import(ModuleNameImport { - name: ruff_python_semantic::Alias { - name: alias.name.to_string(), - as_name: None, - }, - }), _ => return false, }; - required_imports.contains(&name_import) + required_imports + .iter() + .any(|required_import| required_import.qualified_name().segments() == segments) } /// UP010