Avoid fixing UP022 when capture_output is provided (#7149)

crates/ruff/resources/test/fixtures/pyupgrade/UP022.py

@@ -43,7 +43,11 @@ output = subprocess.run(
     ["foo"], stdout=subprocess.PIPE, capture_output=False, stderr=subprocess.PIPE
 )
 
-# Examples that should NOT trigger the rule
+output = subprocess.run(
+    ["foo"], capture_output=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE
+)
+
+# OK
 from foo import PIPE
 subprocess.run(["foo"], stdout=PIPE, stderr=PIPE)
 run(["foo"], stdout=None, stderr=PIPE)

crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs

@@ -1,6 +1,6 @@
 use anyhow::Result;
 
-use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{self as ast, Keyword};
 use ruff_text_size::Ranged;
@@ -39,14 +39,16 @@ use crate::registry::AsRule;
 #[violation]
 pub struct ReplaceStdoutStderr;
 
-impl AlwaysAutofixableViolation for ReplaceStdoutStderr {
+impl Violation for ReplaceStdoutStderr {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`")
     }
 
-    fn autofix_title(&self) -> String {
+    fn autofix_title(&self) -> Option<String> {
-        "Replace with `capture_output` keyword argument".to_string()
+        Some("Replace with `capture_output` keyword argument".to_string())
     }
 }
 
@@ -80,8 +82,11 @@ pub(crate) fn replace_stdout_stderr(checker: &mut Checker, call: &ast::ExprCall)
 
         let mut diagnostic = Diagnostic::new(ReplaceStdoutStderr, call.range());
         if checker.patch(diagnostic.kind.rule()) {
-            diagnostic
+            if call.arguments.find_keyword("capture_output").is_none() {
-                .try_set_fix(|| generate_fix(stdout, stderr, call, checker.locator().contents()));
+                diagnostic.try_set_fix(|| {
+                    generate_fix(stdout, stderr, call, checker.locator().contents())
+                });
+            }
         }
         checker.diagnostics.push(diagnostic);
     }
@@ -99,19 +104,6 @@ fn generate_fix(
     } else {
         (stderr, stdout)
     };
-
-    if call.arguments.find_keyword("capture_output").is_some() {
-        // Remove both arguments.
-        Ok(Fix::suggested_edits(
-            remove_argument(first, &call.arguments, Parentheses::Preserve, source)?,
-            [remove_argument(
-                second,
-                &call.arguments,
-                Parentheses::Preserve,
-                source,
-            )?],
-        ))
-    } else {
     // Replace one argument with `capture_output=True`, and remove the other.
     Ok(Fix::suggested_edits(
         Edit::range_replacement("capture_output=True".to_string(), first.range()),
@@ -123,4 +115,3 @@ fn generate_fix(
         )?],
     ))
 }
-}

crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP022.py.snap

@@ -174,7 +174,7 @@ UP022.py:29:14: UP022 [*] Sending `stdout` and `stderr` to `PIPE` is deprecated,
 35 34 |         encoding="utf-8",
 36 35 |     )
 
-UP022.py:38:10: UP022 [*] Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`
+UP022.py:38:10: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`
    |
 36 |       )
 37 |   
@@ -188,17 +188,7 @@ UP022.py:38:10: UP022 [*] Sending `stdout` and `stderr` to `PIPE` is deprecated,
    |
    = help: Replace with `capture_output` keyword argument
 
-ℹ Suggested fix
+UP022.py:42:10: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`
-36 36 |     )
-37 37 | 
-38 38 | output = subprocess.run(
-39    |-    ["foo"], stdout=subprocess.PIPE, capture_output=True, stderr=subprocess.PIPE
-   39 |+    ["foo"], capture_output=True
-40 40 | )
-41 41 | 
-42 42 | output = subprocess.run(
-
-UP022.py:42:10: UP022 [*] Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`
    |
 40 |   )
 41 |   
@@ -208,18 +198,22 @@ UP022.py:42:10: UP022 [*] Sending `stdout` and `stderr` to `PIPE` is deprecated,
 44 | | )
    | |_^ UP022
 45 |   
-46 |   # Examples that should NOT trigger the rule
+46 |   output = subprocess.run(
    |
    = help: Replace with `capture_output` keyword argument
 
-ℹ Suggested fix
+UP022.py:46:10: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`
-40 40 | )
+   |
-41 41 | 
+44 |   )
-42 42 | output = subprocess.run(
+45 |   
-43    |-    ["foo"], stdout=subprocess.PIPE, capture_output=False, stderr=subprocess.PIPE
+46 |   output = subprocess.run(
-   43 |+    ["foo"], capture_output=False
+   |  __________^
-44 44 | )
+47 | |     ["foo"], capture_output=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE
-45 45 | 
+48 | | )
-46 46 | # Examples that should NOT trigger the rule
+   | |_^ UP022
+49 |   
+50 |   # OK
+   |
+   = help: Replace with `capture_output` keyword argument