[pylint] Implement subprocess-popen-preexec-fn (W1509) (#5978)

## Summary

Implements Pylint rule [`subprocess-popen-preexec-fn`
(`W1509`)](https://pylint.pycqa.org/en/latest/user_guide/messages/warning/subprocess-popen-preexec-fn.html)
as `subprocess-popen-preexec-fn` (`PLW1509`). Includes documentation.
Related to #970.

## Test Plan

`cargo test`

crates/ruff/resources/test/fixtures/pylint/subprocess_popen_preexec_fn.py

@@ -0,0 +1,18 @@
+import subprocess
+
+
+def foo():
+    pass
+
+
+# Errors.
+subprocess.Popen(preexec_fn=foo)
+subprocess.Popen(["ls"], preexec_fn=foo)
+subprocess.Popen(preexec_fn=lambda: print("Hello, world!"))
+subprocess.Popen(["ls"], preexec_fn=lambda: print("Hello, world!"))
+
+# Non-errors.
+subprocess.Popen()
+subprocess.Popen(["ls"])
+subprocess.Popen(preexec_fn=None)  # None is the default.
+subprocess.Popen(["ls"], preexec_fn=None)  # None is the default.

crates/ruff/src/checkers/ast/mod.rs

@@ -2961,6 +2961,9 @@ where
                         self.diagnostics.push(diagnostic);
                     }
                 }
+                if self.enabled(Rule::SubprocessPopenPreexecFn) {
+                    pylint::rules::subprocess_popen_preexec_fn(self, func, keywords);
+                }
                 if self.any_enabled(&[
                     Rule::PytestRaisesWithoutException,
                     Rule::PytestRaisesTooBroad,

crates/ruff/src/codes.rs

@@ -172,10 +172,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "C0131") => (RuleGroup::Unspecified, rules::pylint::rules::TypeBivariance),
         (Pylint, "C0132") => (RuleGroup::Unspecified, rules::pylint::rules::TypeParamNameMismatch),
         (Pylint, "C0205") => (RuleGroup::Unspecified, rules::pylint::rules::SingleStringSlots),
+        (Pylint, "C0208") => (RuleGroup::Unspecified, rules::pylint::rules::IterationOverSet),
         (Pylint, "C0414") => (RuleGroup::Unspecified, rules::pylint::rules::UselessImportAlias),
         (Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
         (Pylint, "C3002") => (RuleGroup::Unspecified, rules::pylint::rules::UnnecessaryDirectLambdaCall),
-        (Pylint, "C0208") => (RuleGroup::Unspecified, rules::pylint::rules::IterationOverSet),
         (Pylint, "E0100") => (RuleGroup::Unspecified, rules::pylint::rules::YieldInInit),
         (Pylint, "E0101") => (RuleGroup::Unspecified, rules::pylint::rules::ReturnInInit),
         (Pylint, "E0116") => (RuleGroup::Unspecified, rules::pylint::rules::ContinueInFinally),
@@ -221,6 +221,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "W0603") => (RuleGroup::Unspecified, rules::pylint::rules::GlobalStatement),
         (Pylint, "W0711") => (RuleGroup::Unspecified, rules::pylint::rules::BinaryOpException),
         (Pylint, "W1508") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidEnvvarDefault),
+        (Pylint, "W1509") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessPopenPreexecFn),
         (Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName),
         (Pylint, "W3301") => (RuleGroup::Unspecified, rules::pylint::rules::NestedMinMax),
 

crates/ruff/src/rules/pylint/mod.rs

@@ -116,6 +116,10 @@ mod tests {
         Rule::RepeatedEqualityComparisonTarget,
         Path::new("repeated_equality_comparison_target.py")
     )]
+    #[test_case(
+        Rule::SubprocessPopenPreexecFn,
+        Path::new("subprocess_popen_preexec_fn.py")
+    )]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff/src/rules/pylint/rules/mod.rs

@@ -33,6 +33,7 @@ pub(crate) use repeated_equality_comparison_target::*;
 pub(crate) use repeated_isinstance_calls::*;
 pub(crate) use return_in_init::*;
 pub(crate) use single_string_slots::*;
+pub(crate) use subprocess_popen_preexec_fn::*;
 pub(crate) use sys_exit_alias::*;
 pub(crate) use too_many_arguments::*;
 pub(crate) use too_many_branches::*;
@@ -84,6 +85,7 @@ mod repeated_equality_comparison_target;
 mod repeated_isinstance_calls;
 mod return_in_init;
 mod single_string_slots;
+mod subprocess_popen_preexec_fn;
 mod sys_exit_alias;
 mod too_many_arguments;
 mod too_many_branches;

