From 2e674ef368525a04b89cd2c320fb143384b93603 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Wed, 10 May 2023 21:38:35 +0200 Subject: [PATCH 01/17] Add boilerplate --- README.md | 1 + crates/ruff/src/checkers/ast/mod.rs | 2 +- crates/ruff/src/codes.rs | 5 ++ crates/ruff/src/registry.rs | 8 ++ crates/ruff/src/rules/flake8_async/mod.rs | 25 +++++++ crates/ruff/src/rules/flake8_async/rules.rs | 82 +++++++++++++++++++++ crates/ruff/src/rules/mod.rs | 1 + ruff.schema.json | 5 +- 8 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_async/mod.rs create mode 100644 crates/ruff/src/rules/flake8_async/rules.rs 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/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 44c007f72f720..55682460436f1 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -42,7 +42,7 @@ 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_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, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index ef7926af69fe5..bfdff5128f278 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::BlockingHttpCallInsideAsyncDef, + (Flake8Async, "101") => Rule::OpenSleepOrSubprocessInsideAsyncDef, + (Flake8Async, "102") => Rule::UnsafeOsMethodInsideAsyncDef, + // 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..ed6bec828a075 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -191,6 +191,11 @@ ruff_macros::register_rules!( rules::pylint::rules::UnexpectedSpecialMethodSignature, rules::pylint::rules::NestedMinMax, rules::pylint::rules::DuplicateBases, + // flake8-async + // TODO: Fix import issues + rules::flake8_async::rules::BlockingHttpCallInsideAsyncDef, + rules::flake8_async::rules::OpenSleepOrSubprocessInsideAsyncDef, + rules::flake8_async::rules::UnsafeOsMethodInsideAsyncDef, // flake8-builtins rules::flake8_builtins::rules::BuiltinVariableShadowing, rules::flake8_builtins::rules::BuiltinArgumentShadowing, @@ -735,6 +740,9 @@ pub enum Linter { /// [flake8-annotations](https://pypi.org/project/flake8-annotations/) #[prefix = "ANN"] Flake8Annotations, + /// [flake8-async](https://pypi.org/project/flake8-async/) + #[prefix = "ASY"] + 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..3aef53d9d12ae --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -0,0 +1,25 @@ +//! 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 crate::registry::Rule; + use crate::test::test_path; + use crate::{assert_messages, settings}; + + fn rules(path: &Path) -> Result<()> { + let snapshot = path.to_string_lossy().into_owned(); + let diagnostics = test_path( + Path::new("flake8_async").join(path).as_path(), + &settings::Settings::for_rules(vec![Rule::BlockingHttpCallInsideAsyncDef, + Rule::OpenSleepOrSubprocessInsideAsyncDef, + Rule::UnsafeOsMethodInsideAsyncDef]), + )?; + 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..96ccc9050882d --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -0,0 +1,82 @@ +use ruff_diagnostics::Violation; +use crate::registry::{AsRule, Rule}; + +// TODO: Fix docstrings and improve messages + +/// ## What it does +/// Checks that async functions do not contain a blocking HTTP call +/// +/// ## Why is this bad? +/// TODO +/// +/// ## Example +/// TODO +/// +/// Use instead: +/// TODO +#[violation] +pub struct BlockingHttpCallInsideAsyncDef; + +impl Violation for BlockingHttpCallInsideAsyncDef { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call a blocking HTTP method") + } +} +/// ASY100 +// TODO: Implement +// pub fn blocking_http_call_inside_async_def {} + + +/// ## What it does +/// Checks that async functions do not contain a call to `open`, `time.sleep` or `subprocess` +/// methods +/// +/// ## Why is this bad? +/// TODO +/// +/// ## Example +/// TODO +/// +/// Use instead: +/// TODO +#[violation] +pub struct OpenSleepOrSubprocessInsideAsyncDef; + +impl Violation for OpenSleepOrSubprocessInsideAsyncDef { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Async functions should not contain a call to `open`, `time.sleep` or any \ + subprocess method" + ) + } +} + +/// ASY101 +// TODO: Implement +// pub fn open_sleep_or_subprocess_inside_async_def {} + +/// ## What it does +/// Checks that async functions do not contain a call to an unsafe `os` method +/// +/// ## Why is this bad? +/// TODO +/// +/// ## Example +/// TODO +/// +/// Use instead: +/// TODO +#[violation] +pub struct UnsafeOsMethodInsideAsyncDef; + +impl Violation for UnsafeOsMethodInsideAsyncDef { + fn message(&self) -> String { + format!("Async functions should not contain a call to unsafe `os` methods") + } +} + +/// ASY102 +// TODO: Implement +// pub fn unsafe_os_method_inside_async_def {} 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/ruff.schema.json b/ruff.schema.json index adf5566311b56..fab875bd705c7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1519,6 +1519,9 @@ "ARG003", "ARG004", "ARG005", + "ASY100", + "ASY101", + "ASY102", "B", "B0", "B00", @@ -2423,4 +2426,4 @@ "type": "string" } } -} \ No newline at end of file +} From 4c30ee37024f0ac45d8170e7d52ebc62c6fa101d Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 11 May 2023 09:13:14 +0200 Subject: [PATCH 02/17] Add fixtures and tests, update ast/mod.rs --- .../test/fixtures/flake8_async/ASY100.py | 6 +++ .../test/fixtures/flake8_async/ASY101.py | 18 +++++++++ .../test/fixtures/flake8_async/ASY102.py | 13 +++++++ crates/ruff/src/checkers/ast/mod.rs | 11 ++++++ crates/ruff/src/registry.rs | 1 - crates/ruff/src/rules/flake8_async/mod.rs | 16 +++++--- crates/ruff/src/rules/flake8_async/rules.rs | 39 +++++++++++++++++-- ...e8_async__tests__ASY100_ASY100.py.snap.new | 19 +++++++++ ...e8_async__tests__ASY101_ASY101.py.snap.new | 33 ++++++++++++++++ ...e8_async__tests__ASY102_ASY102.py.snap.new | 26 +++++++++++++ test.py | 2 + 11 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_async/ASY100.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_async/ASY101.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_async/ASY102.py create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new create mode 100644 test.py diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py new file mode 100644 index 0000000000000..5ec091896f1dc --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py @@ -0,0 +1,6 @@ +import urllib.request + + +async def foo(): + urllib.request.urlopen("http://example.com/foo/bar").read() + diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py new file mode 100644 index 0000000000000..3b946ed0a9717 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py @@ -0,0 +1,18 @@ +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") diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py new file mode 100644 index 0000000000000..6f366d0c182b3 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py @@ -0,0 +1,13 @@ +import os + + +async def foo(): + os.wait() + + +async def foo(): + os.popen("foo") + + +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 55682460436f1..4b61b221691fd 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2584,6 +2584,17 @@ where pyupgrade::rules::use_pep604_isinstance(self, expr, func, args); } + // flake8-async + if self.settings.rules.enabled(Rule::BlockingHttpCallInsideAsyncDef) { + flake8_async::rules::blocking_http_call_inside_async_def(self, func); + } + if self.settings.rules.enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) { + flake8_async::rules::open_sleep_or_subprocess_inside_async_def(self, func); + } + if self.settings.rules.enabled(Rule::UnsafeOsMethodInsideAsyncDef) { + flake8_async::rules::unsafe_os_method_inside_async_def(self, func); + } + // flake8-print if self .settings diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index ed6bec828a075..2702769860f73 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -192,7 +192,6 @@ ruff_macros::register_rules!( rules::pylint::rules::NestedMinMax, rules::pylint::rules::DuplicateBases, // flake8-async - // TODO: Fix import issues rules::flake8_async::rules::BlockingHttpCallInsideAsyncDef, rules::flake8_async::rules::OpenSleepOrSubprocessInsideAsyncDef, rules::flake8_async::rules::UnsafeOsMethodInsideAsyncDef, diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs index 3aef53d9d12ae..f644449ebc266 100644 --- a/crates/ruff/src/rules/flake8_async/mod.rs +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -5,19 +5,23 @@ pub(crate) mod rules; mod tests { use std::path::Path; + use crate::assert_messages; use anyhow::Result; + use test_case::test_case; + use crate::registry::Rule; + use crate::settings::Settings; use crate::test::test_path; - use crate::{assert_messages, settings}; - fn rules(path: &Path) -> Result<()> { - let snapshot = path.to_string_lossy().into_owned(); + #[test_case(Rule::BlockingHttpCallInsideAsyncDef, Path::new("ASY100.py"); "ASY100")] + #[test_case(Rule::OpenSleepOrSubprocessInsideAsyncDef, Path::new("ASY101.py"); "ASY101")] + #[test_case(Rule::UnsafeOsMethodInsideAsyncDef, Path::new("ASY102.py"); "ASY102")] + 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::Settings::for_rules(vec![Rule::BlockingHttpCallInsideAsyncDef, - Rule::OpenSleepOrSubprocessInsideAsyncDef, - Rule::UnsafeOsMethodInsideAsyncDef]), + &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 index 96ccc9050882d..b22cd69dcc273 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -1,4 +1,8 @@ -use ruff_diagnostics::Violation; +use rustpython_parser::ast::{Expr, Keyword}; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::scope::FunctionDef; +use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; // TODO: Fix docstrings and improve messages @@ -23,9 +27,19 @@ impl Violation for BlockingHttpCallInsideAsyncDef { format!("Async functions should not call a blocking HTTP method") } } + /// ASY100 // TODO: Implement -// pub fn blocking_http_call_inside_async_def {} +pub fn blocking_http_call_inside_async_def(checker: &mut Checker, func: &Expr) { + + let diagnostic = Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range()); + + if !checker.settings.rules.enabled(diagnostic.kind.rule()) { + return; + } + + checker.diagnostics.push(diagnostic); +} /// ## What it does @@ -55,7 +69,15 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { /// ASY101 // TODO: Implement -// pub fn open_sleep_or_subprocess_inside_async_def {} +pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, func: &Expr) { + let diagnostic = Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, func.range()); + + if !checker.settings.rules.enabled(diagnostic.kind.rule()) { + return; + } + + checker.diagnostics.push(diagnostic); +} /// ## What it does /// Checks that async functions do not contain a call to an unsafe `os` method @@ -72,6 +94,7 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { pub struct UnsafeOsMethodInsideAsyncDef; impl Violation for UnsafeOsMethodInsideAsyncDef { + #[derive_message_formats] fn message(&self) -> String { format!("Async functions should not contain a call to unsafe `os` methods") } @@ -79,4 +102,12 @@ impl Violation for UnsafeOsMethodInsideAsyncDef { /// ASY102 // TODO: Implement -// pub fn unsafe_os_method_inside_async_def {} +pub fn unsafe_os_method_inside_async_def(checker: &mut Checker, func: &Expr) { + let diagnostic = Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range()); + + if !checker.settings.rules.enabled(diagnostic.kind.rule()) { + return; + } + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new new file mode 100644 index 0000000000000..9ce17d26df55f --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new @@ -0,0 +1,19 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +assertion_line: 26 +--- +ASY100.py:5:5: ASY100 Async functions should not call a blocking HTTP method + | +5 | async def foo(): +6 | urllib.request.urlopen("http://example.com/foo/bar").read() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ASY100 + | + +ASY100.py:5:5: ASY100 Async functions should not call a blocking HTTP method + | +5 | async def foo(): +6 | urllib.request.urlopen("http://example.com/foo/bar").read() + | ^^^^^^^^^^^^^^^^^^^^^^ ASY100 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new new file mode 100644 index 0000000000000..9e2bfe6d078b4 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new @@ -0,0 +1,33 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +assertion_line: 26 +--- +ASY101.py:6:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +6 | async def foo(): +7 | open("foo") + | ^^^^ ASY101 + | + +ASY101.py:10:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +10 | async def foo(): +11 | time.sleep(1) + | ^^^^^^^^^^ ASY101 + | + +ASY101.py:14:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +14 | async def foo(): +15 | subprocess.run("foo") + | ^^^^^^^^^^^^^^ ASY101 + | + +ASY101.py:18:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +18 | async def foo(): +19 | subprocess.call("foo") + | ^^^^^^^^^^^^^^^ ASY101 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new new file mode 100644 index 0000000000000..951dca07d5117 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new @@ -0,0 +1,26 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +assertion_line: 26 +--- +ASY102.py:5:5: ASY102 Async functions should not contain a call to unsafe `os` methods + | +5 | async def foo(): +6 | os.wait() + | ^^^^^^^ ASY102 + | + +ASY102.py:9:5: ASY102 Async functions should not contain a call to unsafe `os` methods + | + 9 | async def foo(): +10 | os.popen("foo") + | ^^^^^^^^ ASY102 + | + +ASY102.py:13:5: ASY102 Async functions should not contain a call to unsafe `os` methods + | +13 | async def foo(): +14 | os.fspath("foo") + | ^^^^^^^^^ ASY102 + | + + diff --git a/test.py b/test.py new file mode 100644 index 0000000000000..a4b11f35691af --- /dev/null +++ b/test.py @@ -0,0 +1,2 @@ +async def some_func(): + print("hallo") \ No newline at end of file From 7b09ab090465c2ed6c637489fb37d08a99d84279 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 11 May 2023 09:36:59 +0200 Subject: [PATCH 03/17] Sort imports, attempt open_sleep_or_subprocess func --- crates/ruff/src/rules/flake8_async/mod.rs | 3 +-- crates/ruff/src/rules/flake8_async/rules.rs | 24 +++++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs index f644449ebc266..d9f2f86a495fb 100644 --- a/crates/ruff/src/rules/flake8_async/mod.rs +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -5,11 +5,10 @@ pub(crate) mod rules; mod tests { use std::path::Path; - use crate::assert_messages; 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; diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index b22cd69dcc273..f92fdc518db22 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -1,9 +1,11 @@ -use rustpython_parser::ast::{Expr, Keyword}; +use rustpython_parser::ast::{Expr, ExprKind}; + use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_semantic::scope::FunctionDef; +use ruff_python_ast::call_path::collect_call_path; + use crate::checkers::ast::Checker; -use crate::registry::{AsRule, Rule}; +use crate::registry::{AsRule}; // TODO: Fix docstrings and improve messages @@ -69,14 +71,18 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { /// ASY101 // TODO: Implement -pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, func: &Expr) { - let diagnostic = Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, func.range()); - - if !checker.settings.rules.enabled(diagnostic.kind.rule()) { - return; +pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { + if let ExprKind::Call { func, .. } = &expr.node { + if let Some(call_path) = collect_call_path(func) { + if call_path.as_slice() == ["sleep"] || + call_path.as_slice() == ["open"] || + call_path.as_slice() == ["subprocess.call"] + { + checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); + } + } } - checker.diagnostics.push(diagnostic); } /// ## What it does From bed1cb3e9d82a518c7685c69591bfbda8b84ab0e Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 11 May 2023 09:41:28 +0200 Subject: [PATCH 04/17] Delete test.py --- test.py | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 test.py diff --git a/test.py b/test.py deleted file mode 100644 index a4b11f35691af..0000000000000 --- a/test.py +++ /dev/null @@ -1,2 +0,0 @@ -async def some_func(): - print("hallo") \ No newline at end of file From e0bd1924a1643c5153045e4f9b721f1f1fb8435f Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 11 May 2023 18:15:12 +0200 Subject: [PATCH 05/17] Pass in expr instead of func in ast/mod.rs --- crates/ruff/src/checkers/ast/mod.rs | 6 +++--- crates/ruff/src/rules/flake8_async/rules.rs | 14 +++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 4b61b221691fd..eb2c4acd9a45c 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2586,13 +2586,13 @@ where // flake8-async if self.settings.rules.enabled(Rule::BlockingHttpCallInsideAsyncDef) { - flake8_async::rules::blocking_http_call_inside_async_def(self, func); + flake8_async::rules::blocking_http_call_inside_async_def(self, expr); } if self.settings.rules.enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) { - flake8_async::rules::open_sleep_or_subprocess_inside_async_def(self, func); + flake8_async::rules::open_sleep_or_subprocess_inside_async_def(self, expr); } if self.settings.rules.enabled(Rule::UnsafeOsMethodInsideAsyncDef) { - flake8_async::rules::unsafe_os_method_inside_async_def(self, func); + flake8_async::rules::unsafe_os_method_inside_async_def(self, expr); } // flake8-print diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index f92fdc518db22..7b4872a5bfdb7 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -33,7 +33,6 @@ impl Violation for BlockingHttpCallInsideAsyncDef { /// ASY100 // TODO: Implement pub fn blocking_http_call_inside_async_def(checker: &mut Checker, func: &Expr) { - let diagnostic = Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range()); if !checker.settings.rules.enabled(diagnostic.kind.rule()) { @@ -74,15 +73,12 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { if let ExprKind::Call { func, .. } = &expr.node { if let Some(call_path) = collect_call_path(func) { - if call_path.as_slice() == ["sleep"] || - call_path.as_slice() == ["open"] || - call_path.as_slice() == ["subprocess.call"] - { - checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); - } - } + if call_path.as_slice() == ["time", "sleep"] + { + checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); + } + } } - } /// ## What it does From fda63777724abaf6bf19f180278cd793d9bcdb4d Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 11 May 2023 18:22:39 +0200 Subject: [PATCH 06/17] Add async check to sleep rule --- crates/ruff/src/rules/flake8_async/rules.rs | 23 ++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 7b4872a5bfdb7..552385e11f2a0 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -3,6 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; +use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; use crate::registry::{AsRule}; @@ -71,11 +72,23 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { /// ASY101 // TODO: Implement pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { - if let ExprKind::Call { func, .. } = &expr.node { - if let Some(call_path) = collect_call_path(func) { - if call_path.as_slice() == ["time", "sleep"] - { - checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); + if checker + .ctx + .scopes() + .find_map(|scope| { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { + Some(*async_) + } else { + None + } + }) + .unwrap_or(false) { + if let ExprKind::Call { func, .. } = &expr.node { + if let Some(call_path) = collect_call_path(func) { + if call_path.as_slice() == ["time", "sleep"] + { + checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); + } } } } From 1506742ecdcaf8f27c29ab9a87e3980617f1fc7d Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sun, 14 May 2023 19:08:19 +0200 Subject: [PATCH 07/17] Implement asuync check logic, expand tests, add docstrings, fix snapshots --- .../test/fixtures/flake8_async/ASY100.py | 17 ++ .../test/fixtures/flake8_async/ASY101.py | 13 ++ .../test/fixtures/flake8_async/ASY102.py | 4 +- crates/ruff/src/rules/flake8_async/rules.rs | 174 ++++++++++++++---- ...flake8_async__tests__ASY100_ASY100.py.snap | 39 ++++ ...e8_async__tests__ASY100_ASY100.py.snap.new | 19 -- ...flake8_async__tests__ASY101_ASY101.py.snap | 46 +++++ ...e8_async__tests__ASY101_ASY101.py.snap.new | 33 ---- ...lake8_async__tests__ASY102_ASY102.py.snap} | 14 +- 9 files changed, 263 insertions(+), 96 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap delete mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap delete mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new rename crates/ruff/src/rules/flake8_async/snapshots/{ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new => ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap} (55%) diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py index 5ec091896f1dc..532273a7b4676 100644 --- a/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py @@ -1,6 +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/ASY101.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py index 3b946ed0a9717..32fbaeb9aabbe 100644 --- a/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py @@ -1,3 +1,4 @@ +import os import subprocess import time @@ -16,3 +17,15 @@ async def 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/ASY102.py b/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py index 6f366d0c182b3..7912bcc9decab 100644 --- a/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py +++ b/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py @@ -2,11 +2,11 @@ async def foo(): - os.wait() + os.popen() async def foo(): - os.popen("foo") + os.spawnl() async def foo(): diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 552385e11f2a0..189b3bd954de3 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -6,21 +6,37 @@ use ruff_python_ast::call_path::collect_call_path; use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; -use crate::registry::{AsRule}; -// TODO: Fix docstrings and improve messages + +struct ViolatingCalls<'a> { + members: &'a [&'a [&'a str]], +} + +impl<'a> ViolatingCalls<'a> { + pub const fn new(members: &'a [&'a [&'a str]]) -> Self { + Self { members } + } +} /// ## What it does /// Checks that async functions do not contain a blocking HTTP call /// /// ## Why is this bad? -/// TODO +/// Because blocking an async function makes the asynchronous nature of the function useless and +/// could confuse a reader or user /// /// ## Example -/// TODO +/// async def foo(): +/// urllib.request.urlopen("http://example.com/foo/bar").read() /// /// Use instead: -/// TODO +/// Many options, but e.g.: +/// +/// async def foo(): +/// async with aiohttp.ClientSession() as session: +/// async with session.get("http://example.com/foo/bar") as resp: +/// result = await resp.json() +/// print(result) #[violation] pub struct BlockingHttpCallInsideAsyncDef; @@ -31,31 +47,65 @@ impl Violation for BlockingHttpCallInsideAsyncDef { } } -/// ASY100 -// TODO: Implement -pub fn blocking_http_call_inside_async_def(checker: &mut Checker, func: &Expr) { - let diagnostic = Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range()); +// TODO: Complete +const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ + ViolatingCalls::new( + &[ + &["urllib", "request", "urlopen"], + &["httpx", "get"], + &["httpx", "post"], + &["httpx", "put"], + &["httpx", "patch"], + &["httpx", "delete"], + &["requests", "get"], + &["requests", "post"], + &["requests", "delete"], + &["requests", "patch"], + &["requests", "put"], + ] + )]; - if !checker.settings.rules.enabled(diagnostic.kind.rule()) { - return; +/// ASY100 +pub fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { + if checker + .ctx + .scopes() + .find_map(|scope| { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { + Some(*async_) + } else { + None + } + }) + .unwrap_or(false) { + if let ExprKind::Call { func, .. } = &expr.node { + if let Some(call_path) = collect_call_path(func) { + for v_call in BLOCKING_HTTP_CALLS { + for member in v_call.members { + if call_path.as_slice() == *member { + checker.diagnostics.push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); + } + } + } + } + } } - - checker.diagnostics.push(diagnostic); } - /// ## What it does /// Checks that async functions do not contain a call to `open`, `time.sleep` or `subprocess` /// methods /// /// ## Why is this bad? -/// TODO +/// Calling these functions in an async process can lead to unexpected behaviour /// /// ## Example -/// TODO +/// async def foo(): +/// time.sleep(1000) /// /// Use instead: -/// TODO +/// def foo(): +/// time.sleep(1000) #[violation] pub struct OpenSleepOrSubprocessInsideAsyncDef; @@ -69,8 +119,28 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { } } +const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ + ViolatingCalls::new( + &[ + &["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"], + ] + )]; + /// ASY101 -// TODO: Implement pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { if checker .ctx @@ -85,9 +155,13 @@ pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &E .unwrap_or(false) { if let ExprKind::Call { func, .. } = &expr.node { if let Some(call_path) = collect_call_path(func) { - if call_path.as_slice() == ["time", "sleep"] - { - checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, expr.range)); + for v_call in OPEN_SLEEP_OR_SUBPROCESS_CALL { + for member in v_call.members { + if call_path.as_slice() == *member + { + checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, func.range)); + } + } } } } @@ -98,13 +172,16 @@ pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &E /// Checks that async functions do not contain a call to an unsafe `os` method /// /// ## Why is this bad? -/// TODO +/// Calling unsafe 'os' methods can lead to unpredictable behaviour mid process and/or state +/// changes /// /// ## Example -/// TODO +/// async def foo(): +/// os.popen() /// /// Use instead: -/// TODO +/// def foo(): +/// os.popen() #[violation] pub struct UnsafeOsMethodInsideAsyncDef; @@ -115,14 +192,49 @@ impl Violation for UnsafeOsMethodInsideAsyncDef { } } +const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ + ViolatingCalls::new( + &[ + &["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"] + ] + )]; + /// ASY102 // TODO: Implement -pub fn unsafe_os_method_inside_async_def(checker: &mut Checker, func: &Expr) { - let diagnostic = Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range()); - - if !checker.settings.rules.enabled(diagnostic.kind.rule()) { - return; +pub fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { + if checker + .ctx + .scopes() + .find_map(|scope| { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { + Some(*async_) + } else { + None + } + }) + .unwrap_or(false) { + if let ExprKind::Call { func, .. } = &expr.node { + if let Some(call_path) = collect_call_path(func) { + for v_call in UNSAFE_OS_METHODS { + for member in v_call.members { + if call_path.as_slice() == *member + { + checker.diagnostics.push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); + } + } + } + } + } } - - checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap new file mode 100644 index 0000000000000..265c0179033e2 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap @@ -0,0 +1,39 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +--- +ASY100.py:7:5: ASY100 Async functions should not call a blocking HTTP method + | +7 | async def foo(): +8 | urllib.request.urlopen("http://example.com/foo/bar").read() + | ^^^^^^^^^^^^^^^^^^^^^^ ASY100 + | + +ASY100.py:11:5: ASY100 Async functions should not call a blocking HTTP method + | +11 | async def foo(): +12 | requests.get() + | ^^^^^^^^^^^^ ASY100 + | + +ASY100.py:15:5: ASY100 Async functions should not call a blocking HTTP method + | +15 | async def foo(): +16 | httpx.get() + | ^^^^^^^^^ ASY100 + | + +ASY100.py:19:5: ASY100 Async functions should not call a blocking HTTP method + | +19 | async def foo(): +20 | requests.post() + | ^^^^^^^^^^^^^ ASY100 + | + +ASY100.py:23:5: ASY100 Async functions should not call a blocking HTTP method + | +23 | async def foo(): +24 | httpx.post() + | ^^^^^^^^^^ ASY100 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new deleted file mode 100644 index 9ce17d26df55f..0000000000000 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap.new +++ /dev/null @@ -1,19 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_async/mod.rs -assertion_line: 26 ---- -ASY100.py:5:5: ASY100 Async functions should not call a blocking HTTP method - | -5 | async def foo(): -6 | urllib.request.urlopen("http://example.com/foo/bar").read() - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ASY100 - | - -ASY100.py:5:5: ASY100 Async functions should not call a blocking HTTP method - | -5 | async def foo(): -6 | urllib.request.urlopen("http://example.com/foo/bar").read() - | ^^^^^^^^^^^^^^^^^^^^^^ ASY100 - | - - diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap new file mode 100644 index 0000000000000..cedf1237509f8 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap @@ -0,0 +1,46 @@ +--- +source: crates/ruff/src/rules/flake8_async/mod.rs +--- +ASY101.py:7:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +7 | async def foo(): +8 | open("foo") + | ^^^^ ASY101 + | + +ASY101.py:11:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +11 | async def foo(): +12 | time.sleep(1) + | ^^^^^^^^^^ ASY101 + | + +ASY101.py:15:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +15 | async def foo(): +16 | subprocess.run("foo") + | ^^^^^^^^^^^^^^ ASY101 + | + +ASY101.py:19:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +19 | async def foo(): +20 | subprocess.call("foo") + | ^^^^^^^^^^^^^^^ ASY101 + | + +ASY101.py:27:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +27 | async def foo(): +28 | os.wait4(10) + | ^^^^^^^^ ASY101 + | + +ASY101.py:31:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method + | +31 | async def foo(): +32 | os.wait(12) + | ^^^^^^^ ASY101 + | + + diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new deleted file mode 100644 index 9e2bfe6d078b4..0000000000000 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap.new +++ /dev/null @@ -1,33 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_async/mod.rs -assertion_line: 26 ---- -ASY101.py:6:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method - | -6 | async def foo(): -7 | open("foo") - | ^^^^ ASY101 - | - -ASY101.py:10:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method - | -10 | async def foo(): -11 | time.sleep(1) - | ^^^^^^^^^^ ASY101 - | - -ASY101.py:14:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method - | -14 | async def foo(): -15 | subprocess.run("foo") - | ^^^^^^^^^^^^^^ ASY101 - | - -ASY101.py:18:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method - | -18 | async def foo(): -19 | subprocess.call("foo") - | ^^^^^^^^^^^^^^^ ASY101 - | - - diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap similarity index 55% rename from crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new rename to crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap index 951dca07d5117..6c1d8954fbe3c 100644 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap.new +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap @@ -1,25 +1,17 @@ --- source: crates/ruff/src/rules/flake8_async/mod.rs -assertion_line: 26 --- ASY102.py:5:5: ASY102 Async functions should not contain a call to unsafe `os` methods | 5 | async def foo(): -6 | os.wait() - | ^^^^^^^ ASY102 +6 | os.popen() + | ^^^^^^^^ ASY102 | ASY102.py:9:5: ASY102 Async functions should not contain a call to unsafe `os` methods | 9 | async def foo(): -10 | os.popen("foo") - | ^^^^^^^^ ASY102 - | - -ASY102.py:13:5: ASY102 Async functions should not contain a call to unsafe `os` methods - | -13 | async def foo(): -14 | os.fspath("foo") +10 | os.spawnl() | ^^^^^^^^^ ASY102 | From 0f61694f1870cb950c1ea7dc175e74497b1d6185 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sun, 14 May 2023 19:12:11 +0200 Subject: [PATCH 08/17] Expand blocking HTTP methods const --- crates/ruff/src/rules/flake8_async/rules.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 189b3bd954de3..d1a519a6ed8de 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -47,21 +47,28 @@ impl Violation for BlockingHttpCallInsideAsyncDef { } } -// TODO: Complete const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ ViolatingCalls::new( &[ &["urllib", "request", "urlopen"], &["httpx", "get"], &["httpx", "post"], - &["httpx", "put"], - &["httpx", "patch"], &["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"], ] )]; From bc23bcc2059bc468175dc0e30dc8532a54fadcbc Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sun, 14 May 2023 19:21:47 +0200 Subject: [PATCH 09/17] Cleanup, generate-all --- crates/ruff/src/rules/flake8_async/rules.rs | 16 ++++++++-------- ruff.schema.json | 5 ++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index d1a519a6ed8de..167f7bfc37f0d 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -1,3 +1,4 @@ +use rustpython_parser::ast; use rustpython_parser::ast::{Expr, ExprKind}; use ruff_diagnostics::{Diagnostic, Violation}; @@ -13,7 +14,7 @@ struct ViolatingCalls<'a> { } impl<'a> ViolatingCalls<'a> { - pub const fn new(members: &'a [&'a [&'a str]]) -> Self { + pub(crate) const fn new(members: &'a [&'a [&'a str]]) -> Self { Self { members } } } @@ -73,7 +74,7 @@ const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ )]; /// ASY100 -pub fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -85,7 +86,7 @@ pub fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { } }) .unwrap_or(false) { - if let ExprKind::Call { func, .. } = &expr.node { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in BLOCKING_HTTP_CALLS { for member in v_call.members { @@ -148,7 +149,7 @@ const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ )]; /// ASY101 -pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -160,7 +161,7 @@ pub fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &E } }) .unwrap_or(false) { - if let ExprKind::Call { func, .. } = &expr.node { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in OPEN_SLEEP_OR_SUBPROCESS_CALL { for member in v_call.members { @@ -218,8 +219,7 @@ const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ )]; /// ASY102 -// TODO: Implement -pub fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -231,7 +231,7 @@ pub fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { } }) .unwrap_or(false) { - if let ExprKind::Call { func, .. } = &expr.node { + if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in UNSAFE_OS_METHODS { for member in v_call.members { diff --git a/ruff.schema.json b/ruff.schema.json index fab875bd705c7..61f8cb00c74e8 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1519,6 +1519,9 @@ "ARG003", "ARG004", "ARG005", + "ASY", + "ASY1", + "ASY10", "ASY100", "ASY101", "ASY102", @@ -2426,4 +2429,4 @@ "type": "string" } } -} +} \ No newline at end of file From b620bbdfc895df1dd5319ebd47d3c37ea2f72d17 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sun, 14 May 2023 19:24:34 +0200 Subject: [PATCH 10/17] cargo fmt --all --- crates/ruff/src/checkers/ast/mod.rs | 24 +++- crates/ruff/src/rules/flake8_async/rules.rs | 142 ++++++++++---------- 2 files changed, 88 insertions(+), 78 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index eb2c4acd9a45c..10a3f6943c061 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_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_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, @@ -2585,13 +2585,25 @@ where } // flake8-async - if self.settings.rules.enabled(Rule::BlockingHttpCallInsideAsyncDef) { + if self + .settings + .rules + .enabled(Rule::BlockingHttpCallInsideAsyncDef) + { flake8_async::rules::blocking_http_call_inside_async_def(self, expr); } - if self.settings.rules.enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) { + if self + .settings + .rules + .enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) + { flake8_async::rules::open_sleep_or_subprocess_inside_async_def(self, expr); } - if self.settings.rules.enabled(Rule::UnsafeOsMethodInsideAsyncDef) { + if self + .settings + .rules + .enabled(Rule::UnsafeOsMethodInsideAsyncDef) + { flake8_async::rules::unsafe_os_method_inside_async_def(self, expr); } diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 167f7bfc37f0d..d520f2326b33b 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -8,7 +8,6 @@ use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; - struct ViolatingCalls<'a> { members: &'a [&'a [&'a str]], } @@ -48,30 +47,27 @@ impl Violation for BlockingHttpCallInsideAsyncDef { } } -const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ - ViolatingCalls::new( - &[ - &["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"], - ] - )]; +const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ + &["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"], +])]; /// ASY100 pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { @@ -85,13 +81,16 @@ pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: & None } }) - .unwrap_or(false) { + .unwrap_or(false) + { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in BLOCKING_HTTP_CALLS { for member in v_call.members { if call_path.as_slice() == *member { - checker.diagnostics.push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); + checker + .diagnostics + .push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); } } } @@ -127,26 +126,23 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { } } -const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ - ViolatingCalls::new( - &[ - &["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"], - ] - )]; +const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ViolatingCalls::new(&[ + &["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"], +])]; /// ASY101 pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { @@ -160,14 +156,17 @@ pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, e None } }) - .unwrap_or(false) { + .unwrap_or(false) + { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in OPEN_SLEEP_OR_SUBPROCESS_CALL { for member in v_call.members { - if call_path.as_slice() == *member - { - checker.diagnostics.push(Diagnostic::new(OpenSleepOrSubprocessInsideAsyncDef, func.range)); + if call_path.as_slice() == *member { + checker.diagnostics.push(Diagnostic::new( + OpenSleepOrSubprocessInsideAsyncDef, + func.range, + )); } } } @@ -200,23 +199,20 @@ impl Violation for UnsafeOsMethodInsideAsyncDef { } } -const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ - ViolatingCalls::new( - &[ - &["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"] - ] - )]; +const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ + &["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"], +])]; /// ASY102 pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { @@ -230,14 +226,16 @@ pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Ex None } }) - .unwrap_or(false) { + .unwrap_or(false) + { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = collect_call_path(func) { for v_call in UNSAFE_OS_METHODS { for member in v_call.members { - if call_path.as_slice() == *member - { - checker.diagnostics.push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); + if call_path.as_slice() == *member { + checker + .diagnostics + .push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); } } } From 14666ab762cacb5d7213efd0a5a63d187d40d0eb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:17:24 -0400 Subject: [PATCH 11/17] Use resolve_call_path --- crates/ruff/src/rules/flake8_async/rules.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index d520f2326b33b..310ce152e3d65 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -3,7 +3,6 @@ use rustpython_parser::ast::{Expr, ExprKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::collect_call_path; use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; @@ -84,7 +83,7 @@ pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: & .unwrap_or(false) { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { - if let Some(call_path) = collect_call_path(func) { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { for v_call in BLOCKING_HTTP_CALLS { for member in v_call.members { if call_path.as_slice() == *member { @@ -127,7 +126,7 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { } const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ViolatingCalls::new(&[ - &["open"], + &["", "open"], &["time", "sleep"], &["subprocess", "run"], &["subprocess", "Popen"], @@ -159,7 +158,7 @@ pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, e .unwrap_or(false) { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { - if let Some(call_path) = collect_call_path(func) { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { for v_call in OPEN_SLEEP_OR_SUBPROCESS_CALL { for member in v_call.members { if call_path.as_slice() == *member { @@ -229,7 +228,7 @@ pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Ex .unwrap_or(false) { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { - if let Some(call_path) = collect_call_path(func) { + if let Some(call_path) = checker.ctx.resolve_call_path(func) { for v_call in UNSAFE_OS_METHODS { for member in v_call.members { if call_path.as_slice() == *member { From 15e4e8b8a6970b05da9073f10fbbf3775326c728 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:36:49 -0400 Subject: [PATCH 12/17] Tweak docs --- crates/ruff/src/rules/flake8_async/rules.rs | 72 ++++++++++++------- ...flake8_async__tests__ASY100_ASY100.py.snap | 10 +-- ...flake8_async__tests__ASY101_ASY101.py.snap | 12 ++-- ...flake8_async__tests__ASY102_ASY102.py.snap | 4 +- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 310ce152e3d65..75a91ee2665a1 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -18,31 +18,36 @@ impl<'a> ViolatingCalls<'a> { } /// ## What it does -/// Checks that async functions do not contain a blocking HTTP call +/// Checks that async functions do not contain blocking HTTP calls. /// /// ## Why is this bad? -/// Because blocking an async function makes the asynchronous nature of the function useless and -/// could confuse a reader or user +/// 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 -/// async def foo(): -/// urllib.request.urlopen("http://example.com/foo/bar").read() +/// ```python +/// async def fetch(): +/// urllib.request.urlopen("https://example.com/foo/bar").read() +/// ``` /// /// Use instead: -/// Many options, but e.g.: -/// -/// async def foo(): -/// async with aiohttp.ClientSession() as session: -/// async with session.get("http://example.com/foo/bar") as resp: -/// result = await resp.json() -/// print(result) +/// ```python +/// async def fetch(): +/// async with aiohttp.ClientSession() as session: +/// async with session.get("https://example.com/foo/bar") as resp: +/// ... +/// ``` #[violation] pub struct BlockingHttpCallInsideAsyncDef; impl Violation for BlockingHttpCallInsideAsyncDef { #[derive_message_formats] fn message(&self) -> String { - format!("Async functions should not call a blocking HTTP method") + format!("Async functions should not call blocking HTTP methods") } } @@ -99,29 +104,35 @@ pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: & } /// ## What it does -/// Checks that async functions do not contain a call to `open`, `time.sleep` or `subprocess` -/// methods +/// Checks that async functions do not contain calls to `open`, `time.sleep`, +/// or `subprocess` methods. /// /// ## Why is this bad? -/// Calling these functions in an async process can lead to unexpected behaviour +/// 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: -/// def foo(): -/// time.sleep(1000) +/// ```python +/// async def foo(): +/// await asyncio.sleep(1000) +/// ``` #[violation] pub struct OpenSleepOrSubprocessInsideAsyncDef; impl Violation for OpenSleepOrSubprocessInsideAsyncDef { #[derive_message_formats] fn message(&self) -> String { - format!( - "Async functions should not contain a call to `open`, `time.sleep` or any \ - subprocess method" - ) + format!("Async functions should not call `open`, `time.sleep`, or `subprocess` methods") } } @@ -175,26 +186,35 @@ pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, e } /// ## What it does -/// Checks that async functions do not contain a call to an unsafe `os` method +/// Checks that async functions do not contain calls to blocking synchronous +/// process calls via the `os` module. /// /// ## Why is this bad? -/// Calling unsafe 'os' methods can lead to unpredictable behaviour mid process and/or state -/// changes +/// 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 UnsafeOsMethodInsideAsyncDef; impl Violation for UnsafeOsMethodInsideAsyncDef { #[derive_message_formats] fn message(&self) -> String { - format!("Async functions should not contain a call to unsafe `os` methods") + format!("Async functions should not call synchronous `os` methods") } } diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap index 265c0179033e2..fdc2093154981 100644 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap @@ -1,35 +1,35 @@ --- source: crates/ruff/src/rules/flake8_async/mod.rs --- -ASY100.py:7:5: ASY100 Async functions should not call a blocking HTTP method +ASY100.py:7:5: ASY100 Async functions should not call blocking HTTP methods | 7 | async def foo(): 8 | urllib.request.urlopen("http://example.com/foo/bar").read() | ^^^^^^^^^^^^^^^^^^^^^^ ASY100 | -ASY100.py:11:5: ASY100 Async functions should not call a blocking HTTP method +ASY100.py:11:5: ASY100 Async functions should not call blocking HTTP methods | 11 | async def foo(): 12 | requests.get() | ^^^^^^^^^^^^ ASY100 | -ASY100.py:15:5: ASY100 Async functions should not call a blocking HTTP method +ASY100.py:15:5: ASY100 Async functions should not call blocking HTTP methods | 15 | async def foo(): 16 | httpx.get() | ^^^^^^^^^ ASY100 | -ASY100.py:19:5: ASY100 Async functions should not call a blocking HTTP method +ASY100.py:19:5: ASY100 Async functions should not call blocking HTTP methods | 19 | async def foo(): 20 | requests.post() | ^^^^^^^^^^^^^ ASY100 | -ASY100.py:23:5: ASY100 Async functions should not call a blocking HTTP method +ASY100.py:23:5: ASY100 Async functions should not call blocking HTTP methods | 23 | async def foo(): 24 | httpx.post() diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap index cedf1237509f8..8fa38f8950507 100644 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap @@ -1,42 +1,42 @@ --- source: crates/ruff/src/rules/flake8_async/mod.rs --- -ASY101.py:7:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:7:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 7 | async def foo(): 8 | open("foo") | ^^^^ ASY101 | -ASY101.py:11:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:11:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 11 | async def foo(): 12 | time.sleep(1) | ^^^^^^^^^^ ASY101 | -ASY101.py:15:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:15:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 15 | async def foo(): 16 | subprocess.run("foo") | ^^^^^^^^^^^^^^ ASY101 | -ASY101.py:19:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:19:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 19 | async def foo(): 20 | subprocess.call("foo") | ^^^^^^^^^^^^^^^ ASY101 | -ASY101.py:27:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:27:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 27 | async def foo(): 28 | os.wait4(10) | ^^^^^^^^ ASY101 | -ASY101.py:31:5: ASY101 Async functions should not contain a call to `open`, `time.sleep` or any subprocess method +ASY101.py:31:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods | 31 | async def foo(): 32 | os.wait(12) diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap index 6c1d8954fbe3c..4a8079e406036 100644 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap +++ b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap @@ -1,14 +1,14 @@ --- source: crates/ruff/src/rules/flake8_async/mod.rs --- -ASY102.py:5:5: ASY102 Async functions should not contain a call to unsafe `os` methods +ASY102.py:5:5: ASY102 Async functions should not call synchronous `os` methods | 5 | async def foo(): 6 | os.popen() | ^^^^^^^^ ASY102 | -ASY102.py:9:5: ASY102 Async functions should not contain a call to unsafe `os` methods +ASY102.py:9:5: ASY102 Async functions should not call synchronous `os` methods | 9 | async def foo(): 10 | os.spawnl() From 9ff18bb90fd2ee7296d99d4e993b42d2697cf5d5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:39:52 -0400 Subject: [PATCH 13/17] Remove ViolatingCalls --- crates/ruff/src/checkers/ast/mod.rs | 6 +- crates/ruff/src/rules/flake8_async/rules.rs | 66 +++++++-------------- 2 files changed, 25 insertions(+), 47 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 10a3f6943c061..3944ee203518d 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2590,21 +2590,21 @@ where .rules .enabled(Rule::BlockingHttpCallInsideAsyncDef) { - flake8_async::rules::blocking_http_call_inside_async_def(self, expr); + flake8_async::rules::blocking_http_call(self, expr); } if self .settings .rules .enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) { - flake8_async::rules::open_sleep_or_subprocess_inside_async_def(self, expr); + flake8_async::rules::open_sleep_or_subprocess_call(self, expr); } if self .settings .rules .enabled(Rule::UnsafeOsMethodInsideAsyncDef) { - flake8_async::rules::unsafe_os_method_inside_async_def(self, expr); + flake8_async::rules::blocking_os_call(self, expr); } // flake8-print diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 75a91ee2665a1..94774c314f6fe 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -7,16 +7,6 @@ use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; -struct ViolatingCalls<'a> { - members: &'a [&'a [&'a str]], -} - -impl<'a> ViolatingCalls<'a> { - pub(crate) const fn new(members: &'a [&'a [&'a str]]) -> Self { - Self { members } - } -} - /// ## What it does /// Checks that async functions do not contain blocking HTTP calls. /// @@ -51,7 +41,7 @@ impl Violation for BlockingHttpCallInsideAsyncDef { } } -const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ +const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ &["urllib", "request", "urlopen"], &["httpx", "get"], &["httpx", "post"], @@ -71,10 +61,10 @@ const BLOCKING_HTTP_CALLS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ &["requests", "connect"], &["requests", "options"], &["requests", "trace"], -])]; +]; /// ASY100 -pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -89,14 +79,10 @@ pub(crate) fn blocking_http_call_inside_async_def(checker: &mut Checker, expr: & { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = checker.ctx.resolve_call_path(func) { - for v_call in BLOCKING_HTTP_CALLS { - for member in v_call.members { - if call_path.as_slice() == *member { - checker - .diagnostics - .push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); - } - } + if BLOCKING_HTTP_CALLS.contains(&call_path.as_slice()) { + checker + .diagnostics + .push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); } } } @@ -136,7 +122,7 @@ impl Violation for OpenSleepOrSubprocessInsideAsyncDef { } } -const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ViolatingCalls::new(&[ +const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ &["", "open"], &["time", "sleep"], &["subprocess", "run"], @@ -152,10 +138,10 @@ const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[ViolatingCalls] = &[ViolatingCalls::new(& &["os", "wait4"], &["os", "waitid"], &["os", "waitpid"], -])]; +]; /// ASY101 -pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -170,15 +156,11 @@ pub(crate) fn open_sleep_or_subprocess_inside_async_def(checker: &mut Checker, e { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = checker.ctx.resolve_call_path(func) { - for v_call in OPEN_SLEEP_OR_SUBPROCESS_CALL { - for member in v_call.members { - if call_path.as_slice() == *member { - checker.diagnostics.push(Diagnostic::new( - OpenSleepOrSubprocessInsideAsyncDef, - func.range, - )); - } - } + if OPEN_SLEEP_OR_SUBPROCESS_CALL.contains(&call_path.as_slice()) { + checker.diagnostics.push(Diagnostic::new( + OpenSleepOrSubprocessInsideAsyncDef, + func.range, + )); } } } @@ -218,7 +200,7 @@ impl Violation for UnsafeOsMethodInsideAsyncDef { } } -const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ +const UNSAFE_OS_METHODS: &[&[&str]] = &[ &["os", "popen"], &["os", "posix_spawn"], &["os", "posix_spawnp"], @@ -231,10 +213,10 @@ const UNSAFE_OS_METHODS: &[ViolatingCalls] = &[ViolatingCalls::new(&[ &["os", "spawnvp"], &["os", "spawnvpe"], &["os", "system"], -])]; +]; /// ASY102 -pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Expr) { +pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { if checker .ctx .scopes() @@ -249,14 +231,10 @@ pub(crate) fn unsafe_os_method_inside_async_def(checker: &mut Checker, expr: &Ex { if let ExprKind::Call(ast::ExprCall { func, .. }) = &expr.node { if let Some(call_path) = checker.ctx.resolve_call_path(func) { - for v_call in UNSAFE_OS_METHODS { - for member in v_call.members { - if call_path.as_slice() == *member { - checker - .diagnostics - .push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); - } - } + if UNSAFE_OS_METHODS.contains(&call_path.as_slice()) { + checker + .diagnostics + .push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); } } } From 7d03388148b57a0535f13938bfbb632575971c39 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:41:34 -0400 Subject: [PATCH 14/17] Rename rules --- crates/ruff/src/checkers/ast/mod.rs | 6 +++--- crates/ruff/src/codes.rs | 6 +++--- crates/ruff/src/registry.rs | 6 +++--- crates/ruff/src/rules/flake8_async/mod.rs | 6 +++--- crates/ruff/src/rules/flake8_async/rules.rs | 18 +++++++++--------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 3944ee203518d..36e5b414cc0d8 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2588,21 +2588,21 @@ where if self .settings .rules - .enabled(Rule::BlockingHttpCallInsideAsyncDef) + .enabled(Rule::BlockingHttpCallInAsyncFunction) { flake8_async::rules::blocking_http_call(self, expr); } if self .settings .rules - .enabled(Rule::OpenSleepOrSubprocessInsideAsyncDef) + .enabled(Rule::OpenSleepOrSubprocessInAsyncFunction) { flake8_async::rules::open_sleep_or_subprocess_call(self, expr); } if self .settings .rules - .enabled(Rule::UnsafeOsMethodInsideAsyncDef) + .enabled(Rule::BlockingOsCallInAsyncFunction) { flake8_async::rules::blocking_os_call(self, expr); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index bfdff5128f278..4443fb4becdef 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -216,9 +216,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Pylint, "E0241") => Rule::DuplicateBases, // flake8-async - (Flake8Async, "100") => Rule::BlockingHttpCallInsideAsyncDef, - (Flake8Async, "101") => Rule::OpenSleepOrSubprocessInsideAsyncDef, - (Flake8Async, "102") => Rule::UnsafeOsMethodInsideAsyncDef, + (Flake8Async, "100") => Rule::BlockingHttpCallInAsyncFunction, + (Flake8Async, "101") => Rule::OpenSleepOrSubprocessInAsyncFunction, + (Flake8Async, "102") => Rule::BlockingOsCallInAsyncFunction, // flake8-builtins (Flake8Builtins, "001") => Rule::BuiltinVariableShadowing, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 2702769860f73..d529d665a62f0 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -192,9 +192,9 @@ ruff_macros::register_rules!( rules::pylint::rules::NestedMinMax, rules::pylint::rules::DuplicateBases, // flake8-async - rules::flake8_async::rules::BlockingHttpCallInsideAsyncDef, - rules::flake8_async::rules::OpenSleepOrSubprocessInsideAsyncDef, - rules::flake8_async::rules::UnsafeOsMethodInsideAsyncDef, + 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, diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs index d9f2f86a495fb..94681057dac6a 100644 --- a/crates/ruff/src/rules/flake8_async/mod.rs +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -13,9 +13,9 @@ mod tests { use crate::settings::Settings; use crate::test::test_path; - #[test_case(Rule::BlockingHttpCallInsideAsyncDef, Path::new("ASY100.py"); "ASY100")] - #[test_case(Rule::OpenSleepOrSubprocessInsideAsyncDef, Path::new("ASY101.py"); "ASY101")] - #[test_case(Rule::UnsafeOsMethodInsideAsyncDef, Path::new("ASY102.py"); "ASY102")] + #[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASY100.py"); "ASY100")] + #[test_case(Rule::OpenSleepOrSubprocessInAsyncFunction, Path::new("ASY101.py"); "ASY101")] + #[test_case(Rule::BlockingOsCallInAsyncFunction, Path::new("ASY102.py"); "ASY102")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 94774c314f6fe..f75370a11cc8e 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -32,9 +32,9 @@ use crate::checkers::ast::Checker; /// ... /// ``` #[violation] -pub struct BlockingHttpCallInsideAsyncDef; +pub struct BlockingHttpCallInAsyncFunction; -impl Violation for BlockingHttpCallInsideAsyncDef { +impl Violation for BlockingHttpCallInAsyncFunction { #[derive_message_formats] fn message(&self) -> String { format!("Async functions should not call blocking HTTP methods") @@ -82,7 +82,7 @@ pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { if BLOCKING_HTTP_CALLS.contains(&call_path.as_slice()) { checker .diagnostics - .push(Diagnostic::new(BlockingHttpCallInsideAsyncDef, func.range)); + .push(Diagnostic::new(BlockingHttpCallInAsyncFunction, func.range)); } } } @@ -113,9 +113,9 @@ pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { /// await asyncio.sleep(1000) /// ``` #[violation] -pub struct OpenSleepOrSubprocessInsideAsyncDef; +pub struct OpenSleepOrSubprocessInAsyncFunction; -impl Violation for OpenSleepOrSubprocessInsideAsyncDef { +impl Violation for OpenSleepOrSubprocessInAsyncFunction { #[derive_message_formats] fn message(&self) -> String { format!("Async functions should not call `open`, `time.sleep`, or `subprocess` methods") @@ -158,7 +158,7 @@ pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) 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( - OpenSleepOrSubprocessInsideAsyncDef, + OpenSleepOrSubprocessInAsyncFunction, func.range, )); } @@ -191,9 +191,9 @@ pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) /// os.popen() /// ``` #[violation] -pub struct UnsafeOsMethodInsideAsyncDef; +pub struct BlockingOsCallInAsyncFunction; -impl Violation for UnsafeOsMethodInsideAsyncDef { +impl Violation for BlockingOsCallInAsyncFunction { #[derive_message_formats] fn message(&self) -> String { format!("Async functions should not call synchronous `os` methods") @@ -234,7 +234,7 @@ pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { if UNSAFE_OS_METHODS.contains(&call_path.as_slice()) { checker .diagnostics - .push(Diagnostic::new(UnsafeOsMethodInsideAsyncDef, func.range)); + .push(Diagnostic::new(BlockingOsCallInAsyncFunction, func.range)); } } } From a9ecc964666313e79f8a4dd719619dd4b3efa1a5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:42:35 -0400 Subject: [PATCH 15/17] DRY up async check --- crates/ruff/src/rules/flake8_async/rules.rs | 54 +++++++-------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index f75370a11cc8e..6c63bff7708fa 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -3,6 +3,7 @@ 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; @@ -65,18 +66,7 @@ const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ /// ASY100 pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { - if checker - .ctx - .scopes() - .find_map(|scope| { - if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { - Some(*async_) - } else { - None - } - }) - .unwrap_or(false) - { + 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()) { @@ -142,18 +132,7 @@ const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ /// ASY101 pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { - if checker - .ctx - .scopes() - .find_map(|scope| { - if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { - Some(*async_) - } else { - None - } - }) - .unwrap_or(false) - { + 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()) { @@ -217,18 +196,7 @@ const UNSAFE_OS_METHODS: &[&[&str]] = &[ /// ASY102 pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { - if checker - .ctx - .scopes() - .find_map(|scope| { - if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { - Some(*async_) - } else { - None - } - }) - .unwrap_or(false) - { + 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()) { @@ -240,3 +208,17 @@ pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { } } } + +/// 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) +} From 12e8c029bb44d56a95517776c8e03470e484bfb8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 May 2023 22:44:50 -0400 Subject: [PATCH 16/17] Rename to ASYNC; add license; add to docs --- LICENSE | 25 ++++++++++ .../flake8_async/{ASY100.py => ASYNC100.py} | 0 .../flake8_async/{ASY101.py => ASYNC101.py} | 0 .../flake8_async/{ASY102.py => ASYNC102.py} | 0 crates/ruff/src/registry.rs | 2 +- crates/ruff/src/rules/flake8_async/mod.rs | 6 +-- ...flake8_async__tests__ASY100_ASY100.py.snap | 39 ---------------- ...flake8_async__tests__ASY101_ASY101.py.snap | 46 ------------------- ...flake8_async__tests__ASY102_ASY102.py.snap | 18 -------- ...e8_async__tests__ASYNC100_ASYNC100.py.snap | 39 ++++++++++++++++ ...e8_async__tests__ASYNC101_ASYNC101.py.snap | 46 +++++++++++++++++++ ...e8_async__tests__ASYNC102_ASYNC102.py.snap | 18 ++++++++ docs/faq.md | 2 + ruff.schema.json | 12 ++--- 14 files changed, 140 insertions(+), 113 deletions(-) rename crates/ruff/resources/test/fixtures/flake8_async/{ASY100.py => ASYNC100.py} (100%) rename crates/ruff/resources/test/fixtures/flake8_async/{ASY101.py => ASYNC101.py} (100%) rename crates/ruff/resources/test/fixtures/flake8_async/{ASY102.py => ASYNC102.py} (100%) delete mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap delete mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap delete mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC101_ASYNC101.py.snap create mode 100644 crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASYNC102_ASYNC102.py.snap 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/crates/ruff/resources/test/fixtures/flake8_async/ASY100.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC100.py similarity index 100% rename from crates/ruff/resources/test/fixtures/flake8_async/ASY100.py rename to crates/ruff/resources/test/fixtures/flake8_async/ASYNC100.py diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY101.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC101.py similarity index 100% rename from crates/ruff/resources/test/fixtures/flake8_async/ASY101.py rename to crates/ruff/resources/test/fixtures/flake8_async/ASYNC101.py diff --git a/crates/ruff/resources/test/fixtures/flake8_async/ASY102.py b/crates/ruff/resources/test/fixtures/flake8_async/ASYNC102.py similarity index 100% rename from crates/ruff/resources/test/fixtures/flake8_async/ASY102.py rename to crates/ruff/resources/test/fixtures/flake8_async/ASYNC102.py diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index d529d665a62f0..e331c2d0a96b9 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -740,7 +740,7 @@ pub enum Linter { #[prefix = "ANN"] Flake8Annotations, /// [flake8-async](https://pypi.org/project/flake8-async/) - #[prefix = "ASY"] + #[prefix = "ASYNC"] Flake8Async, /// [flake8-bandit](https://pypi.org/project/flake8-bandit/) #[prefix = "S"] diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs index 94681057dac6a..8842526400e42 100644 --- a/crates/ruff/src/rules/flake8_async/mod.rs +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -13,9 +13,9 @@ mod tests { use crate::settings::Settings; use crate::test::test_path; - #[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASY100.py"); "ASY100")] - #[test_case(Rule::OpenSleepOrSubprocessInAsyncFunction, Path::new("ASY101.py"); "ASY101")] - #[test_case(Rule::BlockingOsCallInAsyncFunction, Path::new("ASY102.py"); "ASY102")] + #[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( diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap deleted file mode 100644 index fdc2093154981..0000000000000 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY100_ASY100.py.snap +++ /dev/null @@ -1,39 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_async/mod.rs ---- -ASY100.py:7:5: ASY100 Async functions should not call blocking HTTP methods - | -7 | async def foo(): -8 | urllib.request.urlopen("http://example.com/foo/bar").read() - | ^^^^^^^^^^^^^^^^^^^^^^ ASY100 - | - -ASY100.py:11:5: ASY100 Async functions should not call blocking HTTP methods - | -11 | async def foo(): -12 | requests.get() - | ^^^^^^^^^^^^ ASY100 - | - -ASY100.py:15:5: ASY100 Async functions should not call blocking HTTP methods - | -15 | async def foo(): -16 | httpx.get() - | ^^^^^^^^^ ASY100 - | - -ASY100.py:19:5: ASY100 Async functions should not call blocking HTTP methods - | -19 | async def foo(): -20 | requests.post() - | ^^^^^^^^^^^^^ ASY100 - | - -ASY100.py:23:5: ASY100 Async functions should not call blocking HTTP methods - | -23 | async def foo(): -24 | httpx.post() - | ^^^^^^^^^^ ASY100 - | - - diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap deleted file mode 100644 index 8fa38f8950507..0000000000000 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY101_ASY101.py.snap +++ /dev/null @@ -1,46 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_async/mod.rs ---- -ASY101.py:7:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -7 | async def foo(): -8 | open("foo") - | ^^^^ ASY101 - | - -ASY101.py:11:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -11 | async def foo(): -12 | time.sleep(1) - | ^^^^^^^^^^ ASY101 - | - -ASY101.py:15:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -15 | async def foo(): -16 | subprocess.run("foo") - | ^^^^^^^^^^^^^^ ASY101 - | - -ASY101.py:19:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -19 | async def foo(): -20 | subprocess.call("foo") - | ^^^^^^^^^^^^^^^ ASY101 - | - -ASY101.py:27:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -27 | async def foo(): -28 | os.wait4(10) - | ^^^^^^^^ ASY101 - | - -ASY101.py:31:5: ASY101 Async functions should not call `open`, `time.sleep`, or `subprocess` methods - | -31 | async def foo(): -32 | os.wait(12) - | ^^^^^^^ ASY101 - | - - diff --git a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap b/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap deleted file mode 100644 index 4a8079e406036..0000000000000 --- a/crates/ruff/src/rules/flake8_async/snapshots/ruff__rules__flake8_async__tests__ASY102_ASY102.py.snap +++ /dev/null @@ -1,18 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_async/mod.rs ---- -ASY102.py:5:5: ASY102 Async functions should not call synchronous `os` methods - | -5 | async def foo(): -6 | os.popen() - | ^^^^^^^^ ASY102 - | - -ASY102.py:9:5: ASY102 Async functions should not call synchronous `os` methods - | - 9 | async def foo(): -10 | os.spawnl() - | ^^^^^^^^^ ASY102 - | - - 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/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 61f8cb00c74e8..8f9e359955005 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1519,12 +1519,12 @@ "ARG003", "ARG004", "ARG005", - "ASY", - "ASY1", - "ASY10", - "ASY100", - "ASY101", - "ASY102", + "ASYNC", + "ASYNC1", + "ASYNC10", + "ASYNC100", + "ASYNC101", + "ASYNC102", "B", "B0", "B00", From 935885d9bf7e54d2d1bd967799c5000efe799d55 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 15 May 2023 10:12:02 +0200 Subject: [PATCH 17/17] Rename fn docstrings from ASY -> ASYNC --- crates/ruff/src/rules/flake8_async/rules.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs index 6c63bff7708fa..2cc748974ecec 100644 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ b/crates/ruff/src/rules/flake8_async/rules.rs @@ -64,7 +64,7 @@ const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ &["requests", "trace"], ]; -/// ASY100 +/// 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 { @@ -130,7 +130,7 @@ const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ &["os", "waitpid"], ]; -/// ASY101 +/// 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 { @@ -194,7 +194,7 @@ const UNSAFE_OS_METHODS: &[&[&str]] = &[ &["os", "system"], ]; -/// ASY102 +/// 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 {