diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 3f64cb323f..65e0313f2d 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,63 @@ mod tests { assert_diagnostics!(diagnostics); Ok(()) } + + #[test] + fn i002_conflict() { + 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 pipes import Template + 2 |+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 022db39097..c3428ef945 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, StmtImportFrom, StmtRef}; use ruff_python_codegen::Stylist; use ruff_python_parser::Tokens; use ruff_text_size::Ranged; @@ -10,9 +10,12 @@ 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; +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)] @@ -410,6 +413,7 @@ struct ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, + required_imports: &'a RequiredImports, } impl<'a> ImportReplacer<'a> { @@ -420,6 +424,7 @@ impl<'a> ImportReplacer<'a> { stylist: &'a Stylist<'a>, tokens: &'a Tokens, version: PythonVersion, + required_imports: &'a RequiredImports, ) -> Self { Self { import_from_stmt, @@ -428,6 +433,7 @@ impl<'a> ImportReplacer<'a> { stylist, tokens, version, + required_imports, } } @@ -437,6 +443,13 @@ impl<'a> ImportReplacer<'a> { if self.module == "typing" { if self.version >= PythonVersion::PY39 { for member in &self.import_from_stmt.names { + 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)| { if &member.name == *name { Some(*target) @@ -673,7 +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 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); @@ -726,6 +745,7 @@ pub(crate) fn deprecated_import(checker: &Checker, import_from_stmt: &StmtImport checker.stylist(), checker.tokens(), checker.target_version(), + &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 c713bdf718..1809cc3e16 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,7 +1,10 @@ +use std::collections::BTreeSet; + use itertools::Itertools; -use ruff_python_ast::{Alias, Stmt}; use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef}; +use ruff_python_semantic::NameImport; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -84,6 +87,29 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ "generator_stop", ]; +pub(crate) type RequiredImports = BTreeSet; + +pub(crate) fn is_import_required_by_isort( + required_imports: &RequiredImports, + stmt: StmtRef, + alias: &Alias, +) -> bool { + 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()] + } + _ => return false, + }; + + required_imports + .iter() + .any(|required_import| required_import.qualified_name().segments() == segments) +} + /// UP010 pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, names: &[Alias]) { let mut unused_imports: Vec<&Alias> = vec![]; @@ -91,6 +117,15 @@ pub(crate) fn unnecessary_future_import(checker: &Checker, stmt: &Stmt, 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()) { @@ -119,7 +154,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(), diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index cf63762b8a..02eca4ca05 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) => {{