Skip to content

Commit

Permalink
move everything to a single match
Browse files Browse the repository at this point in the history
  • Loading branch information
@augustelalande
augustelalande committed Mar 15, 2024
1 parent 838827a commit 66f14b4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 134 deletions.
126 changes: 32 additions & 94 deletions crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::analyze::typing::find_assigned_value;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::registry::Rule;
use crate::registry::AsRule;

/// ## What it does
/// Checks that async functions do not create subprocesses with blocking methods.
Expand Down Expand Up @@ -114,59 +114,39 @@ pub(crate) fn blocking_process_invocation(checker: &mut Checker, call: &ast::Exp
return;
}

if checker.enabled(Rule::CreateSubprocessInAsyncFunction) {
if is_subprocess_call(&call.func, checker.semantic())
|| (is_os_spawn(&call.func, checker.semantic()) && !is_p_wait(call, checker.semantic()))
{
checker.diagnostics.push(Diagnostic::new(
CreateSubprocessInAsyncFunction,
call.func.range(),
));
}
}

if checker.enabled(Rule::RunProcessInAsyncFunction) {
if is_run_process_call(&call.func, checker.semantic())
|| (is_os_spawn(&call.func, checker.semantic()) && is_p_wait(call, checker.semantic()))
{
checker.diagnostics.push(Diagnostic::new(
RunProcessInAsyncFunction,
call.func.range(),
));
}
}

if checker.enabled(Rule::WaitForProcessInAsyncFunction) {
if is_wait_for_process_call(&call.func, checker.semantic()) {
checker.diagnostics.push(Diagnostic::new(
WaitForProcessInAsyncFunction,
call.func.range(),
));
}
let Some(diagnostic_kind) =
checker
.semantic()
.resolve_qualified_name(call.func.as_ref())
.and_then(|qualified_name| match qualified_name.segments() {
["subprocess", "Popen"] | ["os", "popen"] => {
Some(CreateSubprocessInAsyncFunction.into())
}
["os", "system" | "posix_spawn" | "posix_spawnp"]
| ["subprocess", "run" | "call" | "check_call" | "check_output" | "getoutput"
| "getstatusoutput"] => Some(RunProcessInAsyncFunction.into()),
["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"] | ["time", "sleep"] => {
Some(WaitForProcessInAsyncFunction.into())
}
["os", "spawnl" | "spawnle" | "spawnlp" | "spawnlpe" | "spawnv" | "spawnve"
| "spawnvp" | "spawnvpe"] => {
if is_p_wait(call, checker.semantic()) {
Some(RunProcessInAsyncFunction.into())
} else {
Some(CreateSubprocessInAsyncFunction.into())
}
}
_ => None,
})
else {
return;
};
let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, call.range());
if checker.enabled(diagnostic.kind.rule()) {
checker.diagnostics.push(diagnostic);
}
}

fn is_os_spawn(func: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
[
"os",
"spawnl"
| "spawnle"
| "spawnlp"
| "spawnlpe"
| "spawnv"
| "spawnve"
| "spawnvp"
| "spawnvpe"
]
)
})
}

fn is_p_wait(call: &ast::ExprCall, semantic: &SemanticModel) -> bool {
let Some(arg) = call.arguments.find_argument("mode", 0) else {
return true;
Expand All @@ -184,45 +164,3 @@ fn is_p_wait(call: &ast::ExprCall, semantic: &SemanticModel) -> bool {
}
false
}

fn is_subprocess_call(func: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["subprocess", "Popen"] | ["os", "popen"]
)
})
}

fn is_run_process_call(func: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["os", "system" | "posix_spawn" | "posix_spawnp"]
| [
"subprocess",
"run"
| "call"
| "check_call"
| "check_output"
| "getoutput"
| "getstatusoutput"
]
)
})
}

fn is_wait_for_process_call(func: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"] | ["time", "sleep"]
)
})
}
16 changes: 8 additions & 8 deletions ...flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC220_ASYNC22x.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ ASYNC22x.py:36:5: ASYNC220 Async functions should not create subprocesses with b
34 | subprocess.getoutput() # ASYNC221
35 | )
36 | subprocess.Popen() # ASYNC220
| ^^^^^^^^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^^^ ASYNC220
37 | os.system() # ASYNC221
|

ASYNC22x.py:78:5: ASYNC220 Async functions should not create subprocesses with blocking methods
|
77 | # if mode is given, and is not os.P_WAIT: ASYNC220
78 | os.spawnl(os.P_NOWAIT) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^^^^^^^ ASYNC220
79 | os.spawnl(P_NOWAIT) # ASYNC220
80 | os.spawnl(mode=os.P_NOWAIT) # ASYNC220
|
Expand All @@ -24,7 +24,7 @@ ASYNC22x.py:79:5: ASYNC220 Async functions should not create subprocesses with b
77 | # if mode is given, and is not os.P_WAIT: ASYNC220
78 | os.spawnl(os.P_NOWAIT) # ASYNC220
79 | os.spawnl(P_NOWAIT) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^^^^ ASYNC220
80 | os.spawnl(mode=os.P_NOWAIT) # ASYNC220
81 | os.spawnl(mode=P_NOWAIT) # ASYNC220
|
Expand All @@ -34,7 +34,7 @@ ASYNC22x.py:80:5: ASYNC220 Async functions should not create subprocesses with b
78 | os.spawnl(os.P_NOWAIT) # ASYNC220
79 | os.spawnl(P_NOWAIT) # ASYNC220
80 | os.spawnl(mode=os.P_NOWAIT) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ASYNC220
81 | os.spawnl(mode=P_NOWAIT) # ASYNC220
|

Expand All @@ -43,7 +43,7 @@ ASYNC22x.py:81:5: ASYNC220 Async functions should not create subprocesses with b
79 | os.spawnl(P_NOWAIT) # ASYNC220
80 | os.spawnl(mode=os.P_NOWAIT) # ASYNC220
81 | os.spawnl(mode=P_NOWAIT) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^^^^^^^^^ ASYNC220
82 |
83 | P_WAIT = os.P_WAIT
|
Expand All @@ -52,7 +52,7 @@ ASYNC22x.py:91:5: ASYNC220 Async functions should not create subprocesses with b
|
90 | # other weird cases: ASYNC220
91 | os.spawnl(0) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^ ASYNC220
92 | os.spawnl(1) # ASYNC220
93 | os.spawnl(foo()) # ASYNC220
|
Expand All @@ -62,7 +62,7 @@ ASYNC22x.py:92:5: ASYNC220 Async functions should not create subprocesses with b
90 | # other weird cases: ASYNC220
91 | os.spawnl(0) # ASYNC220
92 | os.spawnl(1) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^ ASYNC220
93 | os.spawnl(foo()) # ASYNC220
|

Expand All @@ -71,7 +71,7 @@ ASYNC22x.py:93:5: ASYNC220 Async functions should not create subprocesses with b
91 | os.spawnl(0) # ASYNC220
92 | os.spawnl(1) # ASYNC220
93 | os.spawnl(foo()) # ASYNC220
| ^^^^^^^^^ ASYNC220
| ^^^^^^^^^^^^^^^^ ASYNC220
94 |
95 | # ASYNC222
|
Loading

0 comments on commit 66f14b4

Please sign in to comment.