From e8f78fa2cf7d6303e8d032ac5532fb626cfa4736 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 5 Sep 2023 13:44:17 +0200 Subject: [PATCH] Avoid fixing UP022 when `capture_output` is provided (#7149) --- .../test/fixtures/pyupgrade/UP022.py | 6 ++- .../pyupgrade/rules/replace_stdout_stderr.rs | 51 ++++++++----------- ...ff__rules__pyupgrade__tests__UP022.py.snap | 38 ++++++-------- 3 files changed, 42 insertions(+), 53 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP022.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP022.py index e93901bde1..01cf7e107d 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP022.py +++ b/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) diff --git a/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs b/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs index 11c3591144..67ddaa118f 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs +++ b/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 { - "Replace with `capture_output` keyword argument".to_string() + fn autofix_title(&self) -> Option { + 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 - .try_set_fix(|| generate_fix(stdout, stderr, call, checker.locator().contents())); + if call.arguments.find_keyword("capture_output").is_none() { + diagnostic.try_set_fix(|| { + generate_fix(stdout, stderr, call, checker.locator().contents()) + }); + } } checker.diagnostics.push(diagnostic); } @@ -99,28 +104,14 @@ 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()), - [remove_argument( - second, - &call.arguments, - Parentheses::Preserve, - source, - )?], - )) - } + // 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()), + [remove_argument( + second, + &call.arguments, + Parentheses::Preserve, + source, + )?], + )) } diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP022.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP022.py.snap index 2d7ed39efa..5d8ff0b364 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP022.py.snap +++ b/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 -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` +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 -40 40 | ) -41 41 | -42 42 | output = subprocess.run( -43 |- ["foo"], stdout=subprocess.PIPE, capture_output=False, stderr=subprocess.PIPE - 43 |+ ["foo"], capture_output=False -44 44 | ) -45 45 | -46 46 | # Examples that should NOT trigger the rule +UP022.py:46:10: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output` + | +44 | ) +45 | +46 | output = subprocess.run( + | __________^ +47 | | ["foo"], capture_output=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE +48 | | ) + | |_^ UP022 +49 | +50 | # OK + | + = help: Replace with `capture_output` keyword argument