diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP021.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP021.py index c6e0b5e59133e..04b837951cfbd 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP021.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP021.py @@ -1,12 +1,10 @@ import subprocess -import subprocess as somename from subprocess import run -from subprocess import run as anothername +# Errors subprocess.run(["foo"], universal_newlines=True, check=True) -somename.run(["foo"], universal_newlines=True) - +subprocess.run(["foo"], universal_newlines=True, text=True) run(["foo"], universal_newlines=True, check=False) -anothername(["foo"], universal_newlines=True) +# OK subprocess.run(["foo"], check=True) diff --git a/crates/ruff/src/rules/pyupgrade/rules/replace_universal_newlines.rs b/crates/ruff/src/rules/pyupgrade/rules/replace_universal_newlines.rs index 46b8f96e88e48..6f82e5940fec4 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/replace_universal_newlines.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/replace_universal_newlines.rs @@ -1,9 +1,9 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast}; +use ruff_python_ast as ast; use ruff_text_size::Ranged; -use ruff_text_size::{TextLen, TextRange}; +use crate::autofix::edits::{remove_argument, Parentheses}; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -58,13 +58,30 @@ pub(crate) fn replace_universal_newlines(checker: &mut Checker, call: &ast::Expr let Some(kwarg) = call.arguments.find_keyword("universal_newlines") else { return; }; - let range = TextRange::at(kwarg.start(), "universal_newlines".text_len()); - let mut diagnostic = Diagnostic::new(ReplaceUniversalNewlines, range); + + let Some(arg) = kwarg.arg.as_ref() else { + return; + }; + + let mut diagnostic = Diagnostic::new(ReplaceUniversalNewlines, arg.range()); + if checker.patch(diagnostic.kind.rule()) { - diagnostic.set_fix(Fix::suggested(Edit::range_replacement( - "text".to_string(), - range, - ))); + if call.arguments.find_keyword("text").is_some() { + diagnostic.try_set_fix(|| { + remove_argument( + kwarg, + &call.arguments, + Parentheses::Preserve, + checker.locator().contents(), + ) + .map(Fix::suggested) + }); + } else { + diagnostic.set_fix(Fix::suggested(Edit::range_replacement( + "text".to_string(), + arg.range(), + ))); + } } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP021.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP021.py.snap index 605f560b229bc..6e82b0a418821 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP021.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP021.py.snap @@ -1,83 +1,65 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP021.py:6:25: UP021 [*] `universal_newlines` is deprecated, use `text` +UP021.py:5:25: UP021 [*] `universal_newlines` is deprecated, use `text` | -4 | from subprocess import run as anothername -5 | -6 | subprocess.run(["foo"], universal_newlines=True, check=True) +4 | # Errors +5 | subprocess.run(["foo"], universal_newlines=True, check=True) | ^^^^^^^^^^^^^^^^^^ UP021 -7 | somename.run(["foo"], universal_newlines=True) +6 | subprocess.run(["foo"], universal_newlines=True, text=True) +7 | run(["foo"], universal_newlines=True, check=False) | = help: Replace with `text` keyword argument ℹ Suggested fix -3 3 | from subprocess import run -4 4 | from subprocess import run as anothername -5 5 | -6 |-subprocess.run(["foo"], universal_newlines=True, check=True) - 6 |+subprocess.run(["foo"], text=True, check=True) -7 7 | somename.run(["foo"], universal_newlines=True) +2 2 | from subprocess import run +3 3 | +4 4 | # Errors +5 |-subprocess.run(["foo"], universal_newlines=True, check=True) + 5 |+subprocess.run(["foo"], text=True, check=True) +6 6 | subprocess.run(["foo"], universal_newlines=True, text=True) +7 7 | run(["foo"], universal_newlines=True, check=False) 8 8 | -9 9 | run(["foo"], universal_newlines=True, check=False) -UP021.py:7:23: UP021 [*] `universal_newlines` is deprecated, use `text` +UP021.py:6:25: UP021 [*] `universal_newlines` is deprecated, use `text` | -6 | subprocess.run(["foo"], universal_newlines=True, check=True) -7 | somename.run(["foo"], universal_newlines=True) - | ^^^^^^^^^^^^^^^^^^ UP021 -8 | -9 | run(["foo"], universal_newlines=True, check=False) +4 | # Errors +5 | subprocess.run(["foo"], universal_newlines=True, check=True) +6 | subprocess.run(["foo"], universal_newlines=True, text=True) + | ^^^^^^^^^^^^^^^^^^ UP021 +7 | run(["foo"], universal_newlines=True, check=False) | = help: Replace with `text` keyword argument ℹ Suggested fix -4 4 | from subprocess import run as anothername -5 5 | -6 6 | subprocess.run(["foo"], universal_newlines=True, check=True) -7 |-somename.run(["foo"], universal_newlines=True) - 7 |+somename.run(["foo"], text=True) +3 3 | +4 4 | # Errors +5 5 | subprocess.run(["foo"], universal_newlines=True, check=True) +6 |-subprocess.run(["foo"], universal_newlines=True, text=True) + 6 |+subprocess.run(["foo"], text=True) +7 7 | run(["foo"], universal_newlines=True, check=False) 8 8 | -9 9 | run(["foo"], universal_newlines=True, check=False) -10 10 | anothername(["foo"], universal_newlines=True) - -UP021.py:9:14: UP021 [*] `universal_newlines` is deprecated, use `text` - | - 7 | somename.run(["foo"], universal_newlines=True) - 8 | - 9 | run(["foo"], universal_newlines=True, check=False) - | ^^^^^^^^^^^^^^^^^^ UP021 -10 | anothername(["foo"], universal_newlines=True) - | - = help: Replace with `text` keyword argument - -ℹ Suggested fix -6 6 | subprocess.run(["foo"], universal_newlines=True, check=True) -7 7 | somename.run(["foo"], universal_newlines=True) -8 8 | -9 |-run(["foo"], universal_newlines=True, check=False) - 9 |+run(["foo"], text=True, check=False) -10 10 | anothername(["foo"], universal_newlines=True) -11 11 | -12 12 | subprocess.run(["foo"], check=True) +9 9 | # OK -UP021.py:10:22: UP021 [*] `universal_newlines` is deprecated, use `text` - | - 9 | run(["foo"], universal_newlines=True, check=False) -10 | anothername(["foo"], universal_newlines=True) - | ^^^^^^^^^^^^^^^^^^ UP021 -11 | -12 | subprocess.run(["foo"], check=True) - | - = help: Replace with `text` keyword argument +UP021.py:7:14: UP021 [*] `universal_newlines` is deprecated, use `text` + | +5 | subprocess.run(["foo"], universal_newlines=True, check=True) +6 | subprocess.run(["foo"], universal_newlines=True, text=True) +7 | run(["foo"], universal_newlines=True, check=False) + | ^^^^^^^^^^^^^^^^^^ UP021 +8 | +9 | # OK + | + = help: Replace with `text` keyword argument ℹ Suggested fix -7 7 | somename.run(["foo"], universal_newlines=True) -8 8 | -9 9 | run(["foo"], universal_newlines=True, check=False) -10 |-anothername(["foo"], universal_newlines=True) - 10 |+anothername(["foo"], text=True) -11 11 | -12 12 | subprocess.run(["foo"], check=True) +4 4 | # Errors +5 5 | subprocess.run(["foo"], universal_newlines=True, check=True) +6 6 | subprocess.run(["foo"], universal_newlines=True, text=True) +7 |-run(["foo"], universal_newlines=True, check=False) + 7 |+run(["foo"], text=True, check=False) +8 8 | +9 9 | # OK +10 10 | subprocess.run(["foo"], check=True)