Merge 5b2a243112 into b0bc990cbf

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"

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);
 

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"
+    |     ^^^^^^^^^^^^^^^^^^^^^
+    |

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: <https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html#hook-wrappers-executing-around-other-hooks>
+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:

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;