diff --git a/LICENSE b/LICENSE index 21e7c353637ca..04ed9285de85a 100644 --- a/LICENSE +++ b/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 diff --git a/README.md b/README.md index bb28ef8478092..adeb99b4db80f 100644 --- a/README.md +++ b/README.md @@ -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)) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO100.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO100.py new file mode 100644 index 0000000000000..86beeb95cd7c3 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO100.py @@ -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 ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 4c696ae33e71f..0a567da1c4961 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -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) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 23720c76d59a1..605bafd4c1ff7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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), diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 7bd5ba4faeedb..2e6fcc199580e 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -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, diff --git a/crates/ruff_linter/src/rules/flake8_trio/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/mod.rs new file mode 100644 index 0000000000000..cde1aae100e6f --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/mod.rs @@ -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(()) + } +} diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs new file mode 100644 index 0000000000000..73521d47fe224 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use timeout_without_await::*; + +mod timeout_without_await; diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/timeout_without_await.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/timeout_without_await.rs new file mode 100644 index 0000000000000..8492933b354ed --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/timeout_without_await.rs @@ -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 { + 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); + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO100_TRIO100.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO100_TRIO100.py.snap new file mode 100644 index 0000000000000..ad7e5b0475ae1 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO100_TRIO100.py.snap @@ -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(): + | + + diff --git a/crates/ruff_linter/src/rules/mod.rs b/crates/ruff_linter/src/rules/mod.rs index 6240d93d12719..64b0128cae164 100644 --- a/crates/ruff_linter/src/rules/mod.rs +++ b/crates/ruff_linter/src/rules/mod.rs @@ -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; diff --git a/docs/faq.md b/docs/faq.md index eb32c3e9311e8..eef55607d3df3 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -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)) diff --git a/ruff.schema.json b/ruff.schema.json index 72a967d1b277c..c09dae9daf941 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3465,6 +3465,10 @@ "TID251", "TID252", "TID253", + "TRIO", + "TRIO1", + "TRIO10", + "TRIO100", "TRY", "TRY0", "TRY00",