better comma handling

astral-sh · Aug 20, 2023 · eaa6274 · eaa6274
1 parent 51cca27
commit eaa6274
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 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
@@ -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/...pylint/snapshots/ruff__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap b/...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,17 +18,17 @@ 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
   |
 3 | # Errors.
 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)