Skip to content

Commit bc9d2a8

Browse files
committed
[flake8-async] Implement blocking-path-method (ASYNC240).
Adds a new rule to find and report use of `os.path` or `pathlib.Path` in async functions.
1 parent ec55842 commit bc9d2a8

File tree

8 files changed

+438
-0
lines changed

8 files changed

+438
-0
lines changed
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import os
2+
from typing import Optional
3+
4+
## Various os.path cases:
5+
6+
def os_path_in_foo():
7+
file = "file.txt"
8+
9+
os.path.abspath(file) # OK
10+
os.path.exists(file) # OK
11+
12+
async def os_path_in_foo():
13+
file = "file.txt"
14+
15+
os.path.abspath(file) # ASYNC240
16+
os.path.exists(file) # ASYNC240
17+
18+
## Various pathlib.Path cases:
19+
from pathlib import Path
20+
21+
def pathlib_path_in_foo():
22+
path = Path("src/my_text.txt") # OK
23+
path.absolute() # OK
24+
path.exists() # OK
25+
with path.open() as f: # OK
26+
...
27+
28+
async def pathlib_path_in_foo():
29+
path = Path("src/my_text.txt") # ASYNC240
30+
path.absolute() # ASYNC240
31+
path.exists() # ASYNC240
32+
with path.open() as f: # ASYNC240
33+
...
34+
35+
async def pathlib_path_in_foo():
36+
import pathlib
37+
38+
path = pathlib.Path("src/my_text.txt") # ASYNC240
39+
path.absolute() # ASYNC240
40+
path.exists() # ASYNC240
41+
42+
async def pathlib_path_in_fo():
43+
from pathlib import Path as PathAlias
44+
45+
path = PathAlias("src/my_text.txt") # ASYNC240
46+
path.absolute() # ASYNC240
47+
path.exists() # ASYNC240
48+
49+
global_path = Path("src/my_text.txt")
50+
51+
async def pathlib_path_as_variable_in_foo():
52+
global_path.absolute() # ASYNC240
53+
global_path.exists() # ASYNC240
54+
55+
def pathlib_path_in_foo(path: Path):
56+
path.absolute() # OK
57+
path.exists() # OK
58+
59+
async def pathlib_path_in_foo(path: Path):
60+
path.absolute() # ASYNC240
61+
path.exists() # ASYNC240
62+
63+
async def pathlib_path_in_foo(path: Path | None):
64+
path.absolute() # ASYNC240
65+
path.exists() # ASYNC240
66+
67+
async def pathlib_path_in_foo(path: Optional[Path]):
68+
path.absolute() # ASYNC240
69+
path.exists() # ASYNC240
70+
71+
## Valid cases using trio/anyio:
72+
73+
async def trio_path_in_foo():
74+
from trio import Path # OK
75+
76+
path = Path("src/my_text.txt") # OK
77+
path.absolute() # OK
78+
path.exists() # OK
79+
80+
async def anyio_path_in_foo():
81+
from anyio import Path # OK
82+
83+
path = Path("src/my_text.txt") # OK
84+
path.absolute() # OK
85+
path.exists() # OK
86+
87+

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
666666
if checker.is_rule_enabled(Rule::BlockingOpenCallInAsyncFunction) {
667667
flake8_async::rules::blocking_open_call(checker, call);
668668
}
669+
if checker.is_rule_enabled(Rule::BlockingPathMethodInAsyncFunction) {
670+
flake8_async::rules::blocking_os_path(checker, call);
671+
}
669672
if checker.any_rule_enabled(&[
670673
Rule::CreateSubprocessInAsyncFunction,
671674
Rule::RunProcessInAsyncFunction,

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
341341
(Flake8Async, "221") => (RuleGroup::Stable, rules::flake8_async::rules::RunProcessInAsyncFunction),
342342
(Flake8Async, "222") => (RuleGroup::Stable, rules::flake8_async::rules::WaitForProcessInAsyncFunction),
343343
(Flake8Async, "230") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingOpenCallInAsyncFunction),
344+
(Flake8Async, "240") => (RuleGroup::Preview, rules::flake8_async::rules::BlockingPathMethodInAsyncFunction),
344345
(Flake8Async, "250") => (RuleGroup::Preview, rules::flake8_async::rules::BlockingInputInAsyncFunction),
345346
(Flake8Async, "251") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingSleepInAsyncFunction),
346347