crates/ruff/src/rules/pylint/rules/subprocess_popen_preexec_fn.rs

@@ -0,0 +1,67 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::{find_keyword, is_const_none};
+use rustpython_parser::ast::{Expr, Keyword, Ranged};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for uses of `subprocess.Popen` with a `preexec_fn` argument.
+///
+/// ## Why is this bad?
+/// The `preexec_fn` argument is unsafe within threads as it can lead to
+/// deadlocks. Furthermore, `preexec_fn` is [targeted for deprecation].
+///
+/// Instead, consider using task-specific arguments such as `env`,
+/// `start_new_session`, and `process_group`. These are not prone to deadlocks
+/// and are more explicit.
+///
+/// ## Example
+/// ```python
+/// import os, subprocess
+///
+/// subprocess.Popen(foo, preexec_fn=os.setsid)
+/// subprocess.Popen(bar, preexec_fn=os.setpgid(0, 0))
+/// ```
+///
+/// Use instead:
+/// ```python
+/// import subprocess
+///
+/// subprocess.Popen(foo, start_new_session=True)
+/// subprocess.Popen(bar, process_group=0)  # Introduced in Python 3.11
+/// ```
+///
+/// ## References
+/// - [Python documentation: `subprocess.Popen`](https://docs.python.org/3/library/subprocess.html#popen-constructor)
+/// - [Why `preexec_fn` in `subprocess.Popen` may lead to deadlock?](https://discuss.python.org/t/why-preexec-fn-in-subprocess-popen-may-lead-to-deadlock/16908/2)
+///
+/// [targeted for deprecation]: https://github.com/python/cpython/issues/82616
+#[violation]
+pub struct SubprocessPopenPreexecFn;
+
+impl Violation for SubprocessPopenPreexecFn {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("`preexec_fn` argument is unsafe when using threads")
+    }
+}
+
+/// PLW1509
+pub(crate) fn subprocess_popen_preexec_fn(checker: &mut Checker, func: &Expr, kwargs: &[Keyword]) {
+    if checker
+        .semantic()
+        .resolve_call_path(func)
+        .map_or(false, |call_path| {
+            matches!(call_path.as_slice(), ["subprocess", "Popen"])
+        })
+    {
+        if let Some(keyword) =
+            find_keyword(kwargs, "preexec_fn").filter(|keyword| !is_const_none(&keyword.value))
+        {
+            checker
+                .diagnostics
+                .push(Diagnostic::new(SubprocessPopenPreexecFn, keyword.range()));
+        }
+    }
+}

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1509_subprocess_popen_preexec_fn.py.snap

@@ -0,0 +1,42 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+---
+subprocess_popen_preexec_fn.py:9:18: PLW1509 `preexec_fn` argument is unsafe when using threads
+   |
+ 8 | # Errors.
+ 9 | subprocess.Popen(preexec_fn=foo)
+   |                  ^^^^^^^^^^^^^^ PLW1509
+10 | subprocess.Popen(["ls"], preexec_fn=foo)
+11 | subprocess.Popen(preexec_fn=lambda: print("Hello, world!"))
+   |
+
+subprocess_popen_preexec_fn.py:10:26: PLW1509 `preexec_fn` argument is unsafe when using threads
+   |
+ 8 | # Errors.
+ 9 | subprocess.Popen(preexec_fn=foo)
+10 | subprocess.Popen(["ls"], preexec_fn=foo)
+   |                          ^^^^^^^^^^^^^^ PLW1509
+11 | subprocess.Popen(preexec_fn=lambda: print("Hello, world!"))
+12 | subprocess.Popen(["ls"], preexec_fn=lambda: print("Hello, world!"))
+   |
+
+subprocess_popen_preexec_fn.py:11:18: PLW1509 `preexec_fn` argument is unsafe when using threads
+   |
+ 9 | subprocess.Popen(preexec_fn=foo)
+10 | subprocess.Popen(["ls"], preexec_fn=foo)
+11 | subprocess.Popen(preexec_fn=lambda: print("Hello, world!"))
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW1509
+12 | subprocess.Popen(["ls"], preexec_fn=lambda: print("Hello, world!"))
+   |
+
+subprocess_popen_preexec_fn.py:12:26: PLW1509 `preexec_fn` argument is unsafe when using threads
+   |
+10 | subprocess.Popen(["ls"], preexec_fn=foo)
+11 | subprocess.Popen(preexec_fn=lambda: print("Hello, world!"))
+12 | subprocess.Popen(["ls"], preexec_fn=lambda: print("Hello, world!"))
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW1509
+13 | 
+14 | # Non-errors.
+   |
+
+