PyUpgrade: Replace pipes with `capture_output=True` (#1415)

This commit is contained in:
Colin Delahunty 2022-12-28 21:53:35 +00:00 committed by GitHub
parent dfa6fa8f83
commit 34842b4c4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 295 additions and 1 deletions

View File

@ -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)

View File

@ -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)

View File

@ -881,6 +881,7 @@
"UP02",
"UP020",
"UP021",
"UP022",
"UP023",
"W",
"W2",

View File

@ -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)

View File

@ -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(..)

View File

@ -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,

View File

@ -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());

View File

@ -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;

View File

@ -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<Item = char>) -> 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<MiddleContent> {
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);
}
}

View File

@ -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