Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
from typing import Optional

import httpx


def foo():
client = httpx.Client()
client.close() # Ok
client.delete() # Ok
client.get() # Ok
client.head() # Ok
client.options() # Ok
client.patch() # Ok
client.post() # Ok
client.put() # Ok
client.request() # Ok
client.send() # Ok
client.stream() # Ok

client.anything() # Ok
client.build_request() # Ok
client.is_closed # Ok


async def foo():
client = httpx.Client()
client.close() # ASYNC212
client.delete() # ASYNC212
client.get() # ASYNC212
client.head() # ASYNC212
client.options() # ASYNC212
client.patch() # ASYNC212
client.post() # ASYNC212
client.put() # ASYNC212
client.request() # ASYNC212
client.send() # ASYNC212
client.stream() # ASYNC212

client.anything() # Ok
client.build_request() # Ok
client.is_closed # Ok


async def foo(client: httpx.Client):
client.request() # ASYNC212
client.anything() # Ok


async def foo(client: httpx.Client | None):
client.request() # ASYNC212
client.anything() # Ok


async def foo(client: Optional[httpx.Client]):
client.request() # ASYNC212
client.anything() # Ok


async def foo():
client: httpx.Client = ...
client.request() # ASYNC212
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one still isn't caught, presumably because the TypeChecker bits only capture annotations at the function level?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should work. I played around a bit with the PTH210 rule, which uses the PathlibPathChecker, and it will detect both module-level and function-level bindings like this:

def foo():
    path: Path = ...
    path.with_suffix(".")

I'm wondering if the union traversal might be disrupting this check.

client.anything() # Ok


global_client = httpx.Client()


async def foo():
global_client.request() # ASYNC212
global_client.anything() # Ok


async def foo():
async with httpx.AsyncClient() as client:
await client.get() # Ok
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.is_rule_enabled(Rule::BlockingHttpCallInAsyncFunction) {
flake8_async::rules::blocking_http_call(checker, call);
}
if checker.is_rule_enabled(Rule::BlockingHttpCallHttpxInAsyncFunction) {
flake8_async::rules::blocking_http_call_httpx(checker, call);
}
if checker.is_rule_enabled(Rule::BlockingOpenCallInAsyncFunction) {
flake8_async::rules::blocking_open_call(checker, call);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Async, "115") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncZeroSleep),
(Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::LongSleepNotForever),
(Flake8Async, "210") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingHttpCallInAsyncFunction),
(Flake8Async, "212") => (RuleGroup::Preview, rules::flake8_async::rules::BlockingHttpCallHttpxInAsyncFunction),
(Flake8Async, "220") => (RuleGroup::Stable, rules::flake8_async::rules::CreateSubprocessInAsyncFunction),
(Flake8Async, "221") => (RuleGroup::Stable, rules::flake8_async::rules::RunProcessInAsyncFunction),
(Flake8Async, "222") => (RuleGroup::Stable, rules::flake8_async::rules::WaitForProcessInAsyncFunction),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_async/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ mod tests {
#[test_case(Rule::AsyncZeroSleep, Path::new("ASYNC115.py"))]
#[test_case(Rule::LongSleepNotForever, Path::new("ASYNC116.py"))]
#[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC210.py"))]
#[test_case(Rule::BlockingHttpCallHttpxInAsyncFunction, Path::new("ASYNC212.py"))]
#[test_case(Rule::CreateSubprocessInAsyncFunction, Path::new("ASYNC22x.py"))]
#[test_case(Rule::RunProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
#[test_case(Rule::WaitForProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
use ruff_python_ast::{self as ast, Expr, ExprCall};

use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_semantic::analyze::typing::{TypeChecker, check_type, traverse_union_and_optional};
use ruff_text_size::Ranged;

use crate::Violation;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks that async functions do not use blocking httpx clients.
///
/// ## Why is this bad?
/// Blocking an async function via a blocking HTTP call will block the entire
/// event loop, preventing it from executing other tasks while waiting for the
/// HTTP response, negating the benefits of asynchronous programming.
///
/// Instead of using the blocking `httpx` client, use the asynchronous client.
///
/// ## Example
/// ```python
/// import httpx
///
///
/// async def fetch():
/// client = httpx.Client()
/// response = client.get(...)
/// ```
///
/// Use instead:
/// ```python
/// import httpx
///
///
/// async def fetch():
/// async with httpx.AsyncClient() as client:
/// response = await client.get(...)
/// ```
#[derive(ViolationMetadata)]
pub(crate) struct BlockingHttpCallHttpxInAsyncFunction {
name: String,
call: String,
}

impl Violation for BlockingHttpCallHttpxInAsyncFunction {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"Blocking httpx method {name}.{call}() in async context, use httpx.AsyncClient",
name = self.name,
call = self.call,
)
}
}

struct HttpxClientChecker;

impl TypeChecker for HttpxClientChecker {
fn match_annotation(
annotation: &ruff_python_ast::Expr,
semantic: &ruff_python_semantic::SemanticModel,
) -> bool {
// match base annotation directly
if semantic
.resolve_qualified_name(annotation)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["httpx", "Client"]))
{
return true;
}

// otherwise traverse any union or optional annotation
let mut found = false;
traverse_union_and_optional(
&mut |inner_expr, _| {
if semantic
.resolve_qualified_name(inner_expr)
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["httpx", "Client"])
})
{
found = true;
}
},
semantic,
annotation,
);
found
}

fn match_initializer(
initializer: &ruff_python_ast::Expr,
semantic: &ruff_python_semantic::SemanticModel,
) -> bool {
let Expr::Call(ExprCall { func, .. }) = initializer else {
return false;
};

semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["httpx", "Client"]))
}
}

/// ASYNC212
pub(crate) fn blocking_http_call_httpx(checker: &Checker, call: &ExprCall) {
let semantic = checker.semantic();
if !semantic.in_async_context() {
return;
}

let Some(ast::ExprAttribute { value, attr, .. }) = call.func.as_attribute_expr() else {
return;
};
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::<HttpxClientChecker>(binding, semantic) {
if matches!(
attr.id.as_str(),
"close"
| "delete"
| "get"
| "head"
| "options"
| "patch"
| "post"
| "put"
| "request"
| "send"
| "stream"
) {
checker.report_diagnostic(
BlockingHttpCallHttpxInAsyncFunction {
name: name.id.to_string(),
call: attr.id.to_string(),
},
call.func.range(),
);
}
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_async/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub(crate) use async_busy_wait::*;
pub(crate) use async_function_with_timeout::*;
pub(crate) use async_zero_sleep::*;
pub(crate) use blocking_http_call::*;
pub(crate) use blocking_http_call_httpx::*;
pub(crate) use blocking_open_call::*;
pub(crate) use blocking_process_invocation::*;
pub(crate) use blocking_sleep::*;
Expand All @@ -13,6 +14,7 @@ mod async_busy_wait;
mod async_function_with_timeout;
mod async_zero_sleep;
mod blocking_http_call;
mod blocking_http_call_httpx;
mod blocking_open_call;
mod blocking_process_invocation;
mod blocking_sleep;
Expand Down
Loading
Loading