Skip to content
Draft
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
31 changes: 31 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB122.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,34 @@ def _():
with open("file", "w") as f:
for line in ((1,) if True else (2,)):
f.write(f"{line}")


# https://github.com/astral-sh/ruff/issues/21107
# OK — walrus rebinds the loop variable; converting to a generator expression would
# produce a SyntaxError (PEP 572 forbids rebinding a comprehension iteration variable).

def _():
with open("file", "w") as f:
for line in src:
f.write(line := line.upper())


def _():
with open("file", "w") as f:
for first, *rest in src:
f.write(rest := "".join(rest))


def _():
with open("file", "w") as f:
for a, b in src:
# walrus rebinds `a`, one of the two loop vars
f.write(a := a.upper())


# Error — walrus rebinds `other`, not a loop variable; fix is still valid.

def _():
with open("file", "w") as f:
for line in src:
f.write(other := line.upper())
30 changes: 30 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/for_loop_writes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::helpers::any_over_expr;
use ruff_python_ast::{Expr, ExprList, ExprName, ExprTuple, Stmt, StmtFor};
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::{Binding, ScopeId, SemanticModel, TypingOnlyBindingsStatus};
Expand Down Expand Up @@ -185,6 +186,13 @@ fn for_loop_writes(
return;
}

// The fix converts the loop body into a generator expression, which forbids a walrus
// operator from rebinding a comprehension iteration variable (PEP 572). Skip the
// diagnostic entirely when `write_arg` contains such a named expression.
if write_arg_rebinds_loop_var(write_arg, binding_names) {
return;
}

let locator = checker.locator();
let content = match (for_stmt.target.as_ref(), write_arg) {
(Expr::Name(for_target), Expr::Name(write_arg)) if for_target.id == write_arg.id => {
Expand Down Expand Up @@ -225,6 +233,28 @@ fn for_loop_writes(
.set_fix(fix);
}

/// Return `true` if `write_arg` contains a named (walrus) expression whose target
/// rebinds one of the loop iteration variables.
///
/// Converting `for line in src: dst.write(line := ...)` to a generator expression
/// `dst.writelines(line := ... for line in src)` is a `SyntaxError` (PEP 572 forbids
/// an assignment expression from rebinding a comprehension iteration variable).
fn write_arg_rebinds_loop_var(write_arg: &Expr, binding_names: &[&ExprName]) -> bool {
if binding_names.is_empty() {
return false;
}
any_over_expr(write_arg, |expr| {
if let Expr::Named(named) = expr {
if let Expr::Name(target) = named.target.as_ref() {
return binding_names
.iter()
.any(|loop_var| loop_var.id == target.id);
}
}
false
})
}

fn loop_variables_are_used_outside_loop(
binding_names: &[&ExprName],
loop_range: TextRange,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,4 +373,22 @@ help: Replace with `f.writelines`
- for line in ((1,) if True else (2,)):
- f.write(f"{line}")
215 + f.writelines(f"{line}" for line in ((1,) if True else (2,)))
216 |
|

FURB122 [*] Use of `f.write` in a for loop
--> FURB122.py:246:9
|
244 | def _():
245 | with open("file", "w") as f:
246 | / for line in src:
247 | | f.write(other := line.upper())
| |__________________________________________^
|
help: Replace with `f.writelines`
|
245 | with open("file", "w") as f:
- for line in src:
- f.write(other := line.upper())
246 + f.writelines(other := line.upper() for line in src)
|