Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import subprocess

# Errors.
subprocess.run("ls")
subprocess.run("ls", shell=True)

# Non-errors.
subprocess.run("ls", check=True)
subprocess.run("ls", check=False)
subprocess.run("ls", shell=True, check=True)
subprocess.run("ls", shell=True, check=False)
foo.run("ls") # Not a subprocess.run call.
subprocess.bar("ls") # Not a subprocess.run call.
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::SubprocessPopenPreexecFn) {
pylint::rules::subprocess_popen_preexec_fn(checker, call);
}
if checker.enabled(Rule::SubprocessRunWithoutCheck) {
pylint::rules::subprocess_run_without_check(checker, call);
}
if checker.any_enabled(&[
Rule::PytestRaisesWithoutException,
Rule::PytestRaisesTooBroad,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(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, "W1510") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessRunWithoutCheck),
(Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash),
(Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName),
(Pylint, "W3301") => (RuleGroup::Unspecified, rules::pylint::rules::NestedMinMax),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ mod tests {
Rule::SubprocessPopenPreexecFn,
Path::new("subprocess_popen_preexec_fn.py")
)]
#[test_case(
Rule::SubprocessRunWithoutCheck,
Path::new("subprocess_run_without_check.py")
)]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) use return_in_init::*;
pub(crate) use self_assigning_variable::*;
pub(crate) use single_string_slots::*;
pub(crate) use subprocess_popen_preexec_fn::*;
pub(crate) use subprocess_run_without_check::*;
pub(crate) use sys_exit_alias::*;
pub(crate) use too_many_arguments::*;
pub(crate) use too_many_branches::*;
Expand Down Expand Up @@ -92,6 +93,7 @@ mod return_in_init;
mod self_assigning_variable;
mod single_string_slots;
mod subprocess_popen_preexec_fn;
mod subprocess_run_without_check;
mod sys_exit_alias;
mod too_many_arguments;
mod too_many_branches;
Expand Down
65 changes: 65 additions & 0 deletions crates/ruff/src/rules/pylint/rules/subprocess_run_without_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of `subprocess.run` without an explicit `check` argument.
///
/// ## Why is this bad?
/// By default, `subprocess.run` does not check the return code of the process
/// it runs. This can lead to silent failures.
///
/// Instead, consider using `check=True` to raise an exception if the process
/// fails, or set `check=False` explicitly to mark the behavior as intentional.
///
/// ## Example
/// ```python
/// import subprocess
///
/// subprocess.run(["ls", "nonexistent"]) # No exception raised.
/// ```
///
/// Use instead:
/// ```python
/// import subprocess
///
/// subprocess.run(["ls", "nonexistent"], check=True) # Raises exception.
/// ```
///
/// Or:
/// ```python
/// import subprocess
///
/// subprocess.run(["ls", "nonexistent"], check=False) # Explicitly no check.
/// ```
///
/// ## References
/// - [Python documentation: `subprocess.run`](https://docs.python.org/3/library/subprocess.html#subprocess.run)
#[violation]
pub struct SubprocessRunWithoutCheck;

impl Violation for SubprocessRunWithoutCheck {
#[derive_message_formats]
fn message(&self) -> String {
format!("`subprocess.run` without explicit `check` argument")
}
}

/// PLW1510
pub(crate) fn subprocess_run_without_check(checker: &mut Checker, call: &ast::ExprCall) {
if checker
.semantic()
.resolve_call_path(&call.func)
.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(
SubprocessRunWithoutCheck,
call.func.range(),
));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
---
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)
|

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.
|


2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.