Skip to content

Commit

Permalink
[refurb] Do not emit diagnostic when loop variables are used outsid…
Browse files Browse the repository at this point in the history
…e loop body (`FURB122`) (#15757)

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
InSyncWithFoo and AlexWaygood authored Jan 28, 2025
1 parent 786099a commit 72a4d34
Show file tree
Hide file tree
Showing 5 changed files with 487 additions and 205 deletions.
165 changes: 127 additions & 38 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB122.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,57 +2,101 @@

lines = ["line 1", "line 2", "line 3"]


# Errors

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

other_line = "other line"
with Path("file").open("w") as f:
for line in lines:
f.write(other_line)

with Path("file").open("w") as f:
for line in lines:
f.write(line)
def _():
other_line = "other line"
with Path("file").open("w") as f:
for line in lines:
f.write(other_line)

with Path("file").open("wb") as f:
for line in lines:
f.write(line.encode())

with Path("file").open("w") as f:
for line in lines:
f.write(line.upper())
def _():
with Path("file").open("w") as f:
for line in lines:
f.write(line)

with Path("file").open("w") as f:
pass

for line in lines:
f.write(line)
def _():
with Path("file").open("wb") as f:
for line in lines:
f.write(line.encode())

# Offer unsafe fix if it would delete comments
with open("file","w") as f:
for line in lines:
# a really important comment
f.write(line)

# OK
def _():
with Path("file").open("w") as f:
for line in lines:
f.write(line.upper())

for line in lines:
Path("file").open("w").write(line)

with Path("file").open("w") as f:
for line in lines:
def _():
with Path("file").open("w") as f:
pass

f.write(line)
for line in lines:
f.write(line)


def _():
# Offer unsafe fix if it would delete comments
with open("file","w") as f:
for line in lines:
# a really important comment
f.write(line)


def _():
with open("file", "w") as f:
for () in a:
f.write(())


def _():
with open("file", "w") as f:
for a, b, c in d:
f.write((a, b))


def _():
with open("file", "w") as f:
for [(), [a.b], (c,)] in d:
f.write(())


def _():
with open("file", "w") as f:
for [([([a[b]],)],), [], (c[d],)] in e:
f.write(())

with Path("file").open("w") as f:

# OK

def _():
for line in lines:
f.write(line)
else:
pass
Path("file").open("w").write(line)


def _():
with Path("file").open("w") as f:
for line in lines:
pass

f.write(line)


def _():
with Path("file").open("w") as f:
for line in lines:
f.write(line)
else:
pass


async def func():
Expand All @@ -61,6 +105,51 @@ async def func():
f.write(line)


with Path("file").open("w") as f:
for line in lines:
f.write() # type: ignore
def _():
with Path("file").open("w") as f:
for line in lines:
f.write() # type: ignore


def _():
with open("file", "w") as f:
global CURRENT_LINE
for CURRENT_LINE in lines:
f.write(CURRENT_LINE)


def _():
foo = 1
def __():
with open("file", "w") as f:
nonlocal foo
for foo in lines:
f.write(foo)


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


def _():
with open("file", "w") as f:
for a, b, c in d:
f.write((a, b))
print(a)


def _():
with open("file", "w") as f:
for [*[*a]], b, [[c]] in d:
f.write((a, b))
print(c)


def _():
with open("file", "w") as f:
global global_foo
for [a, b, (global_foo, c)] in d:
f.write((a, b))
8 changes: 7 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::rules::{
flake8_import_conventions, flake8_pyi, flake8_pytest_style, flake8_type_checking, pyflakes,
pylint, ruff,
pylint, refurb, ruff,
};

/// Run lint rules over the [`Binding`]s.
Expand All @@ -22,6 +22,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
Rule::UnquotedTypeAlias,
Rule::UsedDummyVariable,
Rule::PytestUnittestRaisesAssertion,
Rule::ForLoopWrites,
]) {
return;
}
Expand Down Expand Up @@ -109,5 +110,10 @@ pub(crate) fn bindings(checker: &mut Checker) {
checker.diagnostics.push(diagnostic);
}
}
if checker.enabled(Rule::ForLoopWrites) {
if let Some(diagnostic) = refurb::rules::for_loop_writes_binding(checker, binding) {
checker.diagnostics.push(diagnostic);
}
}
}
}
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
refurb::rules::for_loop_set_mutations(checker, for_stmt);
}
if checker.enabled(Rule::ForLoopWrites) {
refurb::rules::for_loop_writes(checker, for_stmt);
refurb::rules::for_loop_writes_stmt(checker, for_stmt);
}
}
if checker.enabled(Rule::NeedlessElse) {
Expand Down
Loading

0 comments on commit 72a4d34

Please sign in to comment.