mirror of https://github.com/astral-sh/ruff
[`flake8-async`] Implement `blocking-path-method` (`ASYNC240`) (#20264)
## Summary Adds a new rule to find and report use of `os.path` or `pathlib.Path` in async functions. Issue: #8451 ## Test Plan Using `cargo insta test`
This commit is contained in:
Pieter Cardillo Kwok
GitHub
parent
346842f003
commit
edb920b4d5
|
|
@ -0,0 +1,104 @@
|
|||
import os
|
||||
from typing import Optional
|
||||
from pathlib import Path
|
||||
|
||||
## Valid cases:
|
||||
|
||||
def os_path_in_foo():
|
||||
file = "file.txt"
|
||||
|
||||
os.path.abspath(file) # OK
|
||||
os.path.exists(file) # OK
|
||||
os.path.split() # OK
|
||||
|
||||
async def non_io_os_path_methods():
|
||||
os.path.split() # OK
|
||||
os.path.dirname() # OK
|
||||
os.path.basename() # OK
|
||||
os.path.join() # OK
|
||||
|
||||
def pathlib_path_in_foo():
|
||||
path = Path("src/my_text.txt") # OK
|
||||
path.exists() # OK
|
||||
with path.open() as f: # OK
|
||||
...
|
||||
path = Path("src/my_text.txt").open() # OK
|
||||
|
||||
async def non_io_pathlib_path_methods():
|
||||
path = Path("src/my_text.txt")
|
||||
path.is_absolute() # OK
|
||||
path.is_relative_to() # OK
|
||||
path.as_posix() # OK
|
||||
path.relative_to() # OK
|
||||
|
||||
def inline_path_method_call():
|
||||
Path("src/my_text.txt").open() # OK
|
||||
Path("src/my_text.txt").open().flush() # OK
|
||||
with Path("src/my_text.txt").open() as f: # OK
|
||||
...
|
||||
|
||||
async def trio_path_in_foo():
|
||||
from trio import Path
|
||||
|
||||
path = Path("src/my_text.txt") # OK
|
||||
await path.absolute() # OK
|
||||
await path.exists() # OK
|
||||
with Path("src/my_text.txt").open() as f: # OK
|
||||
...
|
||||
|
||||
async def anyio_path_in_foo():
|
||||
from anyio import Path
|
||||
|
||||
path = Path("src/my_text.txt") # OK
|
||||
await path.absolute() # OK
|
||||
await path.exists() # OK
|
||||
with Path("src/my_text.txt").open() as f: # OK
|
||||
...
|
||||
|
||||
async def path_open_in_foo():
|
||||
path = Path("src/my_text.txt") # OK
|
||||
path.open() # OK, covered by ASYNC230
|
||||
|
||||
## Invalid cases:
|
||||
|
||||
async def os_path_in_foo():
|
||||
file = "file.txt"
|
||||
|
||||
os.path.abspath(file) # ASYNC240
|
||||
os.path.exists(file) # ASYNC240
|
||||
|
||||
async def pathlib_path_in_foo():
|
||||
path = Path("src/my_text.txt")
|
||||
path.exists() # ASYNC240
|
||||
|
||||
async def pathlib_path_in_foo():
|
||||
import pathlib
|
||||
|
||||
path = pathlib.Path("src/my_text.txt")
|
||||
path.exists() # ASYNC240
|
||||
|
||||
async def inline_path_method_call():
|
||||
Path("src/my_text.txt").exists() # ASYNC240
|
||||
Path("src/my_text.txt").absolute().exists() # ASYNC240
|
||||
|
||||
async def aliased_path_in_foo():
|
||||
from pathlib import Path as PathAlias
|
||||
|
||||
path = PathAlias("src/my_text.txt")
|
||||
path.exists() # ASYNC240
|
||||
|
||||
global_path = Path("src/my_text.txt")
|
||||
|
||||
async def global_path_in_foo():
|
||||
global_path.exists() # ASYNC240
|
||||
|
||||
async def path_as_simple_parameter_type(path: Path):
|
||||
path.exists() # ASYNC240
|
||||
|
||||
async def path_as_union_parameter_type(path: Path | None):
|
||||
path.exists() # ASYNC240
|
||||
|
||||
async def path_as_optional_parameter_type(path: Optional[Path]):
|
||||
path.exists() # ASYNC240
|
||||
|
||||
|
||||
|
|
@ -669,6 +669,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
|
|||
if checker.is_rule_enabled(Rule::BlockingOpenCallInAsyncFunction) {
|
||||
flake8_async::rules::blocking_open_call(checker, call);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::BlockingPathMethodInAsyncFunction) {
|
||||
flake8_async::rules::blocking_os_path(checker, call);
|
||||
}
|
||||
if checker.any_rule_enabled(&[
|
||||
Rule::CreateSubprocessInAsyncFunction,
|
||||
Rule::RunProcessInAsyncFunction,
|
||||
|
|
|
|||
|
|
@ -341,6 +341,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8Async, "221") => (RuleGroup::Stable, rules::flake8_async::rules::RunProcessInAsyncFunction),
|
||||
(Flake8Async, "222") => (RuleGroup::Stable, rules::flake8_async::rules::WaitForProcessInAsyncFunction),
|
||||
(Flake8Async, "230") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingOpenCallInAsyncFunction),
|
||||
(Flake8Async, "240") => (RuleGroup::Preview, rules::flake8_async::rules::BlockingPathMethodInAsyncFunction),
|
||||
(Flake8Async, "250") => (RuleGroup::Preview, rules::flake8_async::rules::BlockingInputInAsyncFunction),
|
||||
(Flake8Async, "251") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingSleepInAsyncFunction),
|
||||
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ mod tests {
|
|||
#[test_case(Rule::RunProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
|
||||
#[test_case(Rule::WaitForProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
|
||||
#[test_case(Rule::BlockingOpenCallInAsyncFunction, Path::new("ASYNC230.py"))]
|
||||
#[test_case(Rule::BlockingPathMethodInAsyncFunction, Path::new("ASYNC240.py"))]
|
||||
#[test_case(Rule::BlockingInputInAsyncFunction, Path::new("ASYNC250.py"))]
|
||||
#[test_case(Rule::BlockingSleepInAsyncFunction, Path::new("ASYNC251.py"))]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,246 @@
|
|||
use crate::Violation;
|
||||
use crate::checkers::ast::Checker;
|
||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast::{self as ast, Expr, ExprCall};
|
||||
use ruff_python_semantic::analyze::typing::{TypeChecker, check_type, traverse_union_and_optional};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks that async functions do not call blocking `os.path` or `pathlib.Path`
|
||||
/// methods.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Calling some `os.path` or `pathlib.Path` methods in an async function will block
|
||||
/// the entire event loop, preventing it from executing other tasks while waiting
|
||||
/// for the operation. This negates the benefits of asynchronous programming.
|
||||
///
|
||||
/// Instead, use the methods' async equivalents from `trio.Path` or `anyio.Path`.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// import os
|
||||
///
|
||||
///
|
||||
/// async def func():
|
||||
/// path = "my_file.txt"
|
||||
/// file_exists = os.path.exists(path)
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// import trio
|
||||
///
|
||||
///
|
||||
/// async def func():
|
||||
/// path = trio.Path("my_file.txt")
|
||||
/// file_exists = await path.exists()
|
||||
/// ```
|
||||
///
|
||||
/// Non-blocking methods are OK to use:
|
||||
/// ```python
|
||||
/// import pathlib
|
||||
///
|
||||
///
|
||||
/// async def func():
|
||||
/// path = pathlib.Path("my_file.txt")
|
||||
/// file_dirname = path.dirname()
|
||||
/// new_path = os.path.join("/tmp/src/", path)
|
||||
/// ```
|
||||
#[derive(ViolationMetadata)]
|
||||
pub(crate) struct BlockingPathMethodInAsyncFunction {
|
||||
path_library: String,
|
||||
}
|
||||
|
||||
impl Violation for BlockingPathMethodInAsyncFunction {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!(
|
||||
"Async functions should not use {path_library} methods, use trio.Path or anyio.path",
|
||||
path_library = self.path_library
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// ASYNC240
|
||||
pub(crate) fn blocking_os_path(checker: &Checker, call: &ExprCall) {
|
||||
let semantic = checker.semantic();
|
||||
if !semantic.in_async_context() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if an expression is calling I/O related os.path method.
|
||||
// Just initializing pathlib.Path object is OK, we can return
|
||||
// early in that scenario.
|
||||
if let Some(qualified_name) = semantic.resolve_qualified_name(call.func.as_ref()) {
|
||||
let segments = qualified_name.segments();
|
||||
if !matches!(segments, ["os", "path", _]) {
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(os_path_method) = segments.last() else {
|
||||
return;
|
||||
};
|
||||
|
||||
if maybe_calling_io_operation(os_path_method) {
|
||||
checker.report_diagnostic(
|
||||
BlockingPathMethodInAsyncFunction {
|
||||
path_library: "os.path".to_string(),
|
||||
},
|
||||
call.func.range(),
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(ast::ExprAttribute { value, attr, .. }) = call.func.as_attribute_expr() else {
|
||||
return;
|
||||
};
|
||||
|
||||
if !maybe_calling_io_operation(attr.id.as_str()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if an expression is a pathlib.Path constructor that directly
|
||||
// calls an I/O method.
|
||||
if PathlibPathChecker::match_initializer(value, semantic) {
|
||||
checker.report_diagnostic(
|
||||
BlockingPathMethodInAsyncFunction {
|
||||
path_library: "pathlib.Path".to_string(),
|
||||
},
|
||||
call.func.range(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Lastly, check if a variable is a pathlib.Path instance and it's
|
||||
// calling an I/O method.
|
||||
let Some(name) = value.as_name_expr() else {
|
||||
return;
|
||||
};
|
||||
|
||||
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
|
||||
return;
|
||||
};
|
||||
|
||||
if check_type::<PathlibPathChecker>(binding, semantic) {
|
||||
checker.report_diagnostic(
|
||||
BlockingPathMethodInAsyncFunction {
|
||||
path_library: "pathlib.Path".to_string(),
|
||||
},
|
||||
call.func.range(),
|
||||
);
|
||||
}
|
||||
}
|
||||
struct PathlibPathChecker;
|
||||
|
||||
impl PathlibPathChecker {
|
||||
fn is_pathlib_path_constructor(
|
||||
semantic: &ruff_python_semantic::SemanticModel,
|
||||
expr: &Expr,
|
||||
) -> bool {
|
||||
let Some(qualified_name) = semantic.resolve_qualified_name(expr) else {
|
||||
return false;
|
||||
};
|
||||
|
||||
matches!(
|
||||
qualified_name.segments(),
|
||||
[
|
||||
"pathlib",
|
||||
"Path"
|
||||
| "PosixPath"
|
||||
| "PurePath"
|
||||
| "PurePosixPath"
|
||||
| "PureWindowsPath"
|
||||
| "WindowsPath"
|
||||
]
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl TypeChecker for PathlibPathChecker {
|
||||
fn match_annotation(annotation: &Expr, semantic: &ruff_python_semantic::SemanticModel) -> bool {
|
||||
if Self::is_pathlib_path_constructor(semantic, annotation) {
|
||||
return true;
|
||||
}
|
||||
|
||||
let mut found = false;
|
||||
traverse_union_and_optional(
|
||||
&mut |inner_expr, _| {
|
||||
if Self::is_pathlib_path_constructor(semantic, inner_expr) {
|
||||
found = true;
|
||||
}
|
||||
},
|
||||
semantic,
|
||||
annotation,
|
||||
);
|
||||
found
|
||||
}
|
||||
|
||||
fn match_initializer(
|
||||
initializer: &Expr,
|
||||
semantic: &ruff_python_semantic::SemanticModel,
|
||||
) -> bool {
|
||||
let Expr::Call(ast::ExprCall { func, .. }) = initializer else {
|
||||
return false;
|
||||
};
|
||||
|
||||
Self::is_pathlib_path_constructor(semantic, func)
|
||||
}
|
||||
}
|
||||
|
||||
fn maybe_calling_io_operation(attr: &str) -> bool {
|
||||
// ".open()" is added to the allow list to let ASYNC 230 handle
|
||||
// that case.
|
||||
!matches!(
|
||||
attr,
|
||||
"ALLOW_MISSING"
|
||||
| "altsep"
|
||||
| "anchor"
|
||||
| "as_posix"
|
||||
| "as_uri"
|
||||
| "basename"
|
||||
| "commonpath"
|
||||
| "commonprefix"
|
||||
| "curdir"
|
||||
| "defpath"
|
||||
| "devnull"
|
||||
| "dirname"
|
||||
| "drive"
|
||||
| "expandvars"
|
||||
| "extsep"
|
||||
| "genericpath"
|
||||
| "is_absolute"
|
||||
| "is_relative_to"
|
||||
| "is_reserved"
|
||||
| "isabs"
|
||||
| "join"
|
||||
| "joinpath"
|
||||
| "match"
|
||||
| "name"
|
||||
| "normcase"
|
||||
| "os"
|
||||
| "open"
|
||||
| "pardir"
|
||||
| "parent"
|
||||
| "parents"
|
||||
| "parts"
|
||||
| "pathsep"
|
||||
| "relative_to"
|
||||
| "root"
|
||||
| "samestat"
|
||||
| "sep"
|
||||
| "split"
|
||||
| "splitdrive"
|
||||
| "splitext"
|
||||
| "splitroot"
|
||||
| "stem"
|
||||
| "suffix"
|
||||
| "suffixes"
|
||||
| "supports_unicode_filenames"
|
||||
| "sys"
|
||||
| "with_name"
|
||||
| "with_segments"
|
||||
| "with_stem"
|
||||
| "with_suffix"
|
||||
)
|
||||
}
|
||||
|
|
@ -5,6 +5,7 @@ pub(crate) use blocking_http_call::*;
|
|||
pub(crate) use blocking_http_call_httpx::*;
|
||||
pub(crate) use blocking_input::*;
|
||||
pub(crate) use blocking_open_call::*;
|
||||
pub(crate) use blocking_path_methods::*;
|
||||
pub(crate) use blocking_process_invocation::*;
|
||||
pub(crate) use blocking_sleep::*;
|
||||
pub(crate) use cancel_scope_no_checkpoint::*;
|
||||
|
|
@ -18,6 +19,7 @@ mod blocking_http_call;
|
|||
mod blocking_http_call_httpx;
|
||||
mod blocking_input;
|
||||
mod blocking_open_call;
|
||||
mod blocking_path_methods;
|
||||
mod blocking_process_invocation;
|
||||
mod blocking_sleep;
|
||||
mod cancel_scope_no_checkpoint;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,111 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_async/mod.rs
|
||||
---
|
||||
ASYNC240 Async functions should not use os.path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:67:5
|
||||
|
|
||||
65 | file = "file.txt"
|
||||
66 |
|
||||
67 | os.path.abspath(file) # ASYNC240
|
||||
| ^^^^^^^^^^^^^^^
|
||||
68 | os.path.exists(file) # ASYNC240
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use os.path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:68:5
|
||||
|
|
||||
67 | os.path.abspath(file) # ASYNC240
|
||||
68 | os.path.exists(file) # ASYNC240
|
||||
| ^^^^^^^^^^^^^^
|
||||
69 |
|
||||
70 | async def pathlib_path_in_foo():
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:72:5
|
||||
|
|
||||
70 | async def pathlib_path_in_foo():
|
||||
71 | path = Path("src/my_text.txt")
|
||||
72 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
73 |
|
||||
74 | async def pathlib_path_in_foo():
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:78:5
|
||||
|
|
||||
77 | path = pathlib.Path("src/my_text.txt")
|
||||
78 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
79 |
|
||||
80 | async def inline_path_method_call():
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:81:5
|
||||
|
|
||||
80 | async def inline_path_method_call():
|
||||
81 | Path("src/my_text.txt").exists() # ASYNC240
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
82 | Path("src/my_text.txt").absolute().exists() # ASYNC240
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:82:5
|
||||
|
|
||||
80 | async def inline_path_method_call():
|
||||
81 | Path("src/my_text.txt").exists() # ASYNC240
|
||||
82 | Path("src/my_text.txt").absolute().exists() # ASYNC240
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
83 |
|
||||
84 | async def aliased_path_in_foo():
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:88:5
|
||||
|
|
||||
87 | path = PathAlias("src/my_text.txt")
|
||||
88 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
89 |
|
||||
90 | global_path = Path("src/my_text.txt")
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:93:5
|
||||
|
|
||||
92 | async def global_path_in_foo():
|
||||
93 | global_path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
94 |
|
||||
95 | async def path_as_simple_parameter_type(path: Path):
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:96:5
|
||||
|
|
||||
95 | async def path_as_simple_parameter_type(path: Path):
|
||||
96 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
97 |
|
||||
98 | async def path_as_union_parameter_type(path: Path | None):
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:99:5
|
||||
|
|
||||
98 | async def path_as_union_parameter_type(path: Path | None):
|
||||
99 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
100 |
|
||||
101 | async def path_as_optional_parameter_type(path: Optional[Path]):
|
||||
|
|
||||
|
||||
ASYNC240 Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
|
||||
--> ASYNC240.py:102:5
|
||||
|
|
||||
101 | async def path_as_optional_parameter_type(path: Optional[Path]):
|
||||
102 | path.exists() # ASYNC240
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
|
|
@ -3020,6 +3020,8 @@
|
|||
"ASYNC222",
|
||||
"ASYNC23",
|
||||
"ASYNC230",
|
||||
"ASYNC24",
|
||||
"ASYNC240",
|
||||
"ASYNC25",
|
||||
"ASYNC250",
|
||||
"ASYNC251",
|
||||
|
|
|
|||
Loading…
Reference in New Issue