Skip to content

Commit c6a6f67

Browse files
assadyousufntBre
authored andcommitted
[flake8-bugbear] Allow B901 in pytest hook wrappers (astral-sh#21931)
## Summary Stop raising return-in-generator with pytest hook wrappers (@hookimpl(wrapper=True)). They are specifically designed to use this pattern: https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html#hook-wrappers-executing-around-other-hooks Before: return-in-generator reports would surface with pytest hook wrappers After: specifically check for pytest hook wrappers before reporting return-in-generator Testing: Wrote some tests to cover different cases --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent d99f520 commit c6a6f67

5 files changed

Lines changed: 105 additions & 2 deletions

File tree

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,43 @@ async def broken6():
8686
async def broken7():
8787
yield 1
8888
return [1, 2, 3]
89+
90+
91+
import pytest
92+
93+
94+
@pytest.hookimpl(wrapper=True)
95+
def pytest_runtest_makereport():
96+
result = yield
97+
return result
98+
99+
100+
@pytest.hookimpl(wrapper=True)
101+
def pytest_fixture_setup():
102+
result = yield
103+
result.some_attr = "modified"
104+
return result
105+
106+
107+
@pytest.hookimpl(hookwrapper=True)
108+
def pytest_runtest_call():
109+
result = yield
110+
return result
111+
112+
113+
@pytest.hookimpl()
114+
def pytest_configure():
115+
yield
116+
return "should error"
117+
118+
119+
@pytest.hookimpl(wrapper=False)
120+
def pytest_unconfigure():
121+
yield
122+
return "should error"
123+
124+
125+
@pytest.fixture()
126+
def my_fixture():
127+
yield
128+
return "should error"

crates/ruff_linter/src/rules/flake8_bugbear/rules/return_in_generator.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use ruff_text_size::TextRange;
55

66
use crate::Violation;
77
use crate::checkers::ast::Checker;
8+
use crate::rules::flake8_pytest_style::helpers::is_pytest_hookimpl_wrapper;
89

910
/// ## What it does
1011
/// 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
100101
return;
101102
}
102103

104+
if function_def
105+
.decorator_list
106+
.iter()
107+
.any(|decorator| is_pytest_hookimpl_wrapper(decorator, checker.semantic()))
108+
{
109+
return;
110+
}
111+
103112
let mut visitor = ReturnInGeneratorVisitor::default();
104113
visitor.visit_body(&function_def.body);
105114

crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B901_B901.py.snap

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,30 @@ B901 Using `yield` and `return {value}` in a generator function can lead to conf
6464
88 | return [1, 2, 3]
6565
| ^^^^^^^^^^^^^^^^
6666
|
67+
68+
B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior
69+
--> B901.py:116:5
70+
|
71+
114 | def pytest_configure():
72+
115 | yield
73+
116 | return "should error"
74+
| ^^^^^^^^^^^^^^^^^^^^^
75+
|
76+
77+
B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior
78+
--> B901.py:122:5
79+
|
80+
120 | def pytest_unconfigure():
81+
121 | yield
82+
122 | return "should error"
83+
| ^^^^^^^^^^^^^^^^^^^^^
84+
|
85+
86+
B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior
87+
--> B901.py:128:5
88+
|
89+
126 | def my_fixture():
90+
127 | yield
91+
128 | return "should error"
92+
| ^^^^^^^^^^^^^^^^^^^^^
93+
|

crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::fmt;
22

3-
use ruff_python_ast::helpers::map_callable;
3+
use ruff_python_ast::helpers::{is_const_true, map_callable};
44
use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Keyword, Stmt, StmtFunctionDef};
55
use ruff_python_semantic::analyze::visibility;
66
use ruff_python_semantic::{ScopeKind, SemanticModel};
@@ -50,6 +50,33 @@ pub(super) fn is_pytest_parametrize(call: &ExprCall, semantic: &SemanticModel) -
5050
})
5151
}
5252

53+
/// Returns `true` if the decorator is `@pytest.hookimpl(wrapper=True)` or
54+
/// `@pytest.hookimpl(hookwrapper=True)`.
55+
///
56+
/// These hook wrappers intentionally use `return` in generator functions as part of the
57+
/// pytest hook wrapper protocol.
58+
///
59+
/// See: <https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html#hook-wrappers-executing-around-other-hooks>
60+
pub(crate) fn is_pytest_hookimpl_wrapper(decorator: &Decorator, semantic: &SemanticModel) -> bool {
61+
let Expr::Call(call) = &decorator.expression else {
62+
return false;
63+
};
64+
65+
// Check if it's pytest.hookimpl
66+
let is_hookimpl = semantic
67+
.resolve_qualified_name(&call.func)
68+
.is_some_and(|name| matches!(name.segments(), ["pytest", "hookimpl"]));
69+
70+
if !is_hookimpl {
71+
return false;
72+
}
73+
74+
let wrapper = call.arguments.find_argument_value("wrapper", 6);
75+
let hookwrapper = call.arguments.find_argument_value("hookwrapper", 1);
76+
77+
wrapper.or(hookwrapper).is_some_and(is_const_true)
78+
}
79+
5380
/// Whether the currently checked `func` is likely to be a Pytest test.
5481
///
5582
/// 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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//! Rules from [flake8-pytest-style](https://pypi.org/project/flake8-pytest-style/).
2-
mod helpers;
2+
pub(crate) mod helpers;
33
pub(crate) mod rules;
44
pub mod settings;
55
pub mod types;

0 commit comments

Comments
 (0)