Skip to content

Commit

Permalink
[flake8-async] Take pathlib.Path into account when analyzing open cal…
Browse files Browse the repository at this point in the history
…ls (ASYNC101)
  • Loading branch information
vai-mikkoleppanen committed Jan 30, 2024
1 parent dacda0f commit b97f1e5
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import os
import subprocess
import time
from pathlib import Path

# Violation cases:

async def foo():
open("foo")
Expand Down Expand Up @@ -29,3 +31,43 @@ async def foo():

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

# Violation cases for pathlib:

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

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

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


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

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


# Non-violation cases for pathlib:


class Foo:
def open(self):
pass

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


async def foo():
def open():
pass
open() # OK

def foo():
Path("foo").open() # OK
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use ruff_python_ast::ExprCall;
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, ExprName, Stmt, StmtAssign};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::CallPath;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -42,19 +43,24 @@ 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(),
));
let should_add_to_diagnostics = if checker.semantic().in_async_context() {
if let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) {
is_open_sleep_or_subprocess_call(&call_path)
|| is_open_call_from_pathlib(call.func.as_ref(), checker.semantic())
} else {
is_open_call_from_pathlib(call.func.as_ref(), checker.semantic())
}
} else {
false
};

if !should_add_to_diagnostics {
return;
}
checker.diagnostics.push(Diagnostic::new(
OpenSleepOrSubprocessInAsyncFunction,
call.func.range(),
));
}

fn is_open_sleep_or_subprocess_call(call_path: &CallPath) -> bool {
Expand All @@ -75,3 +81,61 @@ fn is_open_sleep_or_subprocess_call(call_path: &CallPath) -> bool {
| ["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"]
)
}

/// Analyze if the open call is from `pathlib.Path`
/// PTH123 (builtin-open) suggests to use `pathlib.Path.open` instead of `open`
fn is_open_call_from_pathlib(func: &Expr, semantic: &SemanticModel) -> bool {
let Expr::Attribute(ExprAttribute { attr, value, .. }) = func else {
return false;
};

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

// Check first if the call is from `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;
}
}

// Otherwise, check if Path.open call is bind to a variable:
// ```python
// from pathlib import Path
// p = Path("foo")
// p.open()
//
let Expr::Name(ExprName { id, .. }) = value.as_ref() else {
return false;
};

let Some(binding_id) = semantic.lookup_symbol(id) else {
return false;
};

let Some(node_id) = semantic.binding(binding_id).source else {
return false;
};

let Stmt::Assign(StmtAssign { value, .. }) = semantic.statement(node_id) else {
return false;
};

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;
}
}
false
}
Original file line number Diff line number Diff line change
@@ -1,46 +1,82 @@
---
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
ASYNC101.py:9:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
6 | async def foo():
7 | open("foo")
8 | async def foo():
9 | open("foo")
| ^^^^ ASYNC101
|

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

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

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

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

ASYNC101.py:31:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:33:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
30 | async def foo():
31 | os.wait(12)
32 | async def foo():
33 | os.wait(12)
| ^^^^^^^ ASYNC101
34 |
35 | # Violation cases for pathlib:
|

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

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

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

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


0 comments on commit b97f1e5

Please sign in to comment.