From eaa6274831b6803ee719143dfa3d7e114c786e84 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sun, 20 Aug 2023 13:19:16 -0700 Subject: [PATCH] better comma handling --- .../rules/subprocess_run_without_check.rs | 4 ++- ...W1510_subprocess_run_without_check.py.snap | 36 +++++++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/crates/ruff/src/rules/pylint/rules/subprocess_run_without_check.rs b/crates/ruff/src/rules/pylint/rules/subprocess_run_without_check.rs index 15eab904269eb7..e745b2bd28014b 100644 --- a/crates/ruff/src/rules/pylint/rules/subprocess_run_without_check.rs +++ b/crates/ruff/src/rules/pylint/rules/subprocess_run_without_check.rs @@ -63,8 +63,10 @@ pub(crate) fn subprocess_run_without_check(checker: &mut Checker, call: &ast::Ex { if call.arguments.find_keyword("check").is_none() { let mut diagnostic = Diagnostic::new(SubprocessRunWithoutCheck, call.func.range()); + let text: &str = checker.locator().slice(call.range()); + let ends_with_comma = text[..text.len() - 1].trim_end().ends_with(','); diagnostic.set_fix(Fix::automatic(Edit::insertion( - ", check=False".to_string(), + {if ends_with_comma {"check=False"} else {", check=False"}}.to_string(), call.range().end() - TextSize::from(1), ))); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap index 22a6b4132961de..cf4129380c99ec 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap @@ -7,6 +7,7 @@ subprocess_run_without_check.py:4:1: PLW1510 [*] `subprocess.run` without explic 4 | subprocess.run("ls") | ^^^^^^^^^^^^^^ PLW1510 5 | subprocess.run("ls", shell=True) +6 | subprocess.run( | = help: Add an explicit check=False @@ -17,8 +18,8 @@ subprocess_run_without_check.py:4:1: PLW1510 [*] `subprocess.run` without explic 4 |-subprocess.run("ls") 4 |+subprocess.run("ls", check=False) 5 5 | subprocess.run("ls", shell=True) -6 6 | -7 7 | # Non-errors. +6 6 | subprocess.run( +7 7 | ["ls"], subprocess_run_without_check.py:5:1: PLW1510 [*] `subprocess.run` without explicit `check` argument | @@ -26,8 +27,8 @@ subprocess_run_without_check.py:5:1: PLW1510 [*] `subprocess.run` without explic 4 | subprocess.run("ls") 5 | subprocess.run("ls", shell=True) | ^^^^^^^^^^^^^^ PLW1510 -6 | -7 | # Non-errors. +6 | subprocess.run( +7 | ["ls"], | = help: Add an explicit check=False @@ -37,8 +38,29 @@ subprocess_run_without_check.py:5:1: PLW1510 [*] `subprocess.run` without explic 4 4 | subprocess.run("ls") 5 |-subprocess.run("ls", shell=True) 5 |+subprocess.run("ls", shell=True, check=False) -6 6 | -7 7 | # Non-errors. -8 8 | subprocess.run("ls", check=True) +6 6 | subprocess.run( +7 7 | ["ls"], +8 8 | shell=False, + +subprocess_run_without_check.py:6:1: PLW1510 [*] `subprocess.run` without explicit `check` argument + | +4 | subprocess.run("ls") +5 | subprocess.run("ls", shell=True) +6 | subprocess.run( + | ^^^^^^^^^^^^^^ PLW1510 +7 | ["ls"], +8 | shell=False, + | + = help: Add an explicit check=False + +ℹ Fix +6 6 | subprocess.run( +7 7 | ["ls"], +8 8 | shell=False, +9 |-) + 9 |+check=False) +10 10 | +11 11 | # Non-errors. +12 12 | subprocess.run("ls", check=True)