crates/ruff_linter/src/rules/flake8_async/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ mod tests {
2828
#[test_case(Rule::RunProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
2929
#[test_case(Rule::WaitForProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
3030
#[test_case(Rule::BlockingOpenCallInAsyncFunction, Path::new("ASYNC230.py"))]
31+
#[test_case(Rule::BlockingPathMethodInAsyncFunction, Path::new("ASYNC240.py"))]
3132
#[test_case(Rule::BlockingInputInAsyncFunction, Path::new("ASYNC250.py"))]
3233
#[test_case(Rule::BlockingSleepInAsyncFunction, Path::new("ASYNC251.py"))]
3334
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
use crate::Violation;
2+
use crate::checkers::ast::Checker;
3+
use ruff_macros::{ViolationMetadata, derive_message_formats};
4+
use ruff_python_ast::{self as ast, Expr, ExprCall};
5+
use ruff_python_semantic::analyze::typing::{TypeChecker, check_type, traverse_union_and_optional};
6+
use ruff_text_size::Ranged;
7+
8+
/// ## What it does
9+
/// Checks that async functions do not use `os.path`.
10+
///
11+
/// ## Why is this bad?
12+
/// Blocking an async function via `os.path` calls will block the entire
13+
/// event loop, preventing it from executing other tasks while waiting for the
14+
/// `os.path` operation, negating the benefits of asynchronous programming.
15+
///
16+
/// Instead of `os.path`, use `trio.Path` or `anyio.path` objects.
17+
///
18+
/// ## Example
19+
/// ```python
20+
/// import os
21+
///
22+
///
23+
/// async def func():
24+
/// path = "my_file.txt"
25+
/// file_exists = os.path.exists(path)
26+
/// ```
27+
///
28+
/// Use instead:
29+
/// ```python
30+
/// import trio
31+
///
32+
///
33+
/// async def func():
34+
/// path = trio.Path("my_file.txt")
35+
/// file_exists = await path.exists():
36+
/// ```
37+
#[derive(ViolationMetadata)]
38+
pub(crate) struct BlockingPathMethodInAsyncFunction;
39+
40+
impl Violation for BlockingPathMethodInAsyncFunction {
41+
#[derive_message_formats]
42+
fn message(&self) -> String {
43+
"Async functions should not use pathlib.Path or os.path methods, use trio.Path or anyio.path".to_string()
44+
}
45+
}
46+
47+
const OS_PATH: [&str; 2] = ["os", "path"];
48+
const PATHLIB_PATH: [&str; 2] = ["pathlib", "Path"];
49+
50+
fn is_blocking_path_method(segments: &[&str]) -> bool {
51+
segments.starts_with(&OS_PATH) || segments.starts_with(&PATHLIB_PATH)
52+
}
53+
54+
struct PathMethodInAsyncChecker;
55+
56+
impl TypeChecker for PathMethodInAsyncChecker {
57+
fn match_annotation(
58+
annotation: &ruff_python_ast::Expr,
59+
semantic: &ruff_python_semantic::SemanticModel,
60+
) -> bool {
61+
// match base annotation directly
62+
if semantic
63+
.resolve_qualified_name(annotation)
64+
.is_some_and(|qualified_name| is_blocking_path_method(qualified_name.segments()))
65+
{
66+
return true;
67+
}
68+
69+
// otherwise traverse any union or optional annotation
70+
let mut found = false;
71+
traverse_union_and_optional(
72+
&mut |inner_expr, _| {
73+
if semantic
74+
.resolve_qualified_name(inner_expr)
75+
.is_some_and(|qualified_name| {
76+
is_blocking_path_method(qualified_name.segments())
77+
})
78+
{
79+
found = true;
80+
}
81+
},
82+
semantic,
83+
annotation,
84+
);
85+
found
86+
}
87+
88+
fn match_initializer(
89+
initializer: &ruff_python_ast::Expr,
90+
semantic: &ruff_python_semantic::SemanticModel,
91+
) -> bool {
92+
let Expr::Call(ExprCall { func, .. }) = initializer else {
93+
return false;
94+
};
95+
96+
semantic
97+
.resolve_qualified_name(func)
98+
.is_some_and(|qualified_name| is_blocking_path_method(qualified_name.segments()))
99+
}
100+
}
101+
102+
/// ASYNC240
103+
pub(crate) fn blocking_os_path(checker: &Checker, call: &ExprCall) {
104+
let semantic = checker.semantic();
105+
if !semantic.in_async_context() {
106+
return;
107+
}
108+
109+
// Check if the expression is calling the blocking path method
110+
// using its imported symbol/name.
111+
if let Some(qualified_name) = semantic.resolve_qualified_name(call.func.as_ref()) {
112+
let segments = qualified_name.segments();
113+
if is_blocking_path_method(segments){
114+
checker.report_diagnostic(BlockingPathMethodInAsyncFunction, call.func.range());
115+
}
116+
return;
117+
}
118+
119+
// Past this, we're checking if the expression is binded to one
120+
// of the blocking path methods.
121+
let Some(ast::ExprAttribute { value, .. }) = call.func.as_attribute_expr() else {
122+
return;
123+
};
124+
125+
let Some(name) = value.as_name_expr() else {
126+
return;
127+
};
128+
129+
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
130+
return;
131+
};
132+
133+
if check_type::<PathMethodInAsyncChecker>(binding, semantic) {
134+
checker.report_diagnostic(BlockingPathMethodInAsyncFunction, call.func.range());
135+
}
136+
}

crates/ruff_linter/src/rules/flake8_async/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pub(crate) use blocking_http_call::*;
55
pub(crate) use blocking_http_call_httpx::*;
66
pub(crate) use blocking_input::*;
77
pub(crate) use blocking_open_call::*;
8+
pub(crate) use blocking_path_methods::*;
89
pub(crate) use blocking_process_invocation::*;
910
pub(crate) use blocking_sleep::*;
1011
pub(crate) use cancel_scope_no_checkpoint::*;
@@ -18,6 +19,7 @@ mod blocking_http_call;
1819
mod blocking_http_call_httpx;
1920
mod blocking_input;
2021
mod blocking_open_call;
22+
mod blocking_path_methods;
2123
mod blocking_process_invocation;
2224
mod blocking_sleep;
2325
mod cancel_scope_no_checkpoint;

0 commit comments

Comments
 (0)