Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect unneeded async keywords on functions #9966

Merged
merged 39 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
bd6ccde
[`ruff`] Issue #9951, RUF029: started implementation that detects an …
plredmond Feb 12, 2024
dad3ee2
[`ruff`] Issue #9951, RUF029: detect async function w/o await or asyn…
plredmond Feb 12, 2024
71d908a
[`ruff`] Issue #9951, RUF029: fix rust lister errors by removing unused
plredmond Feb 12, 2024
dc7260d
[`ruff`] Issue #9951, RUF029: add test to suite
plredmond Feb 12, 2024
44b70fe
[`ruff`] Issue #9951, RUF029: documentation; examples do not seem nec…
plredmond Feb 12, 2024
3b8f1be
[`ruff`] Issue #9951, RUF029: cargo dev generate-all and then accept …
plredmond Feb 12, 2024
8635433
[`ruff`] Issue #9951, RUF029: cargo fmt
plredmond Feb 13, 2024
b8af383
Merge branch 'main' into issue9951-spurious_async
plredmond Apr 15, 2024
6070957
[`ruff`] Issue #9951, RUF029: tweak language of test case comments
plredmond Apr 15, 2024
503c468
[`ruff`] Issue #9951, RUF029: tweak language about why it is bad to h…
plredmond Apr 15, 2024
ff5023e
[`ruff`] Issue #9951, RUF029: ruff format the RUF029.py fixture
plredmond Apr 15, 2024
b014a52
[`ruff`] Issue #9951, RUF029: add (small) example to spurious async docs
plredmond Apr 15, 2024
65d85ed
[`ruff`] Issue #9951, RUF029: `cargo insta review` to accept the refo…
plredmond Apr 15, 2024
700dd15
[`ruff`] Issue #9951, RUF029: take the whole function as an argument …
plredmond Apr 15, 2024
5dc2d9e
[`ruff`] Issue #9951, RUF029: rename the field from found_field to fo…
plredmond Apr 15, 2024
939ba6e
[`ruff`] Issue #9951, RUF029: two new pass and fail cases
plredmond Apr 15, 2024
e53f2e7
[`ruff`] Issue #9951, RUF029: one new pass test case
plredmond Apr 15, 2024
4dce6c5
[`ruff`] Issue #9951, RUF029: change the test case that only yields t…
plredmond Apr 15, 2024
b65f072
[`ruff`] Issue #9951, RUF029: change comments and docstrings to remov…
plredmond Apr 15, 2024
2812646
[`ruff`] Issue #9951, RUF029: change identifiers to remove the word y…
plredmond Apr 15, 2024
c8ea017
[`ruff`] Issue #9951, RUF029: format fixture for RUF029 test and rena…
plredmond Apr 16, 2024
e087be2
[`ruff`] Issue #9951, RUF029: reorganize test cases for RUF029
plredmond Apr 16, 2024
1cb7765
[`ruff`] Issue #9951, RUF029: switch to preorder visitor
plredmond Apr 16, 2024
f28fd87
[`ruff`] Issue #9951, RUF029: report only the name of the function as…
plredmond Apr 16, 2024
41f0ac5
[`ruff`] Issue #9951, RUF029: skip the rest of traversal after findin…
plredmond Apr 16, 2024
473e7aa
[`ruff`] Issue #9951, RUF029: disable a test in RUF029 fixture
plredmond Apr 16, 2024
e4abecb
[`ruff`] Issue #9951, RUF029: supplement the tests with asyncs inside…
plredmond Apr 16, 2024
b714e80
[`ruff`] Issue #9951, RUF029: implement traversals of StmtFunctionDef…
plredmond Apr 16, 2024
59c9639
[`ruff`] Issue #9951, RUF029: clippy fixes
plredmond Apr 16, 2024
c5dd1b3
[`ruff`] Issue #9951, RUF029: insta review
plredmond Apr 16, 2024
c53b32f
[`ruff`] Issue #9951, RUF029: rename spurious-async to unused-async
plredmond Apr 16, 2024
2694d4a
[`ruff`] Issue #9951, RUF029: cosmetic change
plredmond Apr 16, 2024
717f884
[`ruff`] Issue #9951, RUF029: add filter to avoid checking functions …
plredmond Apr 16, 2024
10313fd
[`ruff`] Issue #9951, RUF029: insta review - the line numbers changed
plredmond Apr 16, 2024
7f2af61
[`ruff`] Issue #9951, RUF029: cosmetic change to string
plredmond Apr 16, 2024
154d1f4
[`ruff`] Issue #9951, RUF029: insta review - the string explanation h…
plredmond Apr 16, 2024
fc9094e
Merge branch 'main' into issue9951-spurious_async
plredmond Apr 16, 2024
f17dea9
[`ruff`] Issue #9951, RUF029: cargo dev generate-all - fix newline
plredmond Apr 16, 2024
212dfac
[`ruff`] Issue #9951, RUF029: correct rule order in match
plredmond Apr 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 67 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py
Original file line number Diff line number Diff line change
@@ -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
plredmond marked this conversation as resolved.
Show resolved Hide resolved

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
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
177 changes: 177 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/unused_async.rs
Original file line number Diff line number Diff line change
@@ -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(),
));
}
}
Original file line number Diff line number Diff line change
@@ -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
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.