diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC240.py b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC240.py new file mode 100644 index 0000000000..6eff90e56c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC240.py @@ -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 + + diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 65c61ff802..d664b7da98 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -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, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index bcfbcf2d55..6b2a317bae 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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), diff --git a/crates/ruff_linter/src/rules/flake8_async/mod.rs b/crates/ruff_linter/src/rules/flake8_async/mod.rs index bdfee91469..1c92fe8358 100644 --- a/crates/ruff_linter/src/rules/flake8_async/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_async/mod.rs @@ -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<()> { diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs b/crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs new file mode 100644 index 0000000000..14827c5b43 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs @@ -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::(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" + ) +} diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs index 86103c5980..3e12ea360c 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC240_ASYNC240.py.snap b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC240_ASYNC240.py.snap new file mode 100644 index 0000000000..888905aed7 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC240_ASYNC240.py.snap @@ -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 + | ^^^^^^^^^^^ + | diff --git a/ruff.schema.json b/ruff.schema.json index 954d88ab4a..3b1e4da9d3 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3020,6 +3020,8 @@ "ASYNC222", "ASYNC23", "ASYNC230", + "ASYNC24", + "ASYNC240", "ASYNC25", "ASYNC250", "ASYNC251",