Skip to content

Commit 04dc48e

Browse files
LaBatata101LaBatata101
authored
[refurb] Fix FURB129 autofix generating invalid syntax (#18235)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary Fixes #18231 <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan Snapshot tests <!-- How was it tested? -->
1 parent 27743ef commit 04dc48e

File tree

4 files changed

+224
-52
lines changed

4 files changed

+224
-52
lines changed

‎crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,13 @@ def func():
4343

4444
import builtins
4545

46-
4746
with builtins.open("FURB129.py") as f:
4847
for line in f.readlines():
4948
pass
5049

5150

5251
from builtins import open as o
5352

54-
5553
with o("FURB129.py") as f:
5654
for line in f.readlines():
5755
pass
@@ -89,3 +87,18 @@ def readlines(self) -> list[str]:
8987
pass
9088
for _not_line in f.readline():
9189
pass
90+
91+
# https://github.com/astral-sh/ruff/issues/18231
92+
with open("furb129.py") as f:
93+
for line in (f).readlines():
94+
pass
95+
96+
with open("furb129.py") as f:
97+
[line for line in (f).readlines()]
98+
99+
100+
with open("furb129.py") as f:
101+
for line in (((f))).readlines():
102+
pass
103+
for line in(f).readlines():
104+
pass

‎crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
use ruff_python_ast::parenthesize::parenthesized_range;
23
use ruff_python_ast::{Comprehension, Expr, StmtFor};
34
use ruff_python_semantic::analyze::typing;
45
use ruff_python_semantic::analyze::typing::is_io_base_expr;
@@ -84,15 +85,21 @@ fn readlines_in_iter(checker: &Checker, iter_expr: &Expr) {
8485
return;
8586
}
8687
}
88+
let edit = if let Some(parenthesized_range) = parenthesized_range(
89+
expr_attr.value.as_ref().into(),
90+
expr_attr.into(),
91+
checker.comment_ranges(),
92+
checker.source(),
93+
) {
94+
Edit::range_deletion(expr_call.range().add_start(parenthesized_range.len()))
95+
} else {
96+
Edit::range_deletion(expr_call.range().add_start(expr_attr.value.range().len()))
97+
};
8798

8899
let mut diagnostic = checker.report_diagnostic(ReadlinesInFor, expr_call.range());
89100
diagnostic.set_fix(if is_readlines_in_for_fix_safe_enabled(checker.settings) {
90-
Fix::safe_edit(Edit::range_deletion(
91-
expr_call.range().add_start(expr_attr.value.range().len()),
92-
))
101+
Fix::safe_edit(edit)
93102
} else {
94-
Fix::unsafe_edit(Edit::range_deletion(
95-
expr_call.range().add_start(expr_attr.value.range().len()),
96-
))
103+
Fix::unsafe_edit(edit)
97104
});
98105
}

‎crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap

Lines changed: 98 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -204,40 +204,116 @@ FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over fil
204204
40 40 | for _line in bar.readlines():
205205
41 41 | pass
206206

207-
FURB129.py:48:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
207+
FURB129.py:47:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
208208
|
209-
47 | with builtins.open("FURB129.py") as f:
210-
48 | for line in f.readlines():
209+
46 | with builtins.open("FURB129.py") as f:
210+
47 | for line in f.readlines():
211211
| ^^^^^^^^^^^^^ FURB129
212-
49 | pass
212+
48 | pass
213213
|
214214
= help: Remove `readlines()`
215215

216216
ℹ Unsafe fix
217+
44 44 | import builtins
217218
45 45 |
218-
46 46 |
219-
47 47 | with builtins.open("FURB129.py") as f:
220-
48 |- for line in f.readlines():
221-
48 |+ for line in f:
222-
49 49 | pass
219+
46 46 | with builtins.open("FURB129.py") as f:
220+
47 |- for line in f.readlines():
221+
47 |+ for line in f:
222+
48 48 | pass
223+
49 49 |
223224
50 50 |
224-
51 51 |
225225

