Skip to content

Commit

Permalink
[flake8-async] Take pathlib.Path into account when analyzing asyn…
Browse files Browse the repository at this point in the history
…c functions (#9703)

## Summary

This review contains a fix for
[ASYNC101](https://docs.astral.sh/ruff/rules/open-sleep-or-subprocess-in-async-function/)
(open-sleep-or-subprocess-in-async-function)

The problem is that ruff does not take open calls from pathlib.Path into
account in async functions. Path.open() call is still a blocking call.
In addition, PTH123 suggests to use pathlib.Path instead of os.open. So
this might create an additional confusion.

See: #6892

## Test Plan

```bash
cargo test
```
  • Loading branch information
mikeleppane committed Jan 30, 2024
1 parent 0c8d140 commit 79f0522
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 61 deletions.
Original file line number Diff line number Diff line change
@@ -1,31 +1,87 @@
import os
import subprocess
import time
from pathlib import Path

# Violation cases:

async def foo():

async def func():
open("foo")


async def foo():
async def func():
time.sleep(1)


async def foo():
async def func():
subprocess.run("foo")


async def foo():
async def func():
subprocess.call("foo")


async def foo():
async def func():
subprocess.foo(0)


async def foo():
async def func():
os.wait4(10)


async def foo():
async def func():
os.wait(12)


# Violation cases for pathlib:


async def func():
Path("foo").open() # ASYNC101


async def func():
p = Path("foo")
p.open() # ASYNC101


async def func():
with Path("foo").open() as f: # ASYNC101
pass


async def func() -> None:
p = Path("foo")

async def bar():
p.open() # ASYNC101


async def func() -> None:
(p1, p2) = (Path("foo"), Path("bar"))

p1.open() # ASYNC101


# Non-violation cases for pathlib:


class Foo:
def open(self):
pass


async def func():
Foo().open() # OK


async def func():
def open():
pass

open() # OK


def func():
Path("foo").open() # OK
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use ruff_python_ast::ExprCall;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::CallPath;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::{analyze, SemanticModel};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -41,37 +40,90 @@ impl Violation for OpenSleepOrSubprocessInAsyncFunction {
}

/// ASYNC101
pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, call: &ExprCall) {
if checker.semantic().in_async_context() {
if checker
.semantic()
.resolve_call_path(call.func.as_ref())
.as_ref()
.is_some_and(is_open_sleep_or_subprocess_call)
{
checker.diagnostics.push(Diagnostic::new(
OpenSleepOrSubprocessInAsyncFunction,
call.func.range(),
));
}
pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, call: &ast::ExprCall) {
if !checker.semantic().in_async_context() {
return;
}

if is_open_sleep_or_subprocess_call(&call.func, checker.semantic())
|| is_open_call_from_pathlib(call.func.as_ref(), checker.semantic())
{
checker.diagnostics.push(Diagnostic::new(
OpenSleepOrSubprocessInAsyncFunction,
call.func.range(),
));
}
}

fn is_open_sleep_or_subprocess_call(call_path: &CallPath) -> bool {
matches!(
call_path.as_slice(),
["", "open"]
| ["time", "sleep"]
| [
"subprocess",
"run"
| "Popen"
| "call"
| "check_call"
| "check_output"
| "getoutput"
| "getstatusoutput"
]
| ["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"]
)
/// Returns `true` if the expression resolves to a blocking call, like `time.sleep` or
/// `subprocess.run`.
fn is_open_sleep_or_subprocess_call(func: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(func).is_some_and(|call_path| {
matches!(
call_path.as_slice(),
["", "open"]
| ["time", "sleep"]
| [
"subprocess",
"run"
| "Popen"
| "call"
| "check_call"
| "check_output"
| "getoutput"
| "getstatusoutput"
]
| ["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"]
)
})
}

/// Returns `true` if an expression resolves to a call to `pathlib.Path.open`.
fn is_open_call_from_pathlib(func: &Expr, semantic: &SemanticModel) -> bool {
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return false;
};

if attr.as_str() != "open" {
return false;
}

// First: is this an inlined call to `pathlib.Path.open`?
// ```python
// from pathlib import Path
// Path("foo").open()
// ```
if let Expr::Call(call) = value.as_ref() {
let Some(call_path) = semantic.resolve_call_path(call.func.as_ref()) else {
return false;
};
if call_path.as_slice() == ["pathlib", "Path"] {
return true;
}
}

// Second, is this a call to `pathlib.Path.open` via a variable?
// ```python
// from pathlib import Path
// path = Path("foo")
// path.open()
// ```
let Expr::Name(name) = value.as_ref() else {
return false;
};

let Some(binding_id) = semantic.resolve_name(name) else {
return false;
};

let binding = semantic.binding(binding_id);

let Some(Expr::Call(call)) = analyze::typing::find_binding_value(&name.id, binding, semantic)
else {
return false;
};

semantic
.resolve_call_path(call.func.as_ref())
.is_some_and(|call_path| call_path.as_slice() == ["pathlib", "Path"])
}
Original file line number Diff line number Diff line change
@@ -1,45 +1,83 @@
---
source: crates/ruff_linter/src/rules/flake8_async/mod.rs
---
ASYNC101.py:7:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
6 | async def foo():
7 | open("foo")
| ^^^^ ASYNC101
|
ASYNC101.py:10:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
9 | async def func():
10 | open("foo")
| ^^^^ ASYNC101
|

ASYNC101.py:11:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:14:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
10 | async def foo():
11 | time.sleep(1)
13 | async def func():
14 | time.sleep(1)
| ^^^^^^^^^^ ASYNC101
|

ASYNC101.py:15:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:18:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
14 | async def foo():
15 | subprocess.run("foo")
17 | async def func():
18 | subprocess.run("foo")
| ^^^^^^^^^^^^^^ ASYNC101
|

ASYNC101.py:19:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:22:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
18 | async def foo():
19 | subprocess.call("foo")
21 | async def func():
22 | subprocess.call("foo")
| ^^^^^^^^^^^^^^^ ASYNC101
|

ASYNC101.py:27:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:30:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
26 | async def foo():
27 | os.wait4(10)
29 | async def func():
30 | os.wait4(10)
| ^^^^^^^^ ASYNC101
|

ASYNC101.py:31:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:34:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
33 | async def func():
34 | os.wait(12)
| ^^^^^^^ ASYNC101
|

ASYNC101.py:41:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
40 | async def func():
41 | Path("foo").open() # ASYNC101
| ^^^^^^^^^^^^^^^^ ASYNC101
|

ASYNC101.py:46:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
44 | async def func():
45 | p = Path("foo")
46 | p.open() # ASYNC101
| ^^^^^^ ASYNC101
|

ASYNC101.py:50:10: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
49 | async def func():
50 | with Path("foo").open() as f: # ASYNC101
| ^^^^^^^^^^^^^^^^ ASYNC101
51 | pass
|

ASYNC101.py:58:9: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
57 | async def bar():
58 | p.open() # ASYNC101
| ^^^^^^ ASYNC101
|

ASYNC101.py:64:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
30 | async def foo():
31 | os.wait(12)
62 | (p1, p2) = (Path("foo"), Path("bar"))
63 |
64 | p1.open() # ASYNC101
| ^^^^^^^ ASYNC101
|

Expand Down

0 comments on commit 79f0522

Please sign in to comment.