stop reporting return-in-generator

This commit is contained in:
Assad Yousuf 2025-12-11 20:54:46 -07:00
parent ddb7645e9d
commit 5b2a243112
5 changed files with 102 additions and 1 deletions

View File

@ -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"

View File

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

View File

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

View File

@ -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:

View File

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