[pylint] Demote PLW1510 fix to display-only

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

@@ -5,7 +5,7 @@ use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::fix::edits::add_argument;
-use crate::{AlwaysFixableViolation, Applicability, Fix};
+use crate::{Fix, FixAvailability, Violation};
 
 /// ## What it does
 /// Checks for uses of `subprocess.run` without an explicit `check` argument.
@@ -39,24 +39,31 @@ use crate::{AlwaysFixableViolation, Applicability, Fix};
 /// ```
 ///
 /// ## 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.
+///
+/// This rule's fix is marked as display-only because it's not clear whether the
+/// potential exception was meant to be ignored by setting `check=False` or if
+/// the author simply forgot to include `check=True`. The fix adds
+/// `check=False`, making the existing behavior explicit but possibly masking
+/// the original intention.
 ///
 /// ## References
 /// - [Python documentation: `subprocess.run`](https://docs.python.org/3/library/subprocess.html#subprocess.run)
 #[derive(ViolationMetadata)]
 #[violation_metadata(stable_since = "v0.0.285")]
 pub(crate) struct SubprocessRunWithoutCheck;
 
-impl AlwaysFixableViolation for SubprocessRunWithoutCheck {
+impl Violation for SubprocessRunWithoutCheck {
+    // The fix is always set on the diagnostic, but display-only fixes aren't
+    // considered "fixable" in the tests.
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         "`subprocess.run` without explicit `check` argument".to_string()
     }
 
-    fn fix_title(&self) -> String {
-        "Add explicit `check=False`".to_string()
+    fn fix_title(&self) -> Option<String> {
+        Some("Add explicit `check=False`".to_string())
     }
 }
 
@@ -74,20 +81,11 @@ pub(crate) fn subprocess_run_without_check(checker: &Checker, call: &ast::ExprCa
         if call.arguments.find_keyword("check").is_none() {
             let mut diagnostic =
                 checker.report_diagnostic(SubprocessRunWithoutCheck, call.func.range());
-            diagnostic.set_fix(Fix::applicable_edit(
-                add_argument("check=False", &call.arguments, checker.tokens()),
-                // 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
-                },
-            ));
+            diagnostic.set_fix(Fix::display_only_edit(add_argument(
+                "check=False",
+                &call.arguments,
+                checker.tokens(),
+            )));
         }
     }
 }

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

@@ -19,6 +19,7 @@ help: Add explicit `check=False`
 5 | subprocess.run("ls", shell=True)
 6 | subprocess.run(
 7 |     ["ls"],
+note: This is a display-only fix and is likely to be incorrect
 
 PLW1510 [*] `subprocess.run` without explicit `check` argument
  --> subprocess_run_without_check.py:5:1
@@ -39,6 +40,7 @@ help: Add explicit `check=False`
 6 | subprocess.run(
 7 |     ["ls"],
 8 |     shell=False,
+note: This is a display-only fix and is likely to be incorrect
 
 PLW1510 [*] `subprocess.run` without explicit `check` argument
  --> subprocess_run_without_check.py:6:1
@@ -59,6 +61,7 @@ help: Add explicit `check=False`
 9  | )
 10 | subprocess.run(["ls"], **kwargs)
 11 | 
+note: This is a display-only fix and is likely to be incorrect
 
 PLW1510 [*] `subprocess.run` without explicit `check` argument
   --> subprocess_run_without_check.py:10:1
@@ -79,4 +82,4 @@ help: Add explicit `check=False`
 11 | 
 12 | # Non-errors.
 13 | subprocess.run("ls", check=True)
-note: This is an unsafe fix and may change runtime behavior
+note: This is a display-only fix and is likely to be incorrect