diff --git a/README.md b/README.md index e7e1463e70..961e64fb20 100644 --- a/README.md +++ b/README.md @@ -671,6 +671,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | UP019 | TypingTextStrAlias | `typing.Text` is deprecated, use `str` | 🛠 | | UP020 | OpenAlias | Use builtin `open` | 🛠 | | UP021 | ReplaceUniversalNewlines | `universal_newlines` is deprecated, use `text` | 🛠 | +| UP022 | ReplaceStdoutStderr | Sending stdout and stderr to pipe is deprecated, use `capture_output` | 🛠 | | UP023 | RewriteCElementTree | `cElementTree` is deprecated, use `ElementTree` | 🛠 | ### pep8-naming (N) diff --git a/resources/test/fixtures/pyupgrade/UP022.py b/resources/test/fixtures/pyupgrade/UP022.py new file mode 100644 index 0000000000..c5ccb93107 --- /dev/null +++ b/resources/test/fixtures/pyupgrade/UP022.py @@ -0,0 +1,42 @@ +from subprocess import run +import subprocess + +output = run(["foo"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + +output = subprocess.run(["foo"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + +output = subprocess.run(stdout=subprocess.PIPE, args=["foo"], stderr=subprocess.PIPE) + +output = subprocess.run( + ["foo"], stdout=subprocess.PIPE, check=True, stderr=subprocess.PIPE +) + +output = subprocess.run( + ["foo"], stderr=subprocess.PIPE, check=True, stdout=subprocess.PIPE +) + +output = subprocess.run( + ["foo"], + stdout=subprocess.PIPE, + check=True, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + close_fds=True, +) + +if output: + output = subprocess.run( + ["foo"], + stdout=subprocess.PIPE, + check=True, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + ) + + +# Examples that should NOT trigger the rule +from foo import PIPE +subprocess.run(["foo"], stdout=PIPE, stderr=PIPE) +run(["foo"], stdout=None, stderr=PIPE) diff --git a/ruff.schema.json b/ruff.schema.json index 8cba51a5eb..178a63ec0d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -881,6 +881,7 @@ "UP02", "UP020", "UP021", + "UP022", "UP023", "W", "W2", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 0b22774a59..2dd0ebac42 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1679,6 +1679,9 @@ where if self.settings.enabled.contains(&CheckCode::UP021) { pyupgrade::plugins::replace_universal_newlines(self, expr, keywords); } + if self.settings.enabled.contains(&CheckCode::UP022) { + pyupgrade::plugins::replace_stdout_stderr(self, expr, keywords); + } // flake8-print if self.settings.enabled.contains(&CheckCode::T201) diff --git a/src/checks.rs b/src/checks.rs index 7ff1af0e28..a3a126e370 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -229,6 +229,7 @@ pub enum CheckCode { UP019, UP020, UP021, + UP022, UP023, // pydocstyle D100, @@ -845,6 +846,7 @@ pub enum CheckKind { NativeLiterals, OpenAlias, ReplaceUniversalNewlines, + ReplaceStdoutStderr, RewriteCElementTree, // pydocstyle BlankLineAfterLastSection(String), @@ -1225,6 +1227,7 @@ impl CheckCode { CheckCode::UP019 => CheckKind::TypingTextStrAlias, CheckCode::UP020 => CheckKind::OpenAlias, CheckCode::UP021 => CheckKind::ReplaceUniversalNewlines, + CheckCode::UP022 => CheckKind::ReplaceStdoutStderr, CheckCode::UP023 => CheckKind::RewriteCElementTree, // pydocstyle CheckCode::D100 => CheckKind::PublicModule, @@ -1650,6 +1653,7 @@ impl CheckCode { CheckCode::UP019 => CheckCategory::Pyupgrade, CheckCode::UP020 => CheckCategory::Pyupgrade, CheckCode::UP021 => CheckCategory::Pyupgrade, + CheckCode::UP022 => CheckCategory::Pyupgrade, CheckCode::UP023 => CheckCategory::Pyupgrade, CheckCode::W292 => CheckCategory::Pycodestyle, CheckCode::W605 => CheckCategory::Pycodestyle, @@ -1866,6 +1870,7 @@ impl CheckKind { CheckKind::TypingTextStrAlias => &CheckCode::UP019, CheckKind::OpenAlias => &CheckCode::UP020, CheckKind::ReplaceUniversalNewlines => &CheckCode::UP021, + CheckKind::ReplaceStdoutStderr => &CheckCode::UP022, CheckKind::RewriteCElementTree => &CheckCode::UP023, // pydocstyle CheckKind::BlankLineAfterLastSection(..) => &CheckCode::D413, @@ -2600,6 +2605,9 @@ impl CheckKind { CheckKind::ReplaceUniversalNewlines => { "`universal_newlines` is deprecated, use `text`".to_string() } + CheckKind::ReplaceStdoutStderr => { + "Sending stdout and stderr to pipe is deprecated, use `capture_output`".to_string() + } CheckKind::RewriteCElementTree => { "`cElementTree` is deprecated, use `ElementTree`".to_string() } @@ -3048,6 +3056,7 @@ impl CheckKind { | CheckKind::OpenAlias | CheckKind::NewLineAfterLastParagraph | CheckKind::ReplaceUniversalNewlines + | CheckKind::ReplaceStdoutStderr | CheckKind::RewriteCElementTree | CheckKind::NewLineAfterSectionName(..) | CheckKind::NoBlankLineAfterFunction(..) diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 3f6b04c130..c00970f9ad 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -532,6 +532,7 @@ pub enum CheckCodePrefix { UP02, UP020, UP021, + UP022, UP023, W, W2, @@ -760,6 +761,7 @@ impl CheckCodePrefix { CheckCode::UP019, CheckCode::UP020, CheckCode::UP021, + CheckCode::UP022, CheckCode::UP023, CheckCode::D100, CheckCode::D101, @@ -2417,6 +2419,7 @@ impl CheckCodePrefix { CheckCode::UP019, CheckCode::UP020, CheckCode::UP021, + CheckCode::UP022, CheckCode::UP023, ] } @@ -2448,6 +2451,7 @@ impl CheckCodePrefix { CheckCode::UP019, CheckCode::UP020, CheckCode::UP021, + CheckCode::UP022, CheckCode::UP023, ] } @@ -2663,6 +2667,7 @@ impl CheckCodePrefix { CheckCode::UP019, CheckCode::UP020, CheckCode::UP021, + CheckCode::UP022, CheckCode::UP023, ], CheckCodePrefix::UP0 => vec![ @@ -2686,6 +2691,7 @@ impl CheckCodePrefix { CheckCode::UP019, CheckCode::UP020, CheckCode::UP021, + CheckCode::UP022, CheckCode::UP023, ], CheckCodePrefix::UP00 => vec![ @@ -2728,9 +2734,15 @@ impl CheckCodePrefix { CheckCodePrefix::UP017 => vec![CheckCode::UP017], CheckCodePrefix::UP018 => vec![CheckCode::UP018], CheckCodePrefix::UP019 => vec![CheckCode::UP019], - CheckCodePrefix::UP02 => vec![CheckCode::UP020, CheckCode::UP021, CheckCode::UP023], + CheckCodePrefix::UP02 => vec![ + CheckCode::UP020, + CheckCode::UP021, + CheckCode::UP022, + CheckCode::UP023, + ], CheckCodePrefix::UP020 => vec![CheckCode::UP020], CheckCodePrefix::UP021 => vec![CheckCode::UP021], + CheckCodePrefix::UP022 => vec![CheckCode::UP022], CheckCodePrefix::UP023 => vec![CheckCode::UP023], CheckCodePrefix::W => vec![CheckCode::W292, CheckCode::W605], CheckCodePrefix::W2 => vec![CheckCode::W292], @@ -3295,6 +3307,7 @@ impl CheckCodePrefix { CheckCodePrefix::UP02 => SuffixLength::Two, CheckCodePrefix::UP020 => SuffixLength::Three, CheckCodePrefix::UP021 => SuffixLength::Three, + CheckCodePrefix::UP022 => SuffixLength::Three, CheckCodePrefix::UP023 => SuffixLength::Three, CheckCodePrefix::W => SuffixLength::Zero, CheckCodePrefix::W2 => SuffixLength::One, diff --git a/src/pyupgrade/mod.rs b/src/pyupgrade/mod.rs index 191eca820c..672114712a 100644 --- a/src/pyupgrade/mod.rs +++ b/src/pyupgrade/mod.rs @@ -40,6 +40,7 @@ mod tests { #[test_case(CheckCode::UP018, Path::new("UP018.py"); "UP018")] #[test_case(CheckCode::UP019, Path::new("UP019.py"); "UP019")] #[test_case(CheckCode::UP021, Path::new("UP021.py"); "UP021")] + #[test_case(CheckCode::UP022, Path::new("UP022.py"); "UP022")] #[test_case(CheckCode::UP023, Path::new("UP023.py"); "UP023")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); diff --git a/src/pyupgrade/plugins/mod.rs b/src/pyupgrade/plugins/mod.rs index 843496ddd5..151b4599df 100644 --- a/src/pyupgrade/plugins/mod.rs +++ b/src/pyupgrade/plugins/mod.rs @@ -6,6 +6,7 @@ pub use native_literals::native_literals; pub use open_alias::open_alias; pub use redundant_open_modes::redundant_open_modes; pub use remove_six_compat::remove_six_compat; +pub use replace_stdout_stderr::replace_stdout_stderr; pub use replace_universal_newlines::replace_universal_newlines; pub use rewrite_c_element_tree::replace_c_element_tree; pub use super_call_with_parameters::super_call_with_parameters; @@ -27,6 +28,7 @@ mod native_literals; mod open_alias; mod redundant_open_modes; mod remove_six_compat; +mod replace_stdout_stderr; mod replace_universal_newlines; mod rewrite_c_element_tree; mod super_call_with_parameters; diff --git a/src/pyupgrade/plugins/replace_stdout_stderr.rs b/src/pyupgrade/plugins/replace_stdout_stderr.rs new file mode 100644 index 0000000000..7dd51b3470 --- /dev/null +++ b/src/pyupgrade/plugins/replace_stdout_stderr.rs @@ -0,0 +1,112 @@ +use rustpython_ast::{Expr, Keyword}; + +use crate::ast::helpers::{find_keyword, match_module_member}; +use crate::ast::types::Range; +use crate::ast::whitespace::indentation; +use crate::autofix::Fix; +use crate::checkers::ast::Checker; +use crate::checks::{Check, CheckKind}; + +#[derive(Debug)] +struct MiddleContent<'a> { + contents: &'a str, + multi_line: bool, +} + +/// Return the number of "dirty" characters. +fn dirty_count(iter: impl Iterator) -> usize { + let mut the_count = 0; + for current_char in iter { + if current_char == ' ' || current_char == ',' || current_char == '\n' { + the_count += 1; + } else { + break; + } + } + the_count +} + +/// Extract the `Middle` content between two arguments. +fn extract_middle(contents: &str) -> Option { + let multi_line = contents.contains('\n'); + let start_gap = dirty_count(contents.chars()); + if contents.len() == start_gap { + return None; + } + let end_gap = dirty_count(contents.chars().rev()); + Some(MiddleContent { + contents: &contents[start_gap..contents.len() - end_gap], + multi_line, + }) +} + +/// UP022 +pub fn replace_stdout_stderr(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) { + if match_module_member( + expr, + "subprocess", + "run", + &checker.from_imports, + &checker.import_aliases, + ) { + // Find `stdout` and `stderr` kwargs. + let Some(stdout) = find_keyword(kwargs, "stdout") else { + return; + }; + let Some(stderr) = find_keyword(kwargs, "stderr") else { + return; + }; + + // Verify that they're both set to `subprocess.PIPE`. + if !match_module_member( + &stdout.node.value, + "subprocess", + "PIPE", + &checker.from_imports, + &checker.import_aliases, + ) || !match_module_member( + &stderr.node.value, + "subprocess", + "PIPE", + &checker.from_imports, + &checker.import_aliases, + ) { + return; + } + + let mut check = Check::new(CheckKind::ReplaceStdoutStderr, Range::from_located(expr)); + if checker.patch(check.kind.code()) { + let first = if stdout.location < stderr.location { + stdout + } else { + stderr + }; + let last = if stdout.location > stderr.location { + stdout + } else { + stderr + }; + let mut contents = String::from("capture_output=True"); + if let Some(middle) = extract_middle(&checker.locator.slice_source_code_range(&Range { + location: first.end_location.unwrap(), + end_location: last.location, + })) { + if middle.multi_line { + contents.push(','); + contents.push('\n'); + contents.push_str(&indentation(checker, first)); + } else { + contents.push(','); + contents.push(' '); + } + contents.push_str(middle.contents); + } + check.amend(Fix::replacement( + contents, + first.location, + last.end_location.unwrap(), + )); + } + checker.add_check(check); + } +} diff --git a/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP022_UP022.py.snap b/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP022_UP022.py.snap new file mode 100644 index 0000000000..b9d6bcf599 --- /dev/null +++ b/src/pyupgrade/snapshots/ruff__pyupgrade__tests__UP022_UP022.py.snap @@ -0,0 +1,110 @@ +--- +source: src/pyupgrade/mod.rs +expression: checks +--- +- kind: ReplaceStdoutStderr + location: + row: 4 + column: 9 + end_location: + row: 4 + column: 69 + fix: + content: capture_output=True + location: + row: 4 + column: 22 + end_location: + row: 4 + column: 68 +- kind: ReplaceStdoutStderr + location: + row: 6 + column: 9 + end_location: + row: 6 + column: 80 + fix: + content: capture_output=True + location: + row: 6 + column: 33 + end_location: + row: 6 + column: 79 +- kind: ReplaceStdoutStderr + location: + row: 8 + column: 9 + end_location: + row: 8 + column: 86 + fix: + content: "capture_output=True, args=[\"foo\"]" + location: + row: 8 + column: 24 + end_location: + row: 8 + column: 85 +- kind: ReplaceStdoutStderr + location: + row: 10 + column: 9 + end_location: + row: 12 + column: 1 + fix: + content: "capture_output=True, check=True" + location: + row: 11 + column: 13 + end_location: + row: 11 + column: 71 +- kind: ReplaceStdoutStderr + location: + row: 14 + column: 9 + end_location: + row: 16 + column: 1 + fix: + content: "capture_output=True, check=True" + location: + row: 15 + column: 13 + end_location: + row: 15 + column: 71 +- kind: ReplaceStdoutStderr + location: + row: 18 + column: 9 + end_location: + row: 26 + column: 1 + fix: + content: "capture_output=True,\n check=True" + location: + row: 20 + column: 4 + end_location: + row: 22 + column: 26 +- kind: ReplaceStdoutStderr + location: + row: 29 + column: 13 + end_location: + row: 36 + column: 5 + fix: + content: "capture_output=True,\n check=True" + location: + row: 31 + column: 8 + end_location: + row: 33 + column: 30 +