diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py new file mode 100644 index 0000000000..e2a6cf0af7 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py @@ -0,0 +1,67 @@ +import time +import asyncio + + +async def pass_1a(): # OK: awaits a coroutine + await asyncio.sleep(1) + + +async def pass_1b(): # OK: awaits a coroutine + def foo(optional_arg=await bar()): + pass + + +async def pass_2(): # OK: uses an async context manager + async with None as i: + pass + + +async def pass_3(): # OK: uses an async loop + async for i in []: + pass + + +class Foo: + async def pass_4(): # OK: method of a class + pass + + +def foo(): + async def pass_5(): # OK: uses an await + await bla + + +async def fail_1a(): # RUF029 + time.sleep(1) + + +async def fail_1b(): # RUF029: yield does not require async + yield "hello" + + +async def fail_2(): # RUF029 + with None as i: + pass + + +async def fail_3(): # RUF029 + for i in []: + pass + + return foo + + +async def fail_4a(): # RUF029: the /outer/ function does not await + async def foo(): + await bla + + +async def fail_4b(): # RUF029: the /outer/ function does not await + class Foo: + async def foo(): + await bla + + +def foo(): + async def fail_4c(): # RUF029: the /inner/ function does not await + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index e3ca7a6d11..8eaf670f22 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -348,6 +348,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::SslWithBadDefaults) { flake8_bandit::rules::ssl_with_bad_defaults(checker, function_def); } + if checker.enabled(Rule::UnusedAsync) { + ruff::rules::unused_async(checker, function_def); + } } Stmt::Return(_) => { if checker.enabled(Rule::ReturnOutsideFunction) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index dae1651b78..ec5ae9e792 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -963,6 +963,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg), (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax), (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment), + (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml), #[cfg(feature = "test-rules")] diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 6d2d76f544..b26cfa6afb 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -53,6 +53,7 @@ mod tests { #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))] #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))] #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))] + #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] 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_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index d4461f1f3b..b4f53564c7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -25,6 +25,7 @@ pub(crate) use test_rules::*; pub(crate) use unnecessary_dict_comprehension_for_iterable::*; pub(crate) use unnecessary_iterable_allocation_for_first_element::*; pub(crate) use unnecessary_key_check::*; +pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; mod ambiguous_unicode_character; @@ -58,6 +59,7 @@ pub(crate) mod test_rules; mod unnecessary_dict_comprehension_for_iterable; mod unnecessary_iterable_allocation_for_first_element; mod unnecessary_key_check; +mod unused_async; mod unused_noqa; #[derive(Clone, Copy)] diff --git a/crates/ruff_linter/src/rules/ruff/rules/unused_async.rs b/crates/ruff_linter/src/rules/ruff/rules/unused_async.rs new file mode 100644 index 0000000000..470b16cfa8 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unused_async.rs @@ -0,0 +1,177 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::identifier::Identifier; +use ruff_python_ast::visitor::preorder; +use ruff_python_ast::{self as ast, AnyNodeRef, Expr, Stmt}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for functions declared `async` that do not await or otherwise use features requiring the +/// function to be declared `async`. +/// +/// ## Why is this bad? +/// Declaring a function `async` when it's not is usually a mistake, and will artificially limit the +/// contexts where that function may be called. In some cases, labeling a function `async` is +/// semantically meaningful (e.g. with the trio library). +/// +/// ## Examples +/// ```python +/// async def foo(): +/// bar() +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// bar() +/// ``` +#[violation] +pub struct UnusedAsync { + name: String, +} + +impl Violation for UnusedAsync { + #[derive_message_formats] + fn message(&self) -> String { + let UnusedAsync { name } = self; + format!( + "Function `{name}` is declared `async`, but doesn't `await` or use `async` features." + ) + } +} + +#[derive(Default)] +struct AsyncExprVisitor { + found_await_or_async: bool, +} + +/// Traverse a function's body to find whether it contains an await-expr, an async-with, or an +/// async-for. Stop traversing after one is found. The bodies of inner-functions and inner-classes +/// aren't traversed. +impl<'a> preorder::PreorderVisitor<'a> for AsyncExprVisitor { + fn enter_node(&mut self, _node: AnyNodeRef<'a>) -> preorder::TraversalSignal { + if self.found_await_or_async { + preorder::TraversalSignal::Skip + } else { + preorder::TraversalSignal::Traverse + } + } + fn visit_expr(&mut self, expr: &'a Expr) { + match expr { + Expr::Await(_) => { + self.found_await_or_async = true; + } + _ => preorder::walk_expr(self, expr), + } + } + fn visit_stmt(&mut self, stmt: &'a Stmt) { + match stmt { + Stmt::With(ast::StmtWith { is_async: true, .. }) => { + self.found_await_or_async = true; + } + Stmt::For(ast::StmtFor { is_async: true, .. }) => { + self.found_await_or_async = true; + } + // avoid counting inner classes' or functions' bodies toward the search + Stmt::FunctionDef(function_def) => { + function_def_visit_preorder_except_body(function_def, self); + } + Stmt::ClassDef(class_def) => { + class_def_visit_preorder_except_body(class_def, self); + } + _ => preorder::walk_stmt(self, stmt), + } + } +} + +/// Very nearly `crate::node::StmtFunctionDef.visit_preorder`, except it is specialized and, +/// crucially, doesn't traverse the body. +fn function_def_visit_preorder_except_body<'a, V>( + function_def: &'a ast::StmtFunctionDef, + visitor: &mut V, +) where + V: preorder::PreorderVisitor<'a>, +{ + let ast::StmtFunctionDef { + parameters, + decorator_list, + returns, + type_params, + .. + } = function_def; + + for decorator in decorator_list { + visitor.visit_decorator(decorator); + } + + if let Some(type_params) = type_params { + visitor.visit_type_params(type_params); + } + + visitor.visit_parameters(parameters); + + for expr in returns { + visitor.visit_annotation(expr); + } +} + +/// Very nearly `crate::node::StmtClassDef.visit_preorder`, except it is specialized and, +/// crucially, doesn't traverse the body. +fn class_def_visit_preorder_except_body<'a, V>(class_def: &'a ast::StmtClassDef, visitor: &mut V) +where + V: preorder::PreorderVisitor<'a>, +{ + let ast::StmtClassDef { + arguments, + decorator_list, + type_params, + .. + } = class_def; + + for decorator in decorator_list { + visitor.visit_decorator(decorator); + } + + if let Some(type_params) = type_params { + visitor.visit_type_params(type_params); + } + + if let Some(arguments) = arguments { + visitor.visit_arguments(arguments); + } +} + +/// RUF029 +pub(crate) fn unused_async( + checker: &mut Checker, + function_def @ ast::StmtFunctionDef { + is_async, + name, + body, + .. + }: &ast::StmtFunctionDef, +) { + if !is_async { + return; + } + + if checker.semantic().current_scope().kind.is_class() { + return; + } + + let found_await_or_async = { + let mut visitor = AsyncExprVisitor::default(); + preorder::walk_body(&mut visitor, body); + visitor.found_await_or_async + }; + + if !found_await_or_async { + checker.diagnostics.push(Diagnostic::new( + UnusedAsync { + name: name.to_string(), + }, + function_def.identifier(), + )); + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF029_RUF029.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF029_RUF029.py.snap new file mode 100644 index 0000000000..fba1b92b00 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF029_RUF029.py.snap @@ -0,0 +1,56 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF029.py:34:11: RUF029 Function `fail_1a` is declared `async`, but doesn't `await` or use `async` features. + | +34 | async def fail_1a(): # RUF029 + | ^^^^^^^ RUF029 +35 | time.sleep(1) + | + +RUF029.py:38:11: RUF029 Function `fail_1b` is declared `async`, but doesn't `await` or use `async` features. + | +38 | async def fail_1b(): # RUF029: yield does not require async + | ^^^^^^^ RUF029 +39 | yield "hello" + | + +RUF029.py:42:11: RUF029 Function `fail_2` is declared `async`, but doesn't `await` or use `async` features. + | +42 | async def fail_2(): # RUF029 + | ^^^^^^ RUF029 +43 | with None as i: +44 | pass + | + +RUF029.py:47:11: RUF029 Function `fail_3` is declared `async`, but doesn't `await` or use `async` features. + | +47 | async def fail_3(): # RUF029 + | ^^^^^^ RUF029 +48 | for i in []: +49 | pass + | + +RUF029.py:54:11: RUF029 Function `fail_4a` is declared `async`, but doesn't `await` or use `async` features. + | +54 | async def fail_4a(): # RUF029: the /outer/ function does not await + | ^^^^^^^ RUF029 +55 | async def foo(): +56 | await bla + | + +RUF029.py:59:11: RUF029 Function `fail_4b` is declared `async`, but doesn't `await` or use `async` features. + | +59 | async def fail_4b(): # RUF029: the /outer/ function does not await + | ^^^^^^^ RUF029 +60 | class Foo: +61 | async def foo(): + | + +RUF029.py:66:15: RUF029 Function `fail_4c` is declared `async`, but doesn't `await` or use `async` features. + | +65 | def foo(): +66 | async def fail_4c(): # RUF029: the /inner/ function does not await + | ^^^^^^^ RUF029 +67 | pass + | diff --git a/ruff.schema.json b/ruff.schema.json index 92d0325971..90582fc675 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3619,6 +3619,7 @@ "RUF026", "RUF027", "RUF028", + "RUF029", "RUF1", "RUF10", "RUF100",