226-
FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
226+
FURB129.py:54:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
227227
|
228-
55 | with o("FURB129.py") as f:
229-
56 | for line in f.readlines():
228+
53 | with o("FURB129.py") as f:
229+
54 | for line in f.readlines():
230230
| ^^^^^^^^^^^^^ FURB129
231-
57 | pass
231+
55 | pass
232232
|
233233
= help: Remove `readlines()`
234234

235235
ℹ Unsafe fix
236-
53 53 |
237-
54 54 |
238-
55 55 | with o("FURB129.py") as f:
239-
56 |- for line in f.readlines():
240-
56 |+ for line in f:
241-
57 57 | pass
242-
58 58 |
243-
59 59 |
236+
51 51 | from builtins import open as o
237+
52 52 |
238+
53 53 | with o("FURB129.py") as f:
239+
54 |- for line in f.readlines():
240+
54 |+ for line in f:
241+
55 55 | pass
242+
56 56 |
243+
57 57 |
244+
245+
FURB129.py:93:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
246+
|
247+
91 | # https://github.com/astral-sh/ruff/issues/18231
248+
92 | with open("furb129.py") as f:
249+
93 | for line in (f).readlines():
250+
| ^^^^^^^^^^^^^^^ FURB129
251+
94 | pass
252+
|
253+
= help: Remove `readlines()`
254+
255+
ℹ Unsafe fix
256+
90 90 |
257+
91 91 | # https://github.com/astral-sh/ruff/issues/18231
258+
92 92 | with open("furb129.py") as f:
259+
93 |- for line in (f).readlines():
260+
93 |+ for line in (f):
261+
94 94 | pass
262+
95 95 |
263+
96 96 | with open("furb129.py") as f:
264+
265+
FURB129.py:97:23: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
266+
|
267+
96 | with open("furb129.py") as f:
268+
97 | [line for line in (f).readlines()]
269+
| ^^^^^^^^^^^^^^^ FURB129
270+
|
271+
= help: Remove `readlines()`
272+
273+
ℹ Unsafe fix
274+
94 94 | pass
275+
95 95 |
276+
96 96 | with open("furb129.py") as f:
277+
97 |- [line for line in (f).readlines()]
278+
97 |+ [line for line in (f)]
279+
98 98 |
280+
99 99 |
281+
100 100 | with open("furb129.py") as f:
282+
283+
FURB129.py:101:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
284+
|
285+
100 | with open("furb129.py") as f:
286+
101 | for line in (((f))).readlines():
287+
| ^^^^^^^^^^^^^^^^^^^ FURB129
288+
102 | pass
289+
103 | for line in(f).readlines():
290+
|
291+
= help: Remove `readlines()`
292+
293+
ℹ Unsafe fix
294+
98 98 |
295+
99 99 |
296+
100 100 | with open("furb129.py") as f:
297+
101 |- for line in (((f))).readlines():
298+
101 |+ for line in (((f))):
299+
102 102 | pass
300+
103 103 | for line in(f).readlines():
301+
104 104 | pass
302+
303+
FURB129.py:103:16: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
304+
|
305+
101 | for line in (((f))).readlines():
306+
102 | pass
307+
103 | for line in(f).readlines():
308+
| ^^^^^^^^^^^^^^^ FURB129
309+
104 | pass
310+
|
311+
= help: Remove `readlines()`
312+
313+
ℹ Unsafe fix
314+
100 100 | with open("furb129.py") as f:
315+
101 101 | for line in (((f))).readlines():
316+
102 102 | pass
317+
103 |- for line in(f).readlines():
318+
103 |+ for line in(f):
319+
104 104 | pass

‎crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__preview__FURB129_FURB129.py.snap

Lines changed: 98 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -204,40 +204,116 @@ FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over fil
204204
40 40 | for _line in bar.readlines():
205205
41 41 | pass
206206

207-
FURB129.py:48:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
207+
FURB129.py:47:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
208208
|
209-
47 | with builtins.open("FURB129.py") as f:
210-
48 | for line in f.readlines():
209+
46 | with builtins.open("FURB129.py") as f:
210+
47 | for line in f.readlines():
211211
| ^^^^^^^^^^^^^ FURB129
212-
49 | pass
212+
48 | pass
213213
|
214214
= help: Remove `readlines()`
215215

