Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B901.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
Should emit:
B901 - on lines 9, 36
"""
from collections.abc import Generator

from pluggy import hookimpl as pluggy_hookimpl
from pytest import TestReport, hookimpl


def broken():
Expand Down Expand Up @@ -86,3 +90,28 @@ async def broken6():
async def broken7():
yield 1
return [1, 2, 3]


@hookimpl(wrapper=True)
def pytest_runtest_makereport() -> Generator[None, TestReport, TestReport]:
result = yield
result.outcome = "passed"
return result


@pluggy_hookimpl(wrapper=True)
def pluggy_hook() -> Generator[None, int, int]:
result = yield
return result + 1


@hookimpl(wrapper=False)
def broken_wrapper_false() -> Generator[None, int, int]:
result = yield
return result


@hookimpl
def broken_no_wrapper() -> Generator[None, int, int]:
result = yield
return result
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::visitor::{Visitor, walk_expr, walk_stmt};
use ruff_python_ast::{self as ast, Expr, Stmt, StmtFunctionDef};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::TextRange;

use crate::Violation;
Expand Down Expand Up @@ -100,6 +101,10 @@ pub(crate) fn return_in_generator(checker: &Checker, function_def: &StmtFunction
return;
}

if has_hookimpl_wrapper_decorator(&function_def.decorator_list, checker.semantic()) {
return;
}

let mut visitor = ReturnInGeneratorVisitor::default();
visitor.visit_body(&function_def.body);

Expand All @@ -110,6 +115,39 @@ pub(crate) fn return_in_generator(checker: &Checker, function_def: &StmtFunction
}
}

fn has_hookimpl_wrapper_decorator(
decorator_list: &[ast::Decorator],
semantic: &SemanticModel,
) -> bool {
decorator_list.iter().any(|decorator| {
let Expr::Call(ast::ExprCall {
func, arguments, ..
}) = &decorator.expression
else {
return false;
};

if !semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["pytest", "hookimpl"] | ["pluggy", "hookimpl"]
)
})
{
return false;
}

arguments.find_keyword("wrapper").is_some_and(|keyword| {
matches!(
&keyword.value,
Expr::BooleanLiteral(ast::ExprBooleanLiteral { value: true, .. })
)
})
})
}

#[derive(Default)]
struct ReturnInGeneratorVisitor {
return_: Option<TextRange>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
assertion_line: 83
---
B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior
--> B901.py:9:9
Expand Down Expand Up @@ -64,3 +65,21 @@ 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:115:5
|
113 | def broken_wrapper_false() -> Generator[None, int, int]:
114 | result = yield
115 | return result
| ^^^^^^^^^^^^^
|

B901 Using `yield` and `return {value}` in a generator function can lead to confusing behavior
--> B901.py:121:5
|
119 | def broken_no_wrapper() -> Generator[None, int, int]:
120 | result = yield
121 | return result
| ^^^^^^^^^^^^^
|