mirror of
https://github.com/astral-sh/ruff
synced 2026-01-10 16:15:19 -05:00
[flake8-bugbear] Catch yield expressions within other statements (B901) (#21200)
## Summary This PR re-implements [return-in-generator (B901)](https://docs.astral.sh/ruff/rules/return-in-generator/#return-in-generator-b901) for async generators as a semantic syntax error. This is not a syntax error for sync generators, so we'll need to preserve both the lint rule and the syntax error in this case. It also updates B901 and the new implementation to catch cases where the generator's `yield` or `yield from` expression is part of another statement, as in: ```py def foo(): return (yield) ``` These were previously not caught because we only looked for `Stmt::Expr(Expr::Yield)` in `visit_stmt` instead of visiting `yield` expressions directly. I think this modification is within the spirit of the rule and safe to try out since the rule is in preview. ## Test Plan <!-- How was it tested? --> I have written tests as directed in #17412 --------- Signed-off-by: 11happy <soni5happy@gmail.com> Signed-off-by: 11happy <bhuminjaysoni@gmail.com> Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com> Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
This commit is contained in:
@@ -3,12 +3,13 @@
|
||||
//! This checker is not responsible for traversing the AST itself. Instead, its
|
||||
//! [`SemanticSyntaxChecker::visit_stmt`] and [`SemanticSyntaxChecker::visit_expr`] methods should
|
||||
//! be called in a parent `Visitor`'s `visit_stmt` and `visit_expr` methods, respectively.
|
||||
|
||||
use ruff_python_ast::{
|
||||
self as ast, Expr, ExprContext, IrrefutablePatternKind, Pattern, PythonVersion, Stmt, StmtExpr,
|
||||
StmtImportFrom,
|
||||
StmtFunctionDef, StmtImportFrom,
|
||||
comparable::ComparableExpr,
|
||||
helpers,
|
||||
visitor::{Visitor, walk_expr},
|
||||
visitor::{Visitor, walk_expr, walk_stmt},
|
||||
};
|
||||
use ruff_text_size::{Ranged, TextRange, TextSize};
|
||||
use rustc_hash::{FxBuildHasher, FxHashSet};
|
||||
@@ -739,7 +740,21 @@ impl SemanticSyntaxChecker {
|
||||
self.seen_futures_boundary = true;
|
||||
}
|
||||
}
|
||||
Stmt::FunctionDef(_) => {
|
||||
Stmt::FunctionDef(StmtFunctionDef { is_async, body, .. }) => {
|
||||
if *is_async {
|
||||
let mut visitor = ReturnVisitor::default();
|
||||
visitor.visit_body(body);
|
||||
|
||||
if visitor.has_yield {
|
||||
if let Some(return_range) = visitor.return_range {
|
||||
Self::add_error(
|
||||
ctx,
|
||||
SemanticSyntaxErrorKind::ReturnInGenerator,
|
||||
return_range,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
self.seen_futures_boundary = true;
|
||||
}
|
||||
_ => {
|
||||
@@ -1213,6 +1228,9 @@ impl Display for SemanticSyntaxError {
|
||||
SemanticSyntaxErrorKind::NonlocalWithoutBinding(name) => {
|
||||
write!(f, "no binding for nonlocal `{name}` found")
|
||||
}
|
||||
SemanticSyntaxErrorKind::ReturnInGenerator => {
|
||||
write!(f, "`return` with value in async generator")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1619,6 +1637,9 @@ pub enum SemanticSyntaxErrorKind {
|
||||
|
||||
/// Represents a default type parameter followed by a non-default type parameter.
|
||||
TypeParameterDefaultOrder(String),
|
||||
|
||||
/// Represents a `return` statement with a value in an asynchronous generator.
|
||||
ReturnInGenerator,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)]
|
||||
@@ -1735,6 +1756,40 @@ impl Visitor<'_> for ReboundComprehensionVisitor<'_> {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct ReturnVisitor {
|
||||
return_range: Option<TextRange>,
|
||||
has_yield: bool,
|
||||
}
|
||||
|
||||
impl Visitor<'_> for ReturnVisitor {
|
||||
fn visit_stmt(&mut self, stmt: &Stmt) {
|
||||
match stmt {
|
||||
// Do not recurse into nested functions; they're evaluated separately.
|
||||
Stmt::FunctionDef(_) | Stmt::ClassDef(_) => {}
|
||||
Stmt::Return(ast::StmtReturn {
|
||||
value: Some(_),
|
||||
range,
|
||||
..
|
||||
}) => {
|
||||
self.return_range = Some(*range);
|
||||
walk_stmt(self, stmt);
|
||||
}
|
||||
_ => walk_stmt(self, stmt),
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, expr: &Expr) {
|
||||
match expr {
|
||||
Expr::Lambda(_) => {}
|
||||
Expr::Yield(_) | Expr::YieldFrom(_) => {
|
||||
self.has_yield = true;
|
||||
}
|
||||
_ => walk_expr(self, expr),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct MatchPatternVisitor<'a, Ctx> {
|
||||
names: FxHashSet<&'a ast::name::Name>,
|
||||
ctx: &'a Ctx,
|
||||
|
||||
Reference in New Issue
Block a user