[pyupgrade] Prevent infinite loop with I002 (UP010, UP035)

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"
+        <filename>: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
+
+        <filename>: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
+
+        <filename>:1:1: I002 [*] Missing required import: `from collections import Sequence`
+        ℹ Safe fix
+          1 |+from collections import Sequence
+        1 2 | from pipes import quote, Template
+        ");
+    }
 }

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

@@ -2,17 +2,20 @@ 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;
 
 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() {

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,13 +87,45 @@ const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[
     "generator_stop",
 ];
 
+pub(crate) type RequiredImports = BTreeSet<NameImport>;
+
+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![];
     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())
         {
@@ -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(),

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) => {{