Skip to content

Commit

Permalink
Tweak semantic lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 30, 2024
1 parent b97f1e5 commit 4ce3037
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 128 deletions.
60 changes: 37 additions & 23 deletions crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC101.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,69 +5,83 @@

# 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 foo():
Path("foo").open() # ASYNC101

async def foo():

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


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


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


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

async def bar():
p.open() # ASYNC101
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 foo():
Foo().open() # OK

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

async def foo():

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

def foo():
Path("foo").open() # OK
open() # OK


def func():
Path("foo").open() # OK
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
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_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 @@ -42,61 +40,58 @@ impl Violation for OpenSleepOrSubprocessInAsyncFunction {
}

/// ASYNC101
pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, call: &ExprCall) {
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 {
pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, call: &ast::ExprCall) {
if !checker.semantic().in_async_context() {
return;
}
checker.diagnostics.push(Diagnostic::new(
OpenSleepOrSubprocessInAsyncFunction,
call.func.range(),
));

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"]
)
})
}

/// Analyze if the open call is from `pathlib.Path`
/// PTH123 (builtin-open) suggests to use `pathlib.Path.open` instead of `open`
/// 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(ExprAttribute { attr, value, .. }) = func else {
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return false;
};

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

// Check first if the call is from `pathlib.Path.open`:
// First: is this an inlined call to `pathlib.Path.open`?
// ```python
// from pathlib import Path
// Path("foo").open()
// 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 {
Expand All @@ -107,35 +102,28 @@ fn is_open_call_from_pathlib(func: &Expr, semantic: &SemanticModel) -> bool {
}
}

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

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

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

let Stmt::Assign(StmtAssign { value, .. }) = semantic.statement(node_id) else {
let Some(Expr::Call(call)) = analyze::typing::find_binding_value(&name.id, binding, semantic)
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
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,82 +1,84 @@
---
source: crates/ruff_linter/src/rules/flake8_async/mod.rs
---
ASYNC101.py:9:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
8 | async def foo():
9 | 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:13: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
|
12 | async def foo():
13 | time.sleep(1)
13 | async def func():
14 | time.sleep(1)
| ^^^^^^^^^^ ASYNC101
|

ASYNC101.py:17: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
|
16 | async def foo():
17 | subprocess.run("foo")
17 | async def func():
18 | subprocess.run("foo")
| ^^^^^^^^^^^^^^ ASYNC101
|

ASYNC101.py:21: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
|
20 | async def foo():
21 | subprocess.call("foo")
21 | async def func():
22 | subprocess.call("foo")
| ^^^^^^^^^^^^^^^ ASYNC101
|

ASYNC101.py:29: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
|
28 | async def foo():
29 | os.wait4(10)
29 | async def func():
30 | os.wait4(10)
| ^^^^^^^^ ASYNC101
|

ASYNC101.py:33: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
|
32 | async def foo():
33 | os.wait(12)
33 | async def func():
34 | 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
ASYNC101.py:41:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
37 | async def foo():
38 | Path("foo").open() # ASYNC101
40 | async def func():
41 | 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
ASYNC101.py:46: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
44 | async def func():
45 | p = Path("foo")
46 | 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
ASYNC101.py:50: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
49 | async def func():
50 | with Path("foo").open() as f: # ASYNC101
| ^^^^^^^^^^^^^^^^ ASYNC101
46 | pass
51 | pass
|

ASYNC101.py:53:9: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
ASYNC101.py:58:9: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods
|
52 | async def bar():
53 | p.open() # ASYNC101
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
|
62 | (p1, p2) = (Path("foo"), Path("bar"))
63 |
64 | p1.open() # ASYNC101
| ^^^^^^^ ASYNC101
|


0 comments on commit 4ce3037

Please sign in to comment.