[`flake8-async`] Implement `blocking-path-method` (`ASYNC240`) #20264

PieterCK · 2025-09-05T10:08:09Z

Summary

Adds a new rule to find and report use of os.path or pathlib.Path in
async functions.

Test Plan

Using cargo insta test

ntBre · 2025-09-05T12:52:49Z

Thanks! We're pretty focused on the minor release this week, so I may not get a chance to review until next week. But this looks really good from a quick skim :)

github-actions · 2025-09-05T13:03:30Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

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

Adds a new rule to find and report use of `os.path` or `pathlib.Path` in async functions.

PieterCK · 2025-09-17T05:51:11Z

Bumping this PR for review again.

ntBre · 2025-09-17T16:24:35Z

What's the best way to show new PR changes here?

I usually just push additional commits. That works better with GitHub's "Changes since your last review" button than comparing rebased commits. We always squash and merge PRs anyway. But it's not too big of a deal either way :)

I'll give this another look now!

ntBre

Thank you! This looks great to me overall, I just had a few minor comments. I especially liked the tests. They were really easy to read and review 😄

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

ntBre · 2025-09-17T16:52:04Z

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

+    };
+
+    if check_type::<PathlibPathChecker>(binding, semantic) {
+        if maybe_calling_io_operation(attr.id.as_str()) {


This is the same attr as above right? I think we could do this check once before both the constructor and check_type checks, returning early if it's not one of the known attributes.

Aha, good catch. I moved this a few checks above to just before the constructor check.

crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC240.py

ntBre · 2025-09-17T17:13:46Z

crates/ruff_python_semantic/src/analyze/typing.rs

+        traverse_union_and_optional(
+            &mut |inner_expr, _| {
+                if Self::is_pathlib_path_constructor(semantic, inner_expr) {
+                    found = true;
+                }
+            },
+            semantic,
+            annotation,
+        );


I do like having this check like in #20091, but I'm a little bit hesitant to add it here since it will affect other rules too. I would probably either revert this and accept that we miss option/union annotations for now, until we possibly update the TypeChecker infrastructure more generally in the future, or move this check into the rule itself, as @amyreese did for ASYNC212.

Makes sense, for now I added a copy of PathlibPathChecker that has the traversing logic to this rule.

This also reverts back the changes made to `PathlibPathChecker` in the previous commit.

PieterCK · 2025-09-18T11:42:27Z

Thank you for doing another pass on this! Will post another comment once this is ready for review again.

This commit contains changes addressing some code reviews: - The `Violation::message` implementation doesn't mention that the linter only look for blocking path methods and non-blocking methods are allowed. I've tweaked the comments and added another code example of async function using non-blocking os/pathlib path methods. - The `is_calling_os_path_method` function could be simpler. I've used the suggested implementation and pulled it out from the function since it's only one line now. - The logic to see if an expression is calling a method form the pathlib.Path module could be simpler if it calls `PathlibPathChecker::match_initializer` directly. - The rule function can return early if `maybe_calling_io_operation` returns false when checking the expressions attribute.

This commit contains changes addressing some code reviews: - Updated `maybe_calling_io_operation` to have a more complete allow list. - ASYNC230 already handles `path.open()` case, so it is added to the allow list. There is a new test for that. - Fix typo.

Move the main rule implementation to just below the `BlockingPathMethodInAsyncFunction` implementation.

PieterCK · 2025-09-22T15:35:26Z

@ntBre -- Marking this as ready for you to take a look again!

amyreese

looks good to me!

ntBre

Thank you, this looks good to me too! I just had one comment about moving the todo comment to a separate issue for a follow-up discussion.

ntBre · 2025-09-22T18:35:42Z

crates/ruff_python_semantic/src/analyze/typing.rs

+    // match_annotation doesn't check `Path` in union
+    // or `Optional`.
+    // TODO: Once we have updated it to handle those
+    // cases, update `blocking_path_method.rs`(ASYNC240)
+    // to use this version of `PathlibPathChecker`.


I think it's good to record this TODO somewhere, but I'm kind of leaning toward opening a separate issue and deleting this comment. I feel like we should probably either always traverse unions or never traverse unions and apply that decision to all of our TypeCheckers rather than keep a mix of them.

Okay. This commit removes the comment, Remove comment on PathlibPathChecker.

And opened #20529 to keep track of this.

ntBre

Thank you!

PieterCK · 2025-09-23T14:42:03Z

Awesome! Thank you

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch from bc9d2a8 to a16ea55 Compare September 5, 2025 10:18

PieterCK marked this pull request as ready for review September 5, 2025 10:24

PieterCK mentioned this pull request Sep 5, 2025

Implement flake8-async rules #8451

Open

37 tasks

ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Sep 5, 2025

ntBre self-requested a review September 5, 2025 12:53

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch 4 times, most recently from 19b3ea6 to be73d44 Compare September 5, 2025 14:02

amyreese reviewed Sep 5, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs Show resolved Hide resolved

PieterCK marked this pull request as draft September 8, 2025 15:46

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch 3 times, most recently from 0950d6e to 20b5b46 Compare September 9, 2025 09:01

PieterCK commented Sep 9, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs Outdated Show resolved Hide resolved

PieterCK commented Sep 9, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_async/rules/blocking_path_methods.rs Outdated Show resolved Hide resolved

PieterCK changed the title ~~[flake8-async] Implement blocking-path-method (ASYNC240).~~ wip [flake8-async] Implement blocking-path-method (ASYNC240). Sep 9, 2025

PieterCK added 2 commits September 9, 2025 19:41

[flake8-async] Implement blocking-path-method (ASYNC240).

c2125ed

Adds a new rule to find and report use of `os.path` or `pathlib.Path` in async functions.

Update PathlibPathChecker to check nested annotation.

c5b5f27

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch from 20b5b46 to c5b5f27 Compare September 9, 2025 12:46

PieterCK marked this pull request as ready for review September 9, 2025 13:04

This comment was marked as outdated.

Sign in to view

PieterCK changed the title ~~wip [flake8-async] Implement blocking-path-method (ASYNC240).~~ [flake8-async] Implement blocking-path-method (ASYNC240). Sep 9, 2025

PieterCK requested a review from amyreese September 10, 2025 06:19

ntBre reviewed Sep 17, 2025

View reviewed changes

Create a new TypeChecker for ASYNC240.

439fd2a

This also reverts back the changes made to `PathlibPathChecker` in the previous commit.

PieterCK marked this pull request as draft September 18, 2025 11:41

PieterCK changed the title ~~[flake8-async] Implement blocking-path-method (ASYNC240).~~ [wip] [flake8-async] Implement blocking-path-method (ASYNC240). Sep 18, 2025

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch from caedfa8 to c594fb8 Compare September 22, 2025 15:22

PieterCK added 3 commits September 22, 2025 22:26

Address code reviews.

fc6a4ec

This commit contains changes addressing some code reviews: - Updated `maybe_calling_io_operation` to have a more complete allow list. - ASYNC230 already handles `path.open()` case, so it is added to the allow list. There is a new test for that. - Fix typo.

Address code review.

f31d0b6

Move the main rule implementation to just below the `BlockingPathMethodInAsyncFunction` implementation.

PieterCK force-pushed the add-rule-async-240-blocking-path-usage branch from c594fb8 to f31d0b6 Compare September 22, 2025 15:26

PieterCK changed the title ~~[wip] [flake8-async] Implement blocking-path-method (ASYNC240).~~ [flake8-async] Implement blocking-path-method (ASYNC240). Sep 22, 2025

PieterCK marked this pull request as ready for review September 22, 2025 15:34

amyreese approved these changes Sep 22, 2025

View reviewed changes

ntBre approved these changes Sep 22, 2025

View reviewed changes

Remove comment on PathlibPathChecker.

9c3dd35

PieterCK mentioned this pull request Sep 23, 2025

Standardize traverse union behavior in TypeCheckers. #20529

Open

ntBre approved these changes Sep 23, 2025

View reviewed changes

ntBre changed the title ~~[flake8-async] Implement blocking-path-method (ASYNC240).~~ [flake8-async] Implement blocking-path-method (ASYNC240) Sep 23, 2025

ntBre merged commit edb920b into astral-sh:main Sep 23, 2025
36 checks passed

BrewTestBot mentioned this pull request Sep 25, 2025

ruff 0.13.2 Homebrew/homebrew-core#245692

Merged

[flake8-async] Implement blocking-path-method (ASYNC240) #20264

[flake8-async] Implement blocking-path-method (ASYNC240) #20264

Uh oh!

Conversation

PieterCK commented Sep 5, 2025

Summary

Test Plan

Uh oh!

ntBre commented Sep 5, 2025

Uh oh!

github-actions bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

PieterCK commented Sep 17, 2025

Uh oh!

ntBre commented Sep 17, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ntBre Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

PieterCK Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ntBre Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

PieterCK Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

PieterCK commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PieterCK commented Sep 22, 2025

Uh oh!

amyreese left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

PieterCK Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PieterCK Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PieterCK commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

[`flake8-async`] Implement `blocking-path-method` (`ASYNC240`) #20264

[`flake8-async`] Implement `blocking-path-method` (`ASYNC240`) #20264

github-actions bot commented Sep 5, 2025 •

edited

Loading

`ruff-ecosystem` results

PieterCK commented Sep 18, 2025 •

edited

Loading

PieterCK Sep 23, 2025 •

edited

Loading

PieterCK Sep 23, 2025 •

edited

Loading