mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 10:23:11 +00:00
Add initial flake8-trio rule (#8439)
## Summary This pull request adds [flake8-trio](https://github.com/Zac-HD/flake8-trio) support to ruff, which is a very useful plugin for trio users to avoid very common mistakes. Part of https://github.com/astral-sh/ruff/issues/8451. ## Test Plan Traditional rule testing, as [described in the documentation](https://docs.astral.sh/ruff/contributing/#rule-testing-fixtures-and-snapshots).
This commit is contained in:
parent
7fa6ac976a
commit
2ff1afb15c
13 changed files with 242 additions and 2 deletions
25
LICENSE
25
LICENSE
|
@ -1269,6 +1269,31 @@ are:
|
|||
SOFTWARE.
|
||||
"""
|
||||
|
||||
- flake8-trio, licensed as follows:
|
||||
"""
|
||||
MIT License
|
||||
|
||||
Copyright (c) 2022 Zac Hatfield-Dodds
|
||||
|
||||
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.
|
||||
"""
|
||||
|
||||
- Pyright, licensed as follows:
|
||||
"""
|
||||
MIT License
|
||||
|
|
|
@ -314,6 +314,7 @@ quality tools, including:
|
|||
- [flake8-super](https://pypi.org/project/flake8-super/)
|
||||
- [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/)
|
||||
- [flake8-todos](https://pypi.org/project/flake8-todos/)
|
||||
- [flake8-trio](https://pypi.org/project/flake8-trio/)
|
||||
- [flake8-type-checking](https://pypi.org/project/flake8-type-checking/)
|
||||
- [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)
|
||||
- [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/astral-sh/ruff/issues/2102))
|
||||
|
|
18
crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO100.py
vendored
Normal file
18
crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO100.py
vendored
Normal file
|
@ -0,0 +1,18 @@
|
|||
import trio
|
||||
|
||||
|
||||
async def foo():
|
||||
with trio.fail_after():
|
||||
...
|
||||
|
||||
async def foo():
|
||||
with trio.fail_at():
|
||||
await ...
|
||||
|
||||
async def foo():
|
||||
with trio.move_on_after():
|
||||
...
|
||||
|
||||
async def foo():
|
||||
with trio.move_at():
|
||||
await ...
|
|
@ -12,8 +12,8 @@ use crate::rules::{
|
|||
airflow, flake8_bandit, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_debugger,
|
||||
flake8_django, flake8_errmsg, flake8_import_conventions, flake8_pie, flake8_pyi,
|
||||
flake8_pytest_style, flake8_raise, flake8_return, flake8_simplify, flake8_slots,
|
||||
flake8_tidy_imports, flake8_type_checking, mccabe, pandas_vet, pep8_naming, perflint,
|
||||
pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, tryceratops,
|
||||
flake8_tidy_imports, flake8_trio, flake8_type_checking, mccabe, pandas_vet, pep8_naming,
|
||||
perflint, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, tryceratops,
|
||||
};
|
||||
use crate::settings::types::PythonVersion;
|
||||
|
||||
|
@ -1195,6 +1195,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
if checker.enabled(Rule::UselessWithLock) {
|
||||
pylint::rules::useless_with_lock(checker, with_stmt);
|
||||
}
|
||||
if checker.enabled(Rule::TrioTimeoutWithoutAwait) {
|
||||
flake8_trio::rules::timeout_without_await(checker, with_stmt, items);
|
||||
}
|
||||
}
|
||||
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
|
||||
if checker.enabled(Rule::FunctionUsesLoopVariable) {
|
||||
|
|
|
@ -290,6 +290,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8Async, "101") => (RuleGroup::Stable, rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction),
|
||||
(Flake8Async, "102") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingOsCallInAsyncFunction),
|
||||
|
||||
// flake8-trio
|
||||
(Flake8Trio, "100") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioTimeoutWithoutAwait),
|
||||
|
||||
// flake8-builtins
|
||||
(Flake8Builtins, "001") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinVariableShadowing),
|
||||
(Flake8Builtins, "002") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinArgumentShadowing),
|
||||
|
|
|
@ -64,6 +64,9 @@ pub enum Linter {
|
|||
/// [flake8-async](https://pypi.org/project/flake8-async/)
|
||||
#[prefix = "ASYNC"]
|
||||
Flake8Async,
|
||||
/// [flake8-trio](https://pypi.org/project/flake8-trio/)
|
||||
#[prefix = "TRIO"]
|
||||
Flake8Trio,
|
||||
/// [flake8-bandit](https://pypi.org/project/flake8-bandit/)
|
||||
#[prefix = "S"]
|
||||
Flake8Bandit,
|
||||
|
|
26
crates/ruff_linter/src/rules/flake8_trio/mod.rs
Normal file
26
crates/ruff_linter/src/rules/flake8_trio/mod.rs
Normal file
|
@ -0,0 +1,26 @@
|
|||
//! Rules from [flake8-trio](https://pypi.org/project/flake8-trio/).
|
||||
pub(crate) mod rules;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
|
||||
use anyhow::Result;
|
||||
use test_case::test_case;
|
||||
|
||||
use crate::assert_messages;
|
||||
use crate::registry::Rule;
|
||||
use crate::settings::LinterSettings;
|
||||
use crate::test::test_path;
|
||||
|
||||
#[test_case(Rule::TrioTimeoutWithoutAwait, Path::new("TRIO100.py"))]
|
||||
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_trio").join(path).as_path(),
|
||||
&LinterSettings::for_rule(rule_code),
|
||||
)?;
|
||||
assert_messages!(snapshot, diagnostics);
|
||||
Ok(())
|
||||
}
|
||||
}
|
3
crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs
Normal file
3
crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs
Normal file
|
@ -0,0 +1,3 @@
|
|||
pub(crate) use timeout_without_await::*;
|
||||
|
||||
mod timeout_without_await;
|
|
@ -0,0 +1,125 @@
|
|||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::call_path::CallPath;
|
||||
use ruff_python_ast::visitor::{walk_expr, walk_stmt, Visitor};
|
||||
use ruff_python_ast::{Expr, ExprAwait, Stmt, StmtWith, WithItem};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for trio functions that should contain await but don't.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Some trio context managers, such as `trio.fail_after` and
|
||||
/// `trio.move_on_after`, have no effect unless they contain an `await`
|
||||
/// statement. The use of such functions without an `await` statement is
|
||||
/// likely a mistake.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// async def func():
|
||||
/// with trio.move_on_after(2):
|
||||
/// do_something()
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// async def func():
|
||||
/// with trio.move_on_after(2):
|
||||
/// do_something()
|
||||
/// await awaitable()
|
||||
/// ```
|
||||
#[violation]
|
||||
pub struct TrioTimeoutWithoutAwait {
|
||||
method_name: MethodName,
|
||||
}
|
||||
|
||||
impl Violation for TrioTimeoutWithoutAwait {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let Self { method_name } = self;
|
||||
format!("A `with {method_name}(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.")
|
||||
}
|
||||
}
|
||||
|
||||
/// TRIO100
|
||||
pub(crate) fn timeout_without_await(
|
||||
checker: &mut Checker,
|
||||
with_stmt: &StmtWith,
|
||||
with_items: &[WithItem],
|
||||
) {
|
||||
let Some(method_name) = with_items.iter().find_map(|item| {
|
||||
let call = item.context_expr.as_call_expr()?;
|
||||
let call_path = checker.semantic().resolve_call_path(call.func.as_ref())?;
|
||||
MethodName::try_from(&call_path)
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let mut visitor = AwaitVisitor::default();
|
||||
visitor.visit_body(&with_stmt.body);
|
||||
if visitor.seen_await {
|
||||
return;
|
||||
}
|
||||
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
TrioTimeoutWithoutAwait { method_name },
|
||||
with_stmt.range,
|
||||
));
|
||||
}
|
||||
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
|
||||
enum MethodName {
|
||||
MoveOnAfter,
|
||||
MoveOnAt,
|
||||
FailAfter,
|
||||
FailAt,
|
||||
CancelScope,
|
||||
}
|
||||
|
||||
impl MethodName {
|
||||
fn try_from(call_path: &CallPath<'_>) -> Option<Self> {
|
||||
match call_path.as_slice() {
|
||||
["trio", "move_on_after"] => Some(Self::MoveOnAfter),
|
||||
["trio", "move_on_at"] => Some(Self::MoveOnAt),
|
||||
["trio", "fail_after"] => Some(Self::FailAfter),
|
||||
["trio", "fail_at"] => Some(Self::FailAt),
|
||||
["trio", "CancelScope"] => Some(Self::CancelScope),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for MethodName {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
MethodName::MoveOnAfter => write!(f, "trio.move_on_after"),
|
||||
MethodName::MoveOnAt => write!(f, "trio.move_on_at"),
|
||||
MethodName::FailAfter => write!(f, "trio.fail_after"),
|
||||
MethodName::FailAt => write!(f, "trio.fail_at"),
|
||||
MethodName::CancelScope => write!(f, "trio.CancelScope"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
struct AwaitVisitor {
|
||||
seen_await: bool,
|
||||
}
|
||||
|
||||
impl Visitor<'_> for AwaitVisitor {
|
||||
fn visit_stmt(&mut self, stmt: &Stmt) {
|
||||
match stmt {
|
||||
Stmt::FunctionDef(_) | Stmt::ClassDef(_) => (),
|
||||
_ => walk_stmt(self, stmt),
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, expr: &Expr) {
|
||||
if let Expr::Await(ExprAwait { .. }) = expr {
|
||||
self.seen_await = true;
|
||||
} else {
|
||||
walk_expr(self, expr);
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_trio/mod.rs
|
||||
---
|
||||
TRIO100.py:5:5: TRIO100 A `with trio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
||||
|
|
||||
4 | async def foo():
|
||||
5 | with trio.fail_after():
|
||||
| _____^
|
||||
6 | | ...
|
||||
| |___________^ TRIO100
|
||||
7 |
|
||||
8 | async def foo():
|
||||
|
|
||||
|
||||
TRIO100.py:13:5: TRIO100 A `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
||||
|
|
||||
12 | async def foo():
|
||||
13 | with trio.move_on_after():
|
||||
| _____^
|
||||
14 | | ...
|
||||
| |___________^ TRIO100
|
||||
15 |
|
||||
16 | async def foo():
|
||||
|
|
||||
|
||||
|
|
@ -37,6 +37,7 @@ pub mod flake8_simplify;
|
|||
pub mod flake8_slots;
|
||||
pub mod flake8_tidy_imports;
|
||||
pub mod flake8_todos;
|
||||
pub mod flake8_trio;
|
||||
pub mod flake8_type_checking;
|
||||
pub mod flake8_unused_arguments;
|
||||
pub mod flake8_use_pathlib;
|
||||
|
|
|
@ -81,6 +81,7 @@ natively, including:
|
|||
- [flake8-super](https://pypi.org/project/flake8-super/)
|
||||
- [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/)
|
||||
- [flake8-todos](https://pypi.org/project/flake8-todos/)
|
||||
- [flake8-trio](https://pypi.org/project/flake8-trio/) ([#8451](https://github.com/astral-sh/ruff/issues/8451))
|
||||
- [flake8-type-checking](https://pypi.org/project/flake8-type-checking/)
|
||||
- [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)
|
||||
- [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/astral-sh/ruff/issues/2102))
|
||||
|
@ -185,6 +186,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
|
|||
- [flake8-super](https://pypi.org/project/flake8-super/)
|
||||
- [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/)
|
||||
- [flake8-todos](https://pypi.org/project/flake8-todos/)
|
||||
- [flake8-trio](https://pypi.org/project/flake8-trio/) ([#8451](https://github.com/astral-sh/ruff/issues/8451))
|
||||
- [flake8-type-checking](https://pypi.org/project/flake8-type-checking/)
|
||||
- [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/)
|
||||
- [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/astral-sh/ruff/issues/2102))
|
||||
|
|
4
ruff.schema.json
generated
4
ruff.schema.json
generated
|
@ -3465,6 +3465,10 @@
|
|||
"TID251",
|
||||
"TID252",
|
||||
"TID253",
|
||||
"TRIO",
|
||||
"TRIO1",
|
||||
"TRIO10",
|
||||
"TRIO100",
|
||||
"TRY",
|
||||
"TRY0",
|
||||
"TRY00",
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue