diff --git a/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skip.py b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skip.py new file mode 100644 index 0000000000000..f2e4666538abf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skip.py @@ -0,0 +1,3 @@ +import concurrent.futures as futures + +1 diff --git a/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skips_required_import.py b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skips_required_import.py new file mode 100644 index 0000000000000..f2e4666538abf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/plr0402_skips_required_import.py @@ -0,0 +1,3 @@ +import concurrent.futures as futures + +1 diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index 92d5dfe38d557..923a2d71d29d4 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -991,6 +991,29 @@ mod tests { Ok(()) } + #[test_case(Path::new("plr0402_skip.py"))] + fn plr0402_skips_required_imports(path: &Path) -> Result<()> { + let snapshot = format!("plr0402_skips_required_imports_{}", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("isort/required_imports").join(path).as_path(), + &LinterSettings { + src: vec![test_resource_path("fixtures/isort")], + isort: super::settings::Settings { + required_imports: BTreeSet::from_iter([NameImport::Import( + ModuleNameImport::alias( + "concurrent.futures".to_string(), + "futures".to_string(), + ), + )]), + ..super::settings::Settings::default() + }, + ..LinterSettings::for_rules([Rule::MissingRequiredImport, Rule::ManualFromImport]) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Path::new("from_first.py"))] fn from_first(path: &Path) -> Result<()> { let snapshot = format!("from_first_{}", path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__plr0402_skips_required_imports_plr0402_skip.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__plr0402_skips_required_imports_plr0402_skip.py.snap new file mode 100644 index 0000000000000..ed369f0fd61f0 --- /dev/null +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__plr0402_skips_required_imports_plr0402_skip.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/isort/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pylint/rules/manual_import_from.rs b/crates/ruff_linter/src/rules/pylint/rules/manual_import_from.rs index c53c3c3ddd1c8..8de9c13f9e6d0 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/manual_import_from.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/manual_import_from.rs @@ -4,6 +4,7 @@ use ruff_text_size::{Ranged, TextRange}; use ruff_macros::{ViolationMetadata, derive_message_formats}; use crate::checkers::ast::Checker; +use crate::rules::pyupgrade::rules::is_import_required_by_isort; use crate::{Edit, Fix, FixAvailability, Violation}; /// ## What it does @@ -58,6 +59,15 @@ pub(crate) fn manual_from_import(checker: &Checker, stmt: &Stmt, alias: &Alias, return; } + // Skip if this import is required by isort to prevent infinite loops with I002 + if is_import_required_by_isort( + &checker.settings().isort.required_imports, + stmt.into(), + alias, + ) { + return; + } + let mut diagnostic = checker.report_diagnostic( ManualFromImport { module: module.to_string(), 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 51b8167172fc8..62eea80f2b536 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,6 +4,7 @@ use itertools::{Itertools, chain}; use ruff_python_semantic::NodeId; use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder}; use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef}; use ruff_python_semantic::{NameImport, Scope}; use ruff_text_size::Ranged; @@ -95,20 +96,35 @@ pub(crate) fn is_import_required_by_isort( stmt: StmtRef, alias: &Alias, ) -> bool { - let segments: &[&str] = match stmt { + 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()] + }) => { + let mut builder = QualifiedNameBuilder::with_capacity(module.split('.').count() + 1); + builder.extend(module.split('.')); + builder.push(alias.name.as_str()); + let qualified = builder.build(); + + required_imports + .iter() + .any(|required_import| required_import.qualified_name() == qualified) } - _ => return false, - }; + StmtRef::ImportFrom(ast::StmtImportFrom { module: None, .. }) + | StmtRef::Import(ast::StmtImport { .. }) => { + let name = alias.name.as_str(); + let qualified = if name.contains('.') { + QualifiedName::from_dotted_name(name) + } else { + QualifiedName::user_defined(name) + }; - required_imports - .iter() - .any(|required_import| required_import.qualified_name().segments() == segments) + required_imports + .iter() + .any(|required_import| required_import.qualified_name() == qualified) + } + _ => false, + } } /// UP010