[pylint] Implement subprocess-run-check (W1510) (#6487)

## Summary

Implements [`subprocess-run-check`
(`W1510`)](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/subprocess-run-check.html)
as `subprocess-run-without-check` (`PLW1510`). Includes documentation.

Related to #970.

## Test Plan

`cargo test`
This commit is contained in:
Tom Kuson 2023-08-11 01:54:53 +01:00 committed by GitHub
parent 84ae00c395
commit 9ff80a82b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 112 additions and 0 deletions

View file

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

View file

@ -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,

View file

@ -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),

View file

@ -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(

View file

@ -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::*;
@ -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;

View file

@ -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(),
));
}
}
}

View file

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