[pylint] Add fix for subprocess-run-without-check (PLW1510) (#6708)

crates/ruff_linter/resources/test/fixtures/pylint/subprocess_run_without_check.py

@@ -3,6 +3,11 @@ import subprocess
 # Errors.
 subprocess.run("ls")
 subprocess.run("ls", shell=True)
+subprocess.run(
+    ["ls"],
+    shell=False,
+)
+subprocess.run(["ls"], **kwargs)
 
 # Non-errors.
 subprocess.run("ls", check=True)

crates/ruff_linter/src/rules/pylint/rules/subprocess_run_without_check.rs

@@ -1,9 +1,10 @@
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
+use crate::fix::edits::add_argument;
 
 /// ## What it does
 /// Checks for uses of `subprocess.run` without an explicit `check` argument.
@@ -36,16 +37,25 @@ use crate::checkers::ast::Checker;
 /// subprocess.run(["ls", "nonexistent"], check=False)  # Explicitly no check.
 /// ```
 ///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe for function calls that contain
+/// `**kwargs`, as adding a `check` keyword argument to such a call may lead
+/// to a duplicate keyword argument error.
+///
 /// ## References
 /// - [Python documentation: `subprocess.run`](https://docs.python.org/3/library/subprocess.html#subprocess.run)
 #[violation]
 pub struct SubprocessRunWithoutCheck;
 
-impl Violation for SubprocessRunWithoutCheck {
+impl AlwaysFixableViolation for SubprocessRunWithoutCheck {
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("`subprocess.run` without explicit `check` argument")
     }
+
+    fn fix_title(&self) -> String {
+        "Add explicit `check=False`".to_string()
+    }
 }
 
 /// PLW1510
@@ -56,10 +66,27 @@ pub(crate) fn subprocess_run_without_check(checker: &mut Checker, call: &ast::Ex
         .is_some_and(|call_path| matches!(call_path.as_slice(), ["subprocess", "run"]))
     {
         if call.arguments.find_keyword("check").is_none() {
-            checker.diagnostics.push(Diagnostic::new(
+            let mut diagnostic = Diagnostic::new(SubprocessRunWithoutCheck, call.func.range());
-                SubprocessRunWithoutCheck,
+            diagnostic.set_fix(Fix::applicable_edit(
-                call.func.range(),
+                add_argument(
+                    "check=False",
+                    &call.arguments,
+                    checker.indexer().comment_ranges(),
+                    checker.locator().contents(),
+                ),
+                // If the function call contains `**kwargs`, mark the fix as unsafe.
+                if call
+                    .arguments
+                    .keywords
+                    .iter()
+                    .any(|keyword| keyword.arg.is_none())
+                {
+                    Applicability::Unsafe
+                } else {
+                    Applicability::Safe
+                },
             ));
+            checker.diagnostics.push(diagnostic);
         }
     }
 }

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap

@@ -1,22 +1,87 @@
 ---
 source: crates/ruff_linter/src/rules/pylint/mod.rs
 ---
-subprocess_run_without_check.py:4:1: PLW1510 `subprocess.run` without explicit `check` argument
+subprocess_run_without_check.py:4:1: PLW1510 [*] `subprocess.run` without explicit `check` argument
   |
 3 | # Errors.
 4 | subprocess.run("ls")
   | ^^^^^^^^^^^^^^ PLW1510
 5 | subprocess.run("ls", shell=True)
+6 | subprocess.run(
   |
+  = help: Add explicit `check=False`
 
-subprocess_run_without_check.py:5:1: PLW1510 `subprocess.run` without explicit `check` argument
+ℹ Safe fix
+1 1 | import subprocess
+2 2 | 
+3 3 | # Errors.
+4   |-subprocess.run("ls")
+  4 |+subprocess.run("ls", check=False)
+5 5 | subprocess.run("ls", shell=True)
+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 | 
+6 | subprocess.run(
-7 | # Non-errors.
+7 |     ["ls"],
   |
+  = help: Add explicit `check=False`
+
+ℹ Safe fix
+2 2 | 
+3 3 | # Errors.
+4 4 | subprocess.run("ls")
+5   |-subprocess.run("ls", shell=True)
+  5 |+subprocess.run("ls", shell=True, check=False)
+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 explicit `check=False`
+
+ℹ Safe fix
+5 5 | subprocess.run("ls", shell=True)
+6 6 | subprocess.run(
+7 7 |     ["ls"],
+8   |-    shell=False,
+  8 |+    shell=False, check=False,
+9 9 | )
+10 10 | subprocess.run(["ls"], **kwargs)
+11 11 | 
+
+subprocess_run_without_check.py:10:1: PLW1510 [*] `subprocess.run` without explicit `check` argument
+   |
+ 8 |     shell=False,
+ 9 | )
+10 | subprocess.run(["ls"], **kwargs)
+   | ^^^^^^^^^^^^^^ PLW1510
+11 | 
+12 | # Non-errors.
+   |
+   = help: Add explicit `check=False`
+
+ℹ Unsafe fix
+7  7  |     ["ls"],
+8  8  |     shell=False,
+9  9  | )
+10    |-subprocess.run(["ls"], **kwargs)
+   10 |+subprocess.run(["ls"], **kwargs, check=False)
+11 11 | 
+12 12 | # Non-errors.
+13 13 | subprocess.run("ls", check=True)