216216
ℹ Safe fix
217+
44 44 | import builtins
217218
45 45 |
218-
46 46 |
219-
47 47 | with builtins.open("FURB129.py") as f:
220-
48 |- for line in f.readlines():
221-
48 |+ for line in f:
222-
49 49 | pass
219+
46 46 | with builtins.open("FURB129.py") as f:
220+
47 |- for line in f.readlines():
221+
47 |+ for line in f:
222+
48 48 | pass
223+
49 49 |
223224
50 50 |
224-
51 51 |
225225

226-
FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
226+
FURB129.py:54:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
227227
|
228-
55 | with o("FURB129.py") as f:
229-
56 | for line in f.readlines():
228+
53 | with o("FURB129.py") as f:
229+
54 | for line in f.readlines():
230230
| ^^^^^^^^^^^^^ FURB129
231-
57 | pass
231+
55 | pass
232232
|
233233
= help: Remove `readlines()`
234234

235235
ℹ Safe fix
236-
53 53 |
237-
54 54 |
238-
55 55 | with o("FURB129.py") as f:
239-
56 |- for line in f.readlines():
240-
56 |+ for line in f:
241-
57 57 | pass
242-
58 58 |
243-
59 59 |
236+
51 51 | from builtins import open as o
237+
52 52 |
238+
53 53 | with o("FURB129.py") as f:
239+
54 |- for line in f.readlines():
240+
54 |+ for line in f:
241+
55 55 | pass
242+
56 56 |
243+
57 57 |
244+
245+
FURB129.py:93:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
246+
|
247+
91 | # https://github.com/astral-sh/ruff/issues/18231
248+
92 | with open("furb129.py") as f:
249+
93 | for line in (f).readlines():
250+
| ^^^^^^^^^^^^^^^ FURB129
251+
94 | pass
252+
|
253+
= help: Remove `readlines()`
254+
255+
ℹ Safe fix
256+
90 90 |
257+
91 91 | # https://github.com/astral-sh/ruff/issues/18231
258+
92 92 | with open("furb129.py") as f:
259+
93 |- for line in (f).readlines():
260+
93 |+ for line in (f):
261+
94 94 | pass
262+
95 95 |
263+
96 96 | with open("furb129.py") as f:
264+
265+
FURB129.py:97:23: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
266+
|
267+
96 | with open("furb129.py") as f:
268+
97 | [line for line in (f).readlines()]
269+
| ^^^^^^^^^^^^^^^ FURB129
270+
|
271+
= help: Remove `readlines()`
272+
273+
ℹ Safe fix
274+
94 94 | pass
275+
95 95 |
276+
96 96 | with open("furb129.py") as f:
277+
97 |- [line for line in (f).readlines()]
278+
97 |+ [line for line in (f)]
279+
98 98 |
280+
99 99 |
281+
100 100 | with open("furb129.py") as f:
282+
283+
FURB129.py:101:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
284+
|
285+
100 | with open("furb129.py") as f:
286+
101 | for line in (((f))).readlines():
287+
| ^^^^^^^^^^^^^^^^^^^ FURB129
288+
102 | pass
289+
103 | for line in(f).readlines():
290+
|
291+
= help: Remove `readlines()`
292+
293+
ℹ Safe fix
294+
98 98 |
295+
99 99 |
296+
100 100 | with open("furb129.py") as f:
297+
101 |- for line in (((f))).readlines():
298+
101 |+ for line in (((f))):
299+
102 102 | pass
300+
103 103 | for line in(f).readlines():
301+
104 104 | pass
302+
303+
FURB129.py:103:16: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
304+
|
305+
101 | for line in (((f))).readlines():
306+
102 | pass
307+
103 | for line in(f).readlines():
308+
| ^^^^^^^^^^^^^^^ FURB129
309+
104 | pass
310+
|
311+
= help: Remove `readlines()`
312+
313+
ℹ Safe fix
314+
100 100 | with open("furb129.py") as f:
315+
101 101 | for line in (((f))).readlines():
316+
102 102 | pass
317+
103 |- for line in(f).readlines():
318+
103 |+ for line in(f):
319+
104 104 | pass

0 commit comments

Comments
 (0)