diff --git a/LICENSE b/LICENSE index b98ccc9c78268..534e3357426bf 100644 --- a/LICENSE +++ b/LICENSE @@ -814,6 +814,31 @@ are: SOFTWARE. """ +- flake8-async, licensed as follows: + """ + MIT License + + Copyright (c) 2022 Cooper Lees + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + """ + - flake8-type-checking, licensed as follows: """ Copyright (c) 2021, Sondre Lillebø Gundersen diff --git a/README.md b/README.md index 116ecd7812aaa..dc6160230341f 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ quality tools, including: - [eradicate](https://pypi.org/project/eradicate/) - [flake8-2020](https://pypi.org/project/flake8-2020/) - [flake8-annotations](https://pypi.org/project/flake8-annotations/) +- [flake8-async](https://pypi.org/project/flake8-async) - [flake8-bandit](https://pypi.org/project/flake8-bandit/) ([#1646](https://github.com/charliermarsh/ruff/issues/1646)) - [flake8-blind-except](https://pypi.org/project/flake8-blind-except/) - [flake8-boolean-trap](https://pypi.org/project/flake8-boolean-trap/) diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASYNC100.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC100.py new file mode 100644 index 0000000000000..532273a7b4676 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC100.py @@ -0,0 +1,23 @@ +import urllib.request +import requests +import httpx + + +async def foo(): + urllib.request.urlopen("http://example.com/foo/bar").read() + + +async def foo(): + requests.get() + + +async def foo(): + httpx.get() + + +async def foo(): + requests.post() + + +async def foo(): + httpx.post() diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASYNC101.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC101.py new file mode 100644 index 0000000000000..32fbaeb9aabbe --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC101.py @@ -0,0 +1,31 @@ +import os +import subprocess +import time + + +async def foo(): + open("foo") + + +async def foo(): + time.sleep(1) + + +async def foo(): + subprocess.run("foo") + + +async def foo(): + subprocess.call("foo") + + +async def foo(): + subprocess.foo(0) + + +async def foo(): + os.wait4(10) + + +async def foo(): + os.wait(12) diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASYNC102.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC102.py new file mode 100644 index 0000000000000..7912bcc9decab --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC102.py @@ -0,0 +1,13 @@ +import os + + +async def foo(): + os.popen() + + +async def foo(): + os.spawnl() + + +async def foo(): + os.fspath("foo") diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 7919d6ae29b8f..c5bf4a6da2114 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -42,9 +42,9 @@ use crate::importer::Importer; use crate::noqa::NoqaMapping; use crate::registry::{AsRule, Rule}; use crate::rules::{ - flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap, - flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger, - flake8_django, flake8_errmsg, flake8_future_annotations, flake8_gettext, + flake8_2020, flake8_annotations, flake8_async, flake8_bandit, flake8_blind_except, + flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, + flake8_debugger, flake8_django, flake8_errmsg, flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_import_conventions, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_self, flake8_simplify, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, @@ -2584,6 +2584,29 @@ where pyupgrade::rules::use_pep604_isinstance(self, expr, func, args); } + // flake8-async + if self + .settings + .rules + .enabled(Rule::BlockingHttpCallInAsyncFunction) + { + flake8_async::rules::blocking_http_call(self, expr); + } + if self + .settings + .rules + .enabled(Rule::OpenSleepOrSubprocessInAsyncFunction) + { + flake8_async::rules::open_sleep_or_subprocess_call(self, expr); + } + if self + .settings + .rules + .enabled(Rule::BlockingOsCallInAsyncFunction) + { + flake8_async::rules::blocking_os_call(self, expr); + } + // flake8-print if self .settings diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index ef7926af69fe5..4443fb4becdef 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -215,6 +215,11 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Pylint, "W3301") => Rule::NestedMinMax, (Pylint, "E0241") => Rule::DuplicateBases, + // flake8-async + (Flake8Async, "100") => Rule::BlockingHttpCallInAsyncFunction, + (Flake8Async, "101") => Rule::OpenSleepOrSubprocessInAsyncFunction, + (Flake8Async, "102") => Rule::BlockingOsCallInAsyncFunction, + // flake8-builtins (Flake8Builtins, "001") => Rule::BuiltinVariableShadowing, (Flake8Builtins, "002") => Rule::BuiltinArgumentShadowing, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 79543015017d7..e331c2d0a96b9 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -191,6 +191,10 @@ ruff_macros::register_rules!( rules::pylint::rules::UnexpectedSpecialMethodSignature, rules::pylint::rules::NestedMinMax, rules::pylint::rules::DuplicateBases, + // flake8-async + rules::flake8_async::rules::BlockingHttpCallInAsyncFunction, + rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction, + rules::flake8_async::rules::BlockingOsCallInAsyncFunction, // flake8-builtins rules::flake8_builtins::rules::BuiltinVariableShadowing, rules::flake8_builtins::rules::BuiltinArgumentShadowing, @@ -735,6 +739,9 @@ pub enum Linter { /// [flake8-annotations](https://pypi.org/project/flake8-annotations/) #[prefix = "ANN"] Flake8Annotations, + /// [flake8-async](https://pypi.org/project/flake8-async/) + #[prefix = "ASYNC"] + Flake8Async, /// [flake8-bandit](https://pypi.org/project/flake8-bandit/) #[prefix = "S"] Flake8Bandit, diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs new file mode 100644 index 0000000000000..8842526400e42 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -0,0 +1,28 @@ +//! Rules from [flake8-async](https://pypi.org/project/flake8-async/). +pub(crate) mod rules; + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::assert_messages; + use crate::registry::Rule; + use crate::settings::Settings; + use crate::test::test_path; + + #[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC100.py"); "ASYNC100")] + #[test_case(Rule::OpenSleepOrSubprocessInAsyncFunction, Path::new("ASYNC101.py"); "ASYNC101")] + #[test_case(Rule::BlockingOsCallInAsyncFunction, Path::new("ASYNC102.py"); "ASYNC102")] + fn rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_async").join(path).as_path(), + &Settings::for_rule(rule_code), + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } +} diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs new file mode 100644 index 0000000000000..2cc748974ecec --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -0,0 +1,224 @@ +use rustpython_parser::ast; +use rustpython_parser::ast::{Expr, ExprKind}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::context::Context; +use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks that async functions do not contain blocking HTTP calls. +/// +/// ## 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 making a blocking HTTP call, use an asynchronous HTTP client +/// library such as `aiohttp` or `httpx`. +/// +/// ## Example +/// ```python +/// async def fetch(): +/// urllib.request.urlopen("https://example.com/foo/bar").read() +/// ``` +/// +/// Use instead: +/// ```python +/// async def fetch(): +/// async with aiohttp.ClientSession() as session: +/// async with session.get("https://example.com/foo/bar") as resp: +/// ... +/// ``` +#[violation] +pub struct BlockingHttpCallInAsyncFunction; + +impl Violation for BlockingHttpCallInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call blocking HTTP methods") + } +} + +const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ + &["urllib", "request", "urlopen"], + &["httpx", "get"], + &["httpx", "post"], + &["httpx", "delete"], + &["httpx", "patch"], + &["httpx", "put"], + &["httpx", "head"], + &["httpx", "connect"], + &["httpx", "options"], + &["httpx", "trace"], + &["requests", "get"], + &["requests", "post"], + &["requests", "delete"], + &["requests", "patch"], + &["requests", "put"], + &["requests", "head"], + &["requests", "connect"], + &["requests", "options"], + &["requests", "trace"], +]; + +/// ASYNC100 +pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(&checker.ctx) { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { + if BLOCKING_HTTP_CALLS.contains(&call_path.as_slice()) { + checker + .diagnostics + .push(Diagnostic::new(BlockingHttpCallInAsyncFunction, func.range)); + } + } + } + } +} + +/// ## What it does +/// Checks that async functions do not contain calls to `open`, `time.sleep`, +/// or `subprocess` methods. +/// +/// ## Why is this bad? +/// Blocking an async function via a blocking call will block the entire +/// event loop, preventing it from executing other tasks while waiting for the +/// call to complete, negating the benefits of asynchronous programming. +/// +/// Instead of making a blocking call, use an equivalent asynchronous library +/// or function. +/// +/// ## Example +/// ```python +/// async def foo(): +/// time.sleep(1000) +/// ``` +/// +/// Use instead: +/// ```python +/// async def foo(): +/// await asyncio.sleep(1000) +/// ``` +#[violation] +pub struct OpenSleepOrSubprocessInAsyncFunction; + +impl Violation for OpenSleepOrSubprocessInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call `open`, `time.sleep`, or `subprocess` methods") + } +} + +const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ + &["", "open"], + &["time", "sleep"], + &["subprocess", "run"], + &["subprocess", "Popen"], + // Deprecated subprocess calls: + &["subprocess", "call"], + &["subprocess", "check_call"], + &["subprocess", "check_output"], + &["subprocess", "getoutput"], + &["subprocess", "getstatusoutput"], + &["os", "wait"], + &["os", "wait3"], + &["os", "wait4"], + &["os", "waitid"], + &["os", "waitpid"], +]; + +/// ASYNC101 +pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(&checker.ctx) { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { + if OPEN_SLEEP_OR_SUBPROCESS_CALL.contains(&call_path.as_slice()) { + checker.diagnostics.push(Diagnostic::new( + OpenSleepOrSubprocessInAsyncFunction, + func.range, + )); + } + } + } + } +} + +/// ## What it does +/// Checks that async functions do not contain calls to blocking synchronous +/// process calls via the `os` module. +/// +/// ## Why is this bad? +/// Blocking an async function via a blocking call will block the entire +/// event loop, preventing it from executing other tasks while waiting for the +/// call to complete, negating the benefits of asynchronous programming. +/// +/// Instead of making a blocking call, use an equivalent asynchronous library +/// or function. +/// +/// ## Example +/// ```python +/// async def foo(): +/// os.popen() +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// os.popen() +/// ``` +#[violation] +pub struct BlockingOsCallInAsyncFunction; + +impl Violation for BlockingOsCallInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call synchronous `os` methods") + } +} + +const UNSAFE_OS_METHODS: &[&[&str]] = &[ + &["os", "popen"], + &["os", "posix_spawn"], + &["os", "posix_spawnp"], + &["os", "spawnl"], + &["os", "spawnle"], + &["os", "spawnlp"], + &["os", "spawnlpe"], + &["os", "spawnv"], + &["os", "spawnve"], + &["os", "spawnvp"], + &["os", "spawnvpe"], + &["os", "system"], +]; + +/// ASYNC102 +pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(&checker.ctx) { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { + if UNSAFE_OS_METHODS.contains(&call_path.as_slice()) { + checker + .diagnostics + .push(Diagnostic::new(BlockingOsCallInAsyncFunction, func.range)); + } + } + } + } +} + +/// Return `true` if the [`Context`] is inside an async function definition. +fn in_async_function(context: &Context) -> bool { + context + .scopes() + .find_map(|scope| { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { + Some(*async_) + } else { + None + } + }) + .unwrap_or(false) +} diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap new file mode 100644 index 0000000000000..233e7b4c93431 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap @@ -0,0 +1,39 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +--- +ASYNC100.py:7:5: ASYNC100 Async functions should not call blocking HTTP methods + | +7 | async def foo(): +8 | urllib.request.urlopen("http://example.com/foo/bar").read() + | ^^^^^^^^^^^^^^^^^^^^^^ ASYNC100 + | + +ASYNC100.py:11:5: ASYNC100 Async functions should not call blocking HTTP methods + | +11 | async def foo(): +12 | requests.get() + | ^^^^^^^^^^^^ ASYNC100 + | + +ASYNC100.py:15:5: ASYNC100 Async functions should not call blocking HTTP methods + | +15 | async def foo(): +16 | httpx.get() + | ^^^^^^^^^ ASYNC100 + | + +ASYNC100.py:19:5: ASYNC100 Async functions should not call blocking HTTP methods + | +19 | async def foo(): +20 | requests.post() + | ^^^^^^^^^^^^^ ASYNC100 + | + +ASYNC100.py:23:5: ASYNC100 Async functions should not call blocking HTTP methods + | +23 | async def foo(): +24 | httpx.post() + | ^^^^^^^^^^ ASYNC100 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC101_ASYNC101.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC101_ASYNC101.py.snap new file mode 100644 index 0000000000000..d7af63516ebd6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC101_ASYNC101.py.snap @@ -0,0 +1,46 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +--- +ASYNC101.py:7:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +7 | async def foo(): +8 | open("foo") + | ^^^^ ASYNC101 + | + +ASYNC101.py:11:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +11 | async def foo(): +12 | time.sleep(1) + | ^^^^^^^^^^ ASYNC101 + | + +ASYNC101.py:15:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +15 | async def foo(): +16 | subprocess.run("foo") + | ^^^^^^^^^^^^^^ ASYNC101 + | + +ASYNC101.py:19:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +19 | async def foo(): +20 | subprocess.call("foo") + | ^^^^^^^^^^^^^^^ ASYNC101 + | + +ASYNC101.py:27:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +27 | async def foo(): +28 | os.wait4(10) + | ^^^^^^^^ ASYNC101 + | + +ASYNC101.py:31:5: ASYNC101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods + | +31 | async def foo(): +32 | os.wait(12) + | ^^^^^^^ ASYNC101 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC102_ASYNC102.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC102_ASYNC102.py.snap new file mode 100644 index 0000000000000..f48cd3405b017 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC102_ASYNC102.py.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +--- +ASYNC102.py:5:5: ASYNC102 Async functions should not call synchronous `os` methods + | +5 | async def foo(): +6 | os.popen() + | ^^^^^^^^ ASYNC102 + | + +ASYNC102.py:9:5: ASYNC102 Async functions should not call synchronous `os` methods + | + 9 | async def foo(): +10 | os.spawnl() + | ^^^^^^^^^ ASYNC102 + | + + diff --git a/crates/ruff/src/rules/mod.rs b/crates/ruff/src/rules/mod.rs index a2fc2cfe7509f..c0fcd71b516fe 100644 --- a/crates/ruff/src/rules/mod.rs +++ b/crates/ruff/src/rules/mod.rs @@ -2,6 +2,7 @@ pub mod eradicate; pub mod flake8_2020; pub mod flake8_annotations; +pub mod flake8_async; pub mod flake8_bandit; pub mod flake8_blind_except; pub mod flake8_boolean_trap; diff --git a/docs/faq.md b/docs/faq.md index 19caa945b59c1..8eb1f5942b305 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -33,6 +33,7 @@ natively, including: - [eradicate](https://pypi.org/project/eradicate/) - [flake8-2020](https://pypi.org/project/flake8-2020/) - [flake8-annotations](https://pypi.org/project/flake8-annotations/) +- [flake8-async](https://pypi.org/project/flake8-async) - [flake8-bandit](https://pypi.org/project/flake8-bandit/) ([#1646](https://github.com/charliermarsh/ruff/issues/1646)) - [flake8-blind-except](https://pypi.org/project/flake8-blind-except/) - [flake8-boolean-trap](https://pypi.org/project/flake8-boolean-trap/) @@ -133,6 +134,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [flake8-2020](https://pypi.org/project/flake8-2020/) - [flake8-annotations](https://pypi.org/project/flake8-annotations/) +- [flake8-async](https://pypi.org/project/flake8-async) - [flake8-bandit](https://pypi.org/project/flake8-bandit/) ([#1646](https://github.com/charliermarsh/ruff/issues/1646)) - [flake8-blind-except](https://pypi.org/project/flake8-blind-except/) - [flake8-boolean-trap](https://pypi.org/project/flake8-boolean-trap/) diff --git a/ruff.schema.json b/ruff.schema.json index dafa42567c1d2..8588e62880d91 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1521,6 +1521,12 @@ "ARG003", "ARG004", "ARG005", + "ASYNC", + "ASYNC1", + "ASYNC10", + "ASYNC100", + "ASYNC101", + "ASYNC102", "B", "B0", "B00",