From 5b2a243112991b486db8be03951b491f48de7485 Mon Sep 17 00:00:00 2001 From: Assad Yousuf Date: Thu, 11 Dec 2025 20:54:46 -0700 Subject: [PATCH] stop reporting return-in-generator --- .../test/fixtures/flake8_bugbear/B901.py | 34 ++++++++++++++++++- .../rules/return_in_generator.rs | 9 +++++ ...__flake8_bugbear__tests__B901_B901.py.snap | 28 +++++++++++++++ .../src/rules/flake8_pytest_style/helpers.rs | 30 ++++++++++++++++ .../src/rules/flake8_pytest_style/mod.rs | 2 ++ 5 files changed, 102 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py index acb932f25f..5a506adc50 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py @@ -1,7 +1,8 @@ """ Should emit: -B901 - on lines 9, 36 +B901 - on lines 9, 36, 56, 60, 72, 83, 88, 112, 119, 126 """ +import pytest def broken(): @@ -86,3 +87,34 @@ async def broken6(): async def broken7(): yield 1 return [1, 2, 3] + + +@pytest.hookimpl(wrapper=True) +def pytest_runtest_makereport(): + result = yield + return result + + +@pytest.hookimpl(wrapper=True) +def pytest_fixture_setup(): + result = yield + result.some_attr = "modified" + return result + + +@pytest.hookimpl() +def pytest_configure(): + yield + return "should error" + + +@pytest.hookimpl(wrapper=False) +def pytest_unconfigure(): + yield + return "should error" + + +@pytest.fixture() +def my_fixture(): + yield + return "should error" diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_in_generator.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_in_generator.rs index 0b089b3459..d6ba75c20b 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_in_generator.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_in_generator.rs @@ -5,6 +5,7 @@ use ruff_text_size::TextRange; use crate::Violation; use crate::checkers::ast::Checker; +use crate::rules::flake8_pytest_style::is_pytest_hookimpl_wrapper; /// ## What it does /// Checks for `return {value}` statements in functions that also contain `yield` @@ -100,6 +101,14 @@ pub(crate) fn return_in_generator(checker: &Checker, function_def: &StmtFunction return; } + if function_def + .decorator_list + .iter() + .any(|decorator| is_pytest_hookimpl_wrapper(decorator, checker.semantic())) + { + return; + } + let mut visitor = ReturnInGeneratorVisitor::default(); visitor.visit_body(&function_def.body); diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B901_B901.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B901_B901.py.snap index 21bf1b1645..cdd3187aed 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B901_B901.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B901_B901.py.snap @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +assertion_line: 82 --- B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior --> B901.py:9:9 @@ -64,3 +65,30 @@ B901 Using `yield` and `return {value}` in a generator function can lead to conf 88 | return [1, 2, 3] | ^^^^^^^^^^^^^^^^ | + +B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior + --> B901.py:112:5 + | +110 | def pytest_configure(): +111 | yield +112 | return "should error" + | ^^^^^^^^^^^^^^^^^^^^^ + | + +B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior + --> B901.py:119:5 + | +117 | def pytest_unconfigure(): +118 | yield +119 | return "should error" + | ^^^^^^^^^^^^^^^^^^^^^ + | + +B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior + --> B901.py:126:5 + | +124 | def my_fixture(): +125 | yield +126 | return "should error" + | ^^^^^^^^^^^^^^^^^^^^^ + | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs index 43429c8c4f..effb69b843 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs @@ -50,6 +50,36 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) - }) } +/// Returns `true` if the decorator is `@pytest.hookimpl(wrapper=True)` or `@hookimpl(wrapper=True)`. +/// +/// These hook wrappers intentionally use `return` in generator functions as part of the +/// pytest hook wrapper protocol. +/// +/// See: +pub(crate) fn is_pytest_hookimpl_wrapper(decorator: &Decorator, semantic: &SemanticModel) -> bool { + let Expr::Call(call) = &decorator.expression else { + return false; + }; + + // Check if it's pytest.hookimpl + let is_hookimpl = semantic + .resolve_qualified_name(&call.func) + .is_some_and(|name| matches!(name.segments(), ["pytest", "hookimpl"])); + + if !is_hookimpl { + return false; + } + + // Check for wrapper=True keyword argument + call.arguments.keywords.iter().any(|keyword| { + keyword.arg.as_ref().is_some_and(|arg| arg == "wrapper") + && matches!( + &keyword.value, + Expr::BooleanLiteral(ast::ExprBooleanLiteral { value: true, .. }) + ) + }) +} + /// Whether the currently checked `func` is likely to be a Pytest test. /// /// A normal Pytest test function is one whose name starts with `test` and is either: diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs index 5923d978ec..363a482a3b 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs @@ -4,6 +4,8 @@ pub(crate) mod rules; pub mod settings; pub mod types; +pub(crate) use helpers::is_pytest_hookimpl_wrapper; + #[cfg(test)] mod tests { use std::path::Path;