Skip to content

Commit

Permalink
Fix SIM110 with a yield in the condition (#7801)
Browse files Browse the repository at this point in the history
And allow "await" in the loop iterable.

Fixes #7800
  • Loading branch information
JelleZijlstra authored Oct 4, 2023
1 parent a1509df commit 600471e
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,24 @@ async def f():
if check(x):
return True
return False

async def f():
# SIM110
for x in await iterable:
if check(x):
return True
return False

def f():
# OK (can't turn this into any() because the yield would end up inside a genexp)
for x in iterable:
if (yield check(x)):
return True
return False

def f():
# OK (same)
for x in iterable:
if (yield from check(x)):
return True
return False
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ pub(crate) fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt) {
return;
};

// Check if any of the expressions contain an `await` expression.
if contains_await(loop_.target) || contains_await(loop_.iter) || contains_await(loop_.test) {
// Check if any of the expressions contain an `await`, `yield`, or `yield from` expression.
// If so, turning the code into an any() or all() call would produce a SyntaxError.
if contains_yield_like(loop_.target) || contains_yield_like(loop_.test) {
return;
}

Expand Down Expand Up @@ -413,7 +414,9 @@ fn return_stmt(id: &str, test: &Expr, target: &Expr, iter: &Expr, generator: Gen
generator.stmt(&node3.into())
}

/// Return `true` if the [`Expr`] contains an `await` expression.
fn contains_await(expr: &Expr) -> bool {
/// Return `true` if the [`Expr`] contains an `await`, `yield`, or `yield from` expression.
fn contains_yield_like(expr: &Expr) -> bool {
any_over_expr(expr, &Expr::is_await_expr)
|| any_over_expr(expr, &Expr::is_yield_expr)
|| any_over_expr(expr, &Expr::is_yield_from_expr)
}
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ SIM110.py:184:5: SIM110 [*] Use `return any(check(x) for x in iterable)` instead
186 | | return True
187 | | return False
| |________________^ SIM110
188 |
189 | async def f():
|
= help: Replace with `return any(check(x) for x in iterable)`

Expand All @@ -316,5 +318,36 @@ SIM110.py:184:5: SIM110 [*] Use `return any(check(x) for x in iterable)` instead
186 |- return True
187 |- return False
184 |+ return any(check(x) for x in iterable)
188 185 |
189 186 | async def f():
190 187 | # SIM110

SIM110.py:191:5: SIM110 [*] Use `return any(check(x) for x in await iterable)` instead of `for` loop
|
189 | async def f():
190 | # SIM110
191 | for x in await iterable:
| _____^
192 | | if check(x):
193 | | return True
194 | | return False
| |________________^ SIM110
195 |
196 | def f():
|
= help: Replace with `return any(check(x) for x in await iterable)`

Suggested fix
188 188 |
189 189 | async def f():
190 190 | # SIM110
191 |- for x in await iterable:
192 |- if check(x):
193 |- return True
194 |- return False
191 |+ return any(check(x) for x in await iterable)
195 192 |
196 193 | def f():
197 194 | # OK (can't turn this into any() because the yield would end up inside a genexp)


0 comments on commit 600471e

Please sign in to comment.