Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid fixing UP022 when capture_output is provided #7149

Merged
merged 1 commit into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion crates/ruff/resources/test/fixtures/pyupgrade/UP022.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@
["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)
51 changes: 21 additions & 30 deletions crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<String> {
Some("Replace with `capture_output` keyword argument".to_string())
}
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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,
)?],
))
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand All @@ -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