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 acb932f25fd89e..c747d265b0cb15 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py @@ -86,3 +86,43 @@ async def broken6(): async def broken7(): yield 1 return [1, 2, 3] + + +import pytest + + +@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(hookwrapper=True) +def pytest_runtest_call(): + result = yield + 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 0b089b34595c84..d3aa9fea8dfd4e 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::helpers::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 21bf1b1645a970..666db24c08f756 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 @@ -64,3 +64,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:116:5 + | +114 | def pytest_configure(): +115 | yield +116 | return "should error" + | ^^^^^^^^^^^^^^^^^^^^^ + | + +B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior + --> B901.py:122:5 + | +120 | def pytest_unconfigure(): +121 | yield +122 | return "should error" + | ^^^^^^^^^^^^^^^^^^^^^ + | + +B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior + --> B901.py:128:5 + | +126 | def my_fixture(): +127 | yield +128 | 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 43429c8c4ff934..b0b72868e1c334 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs @@ -1,6 +1,6 @@ use std::fmt; -use ruff_python_ast::helpers::map_callable; +use ruff_python_ast::helpers::{is_const_true, map_callable}; use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword, Stmt, StmtFunctionDef}; use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::{ScopeKind, SemanticModel}; @@ -50,6 +50,33 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) - }) } +/// Returns `true` if the decorator is `@pytest.hookimpl(wrapper=True)` or +/// `@pytest.hookimpl(hookwrapper=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; + } + + let wrapper = call.arguments.find_argument_value("wrapper", 6); + let hookwrapper = call.arguments.find_argument_value("hookwrapper", 1); + + wrapper.or(hookwrapper).is_some_and(is_const_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 5923d978ec7a12..0c0ba0a7e11c52 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs @@ -1,5 +1,5 @@ //! Rules from [flake8-pytest-style](https://pypi.org/project/flake8-pytest-style/). -mod helpers; +pub(crate) mod helpers; pub(crate) mod rules; pub mod settings; pub mod types;