[refurb] Fix FURB129 autofix generating invalid syntax

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

@@ -43,15 +43,13 @@ def func():
 
 import builtins
 
-
 with builtins.open("FURB129.py") as f:
     for line in f.readlines():
         pass
 
 
 from builtins import open as o
 
-
 with o("FURB129.py") as f:
     for line in f.readlines():
         pass
@@ -89,3 +87,18 @@ def readlines(self) -> list[str]:
         pass
     for _not_line in f.readline():
         pass
+
+# https://github.com/astral-sh/ruff/issues/18231
+with open("furb129.py") as f:
+    for line in (f).readlines():
+        pass
+
+with open("furb129.py") as f:
+    [line for line in (f).readlines()]
+
+
+with open("furb129.py") as f:
+    for line in (((f))).readlines():
+        pass
+    for line in(f).readlines():
+        pass

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

@@ -1,6 +1,7 @@
 use crate::preview::is_readlines_in_for_fix_safe_enabled;
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{ViolationMetadata, derive_message_formats};
+use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::{Comprehension, Expr, StmtFor};
 use ruff_python_semantic::analyze::typing;
 use ruff_python_semantic::analyze::typing::is_io_base_expr;
@@ -84,16 +85,22 @@ fn readlines_in_iter(checker: &Checker, iter_expr: &Expr) {
             return;
         }
     }
+    let edit = if let Some(parenthesized_range) = parenthesized_range(
+        expr_attr.value.as_ref().into(),
+        expr_attr.into(),
+        checker.comment_ranges(),
+        checker.source(),
+    ) {
+        Edit::range_deletion(expr_call.range().add_start(parenthesized_range.len()))
+    } else {
+        Edit::range_deletion(expr_call.range().add_start(expr_attr.value.range().len()))
+    };
 
     let mut diagnostic = Diagnostic::new(ReadlinesInFor, expr_call.range());
     diagnostic.set_fix(if is_readlines_in_for_fix_safe_enabled(checker.settings) {
-        Fix::safe_edit(Edit::range_deletion(
-            expr_call.range().add_start(expr_attr.value.range().len()),
-        ))
+        Fix::safe_edit(edit)
     } else {
-        Fix::unsafe_edit(Edit::range_deletion(
-            expr_call.range().add_start(expr_attr.value.range().len()),
-        ))
+        Fix::unsafe_edit(edit)
     });
     checker.report_diagnostic(diagnostic);
 }

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

@@ -204,40 +204,116 @@ FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over fil
 40 40 |         for _line in bar.readlines():
 41 41 |             pass
 
-FURB129.py:48:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+FURB129.py:47:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
    |
-47 | with builtins.open("FURB129.py") as f:
-48 |     for line in f.readlines():
+46 | with builtins.open("FURB129.py") as f:
+47 |     for line in f.readlines():
    |                 ^^^^^^^^^^^^^ FURB129
-49 |         pass
+48 |         pass
    |
    = help: Remove `readlines()`
 
 ℹ Unsafe fix
+44 44 | import builtins
 45 45 | 
-46 46 | 
-47 47 | with builtins.open("FURB129.py") as f:
-48    |-    for line in f.readlines():
-   48 |+    for line in f:
-49 49 |         pass
+46 46 | with builtins.open("FURB129.py") as f:
+47    |-    for line in f.readlines():
+   47 |+    for line in f:
+48 48 |         pass
+49 49 | 
 50 50 | 
-51 51 | 
 
-FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+FURB129.py:54:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
    |
-55 | with o("FURB129.py") as f:
-56 |     for line in f.readlines():
+53 | with o("FURB129.py") as f:
+54 |     for line in f.readlines():
    |                 ^^^^^^^^^^^^^ FURB129
-57 |         pass
+55 |         pass
    |
    = help: Remove `readlines()`
 
 ℹ Unsafe fix
-53 53 | 
-54 54 | 
-55 55 | with o("FURB129.py") as f:
-56    |-    for line in f.readlines():
-   56 |+    for line in f:
-57 57 |         pass
-58 58 | 
-59 59 |
+51 51 | from builtins import open as o
+52 52 | 
+53 53 | with o("FURB129.py") as f:
+54    |-    for line in f.readlines():
+   54 |+    for line in f:
+55 55 |         pass
+56 56 | 
+57 57 | 
+
+FURB129.py:93:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+   |
+91 | # https://github.com/astral-sh/ruff/issues/18231
+92 | with open("furb129.py") as f:
+93 |     for line in (f).readlines():
+   |                 ^^^^^^^^^^^^^^^ FURB129
+94 |         pass
+   |
+   = help: Remove `readlines()`
+
+ℹ Unsafe fix
+90 90 | 
+91 91 | # https://github.com/astral-sh/ruff/issues/18231
+92 92 | with open("furb129.py") as f:
+93    |-    for line in (f).readlines():
+   93 |+    for line in (f):
+94 94 |         pass
+95 95 | 
+96 96 | with open("furb129.py") as f:
+
+FURB129.py:97:23: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+   |
+96 | with open("furb129.py") as f:
+97 |     [line for line in (f).readlines()]
+   |                       ^^^^^^^^^^^^^^^ FURB129
+   |
+   = help: Remove `readlines()`
+
+ℹ Unsafe fix
+94 94 |         pass
+95 95 | 
+96 96 | with open("furb129.py") as f:
+97    |-    [line for line in (f).readlines()]
+   97 |+    [line for line in (f)]
+98 98 | 
+99 99 | 
+100 100 | with open("furb129.py") as f:
+
+FURB129.py:101:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+    |
+100 | with open("furb129.py") as f:
+101 |     for line in (((f))).readlines():
+    |                 ^^^^^^^^^^^^^^^^^^^ FURB129
+102 |         pass
+103 |     for line in(f).readlines():
+    |
+    = help: Remove `readlines()`
+
+ℹ Unsafe fix
+98  98  | 
+99  99  | 
+100 100 | with open("furb129.py") as f:
+101     |-    for line in (((f))).readlines():
+    101 |+    for line in (((f))):
+102 102 |         pass
+103 103 |     for line in(f).readlines():
+104 104 |         pass
+
+FURB129.py:103:16: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+    |
+101 |     for line in (((f))).readlines():
+102 |         pass
+103 |     for line in(f).readlines():
+    |                ^^^^^^^^^^^^^^^ FURB129
+104 |         pass
+    |
+    = help: Remove `readlines()`
+
+ℹ Unsafe fix
+100 100 | with open("furb129.py") as f:
+101 101 |     for line in (((f))).readlines():
+102 102 |         pass
+103     |-    for line in(f).readlines():
+    103 |+    for line in(f):
+104 104 |         pass

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

@@ -204,40 +204,116 @@ FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over fil
 40 40 |         for _line in bar.readlines():
 41 41 |             pass
 
-FURB129.py:48:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+FURB129.py:47:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
    |
-47 | with builtins.open("FURB129.py") as f:
-48 |     for line in f.readlines():
+46 | with builtins.open("FURB129.py") as f:
+47 |     for line in f.readlines():
    |                 ^^^^^^^^^^^^^ FURB129
-49 |         pass
+48 |         pass
    |
    = help: Remove `readlines()`
 
 ℹ Safe fix
+44 44 | import builtins
 45 45 | 
-46 46 | 
-47 47 | with builtins.open("FURB129.py") as f:
-48    |-    for line in f.readlines():
-   48 |+    for line in f:
-49 49 |         pass
+46 46 | with builtins.open("FURB129.py") as f:
+47    |-    for line in f.readlines():
+   47 |+    for line in f:
+48 48 |         pass
+49 49 | 
 50 50 | 
-51 51 | 
 
-FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+FURB129.py:54:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
    |
-55 | with o("FURB129.py") as f:
-56 |     for line in f.readlines():
+53 | with o("FURB129.py") as f:
+54 |     for line in f.readlines():
    |                 ^^^^^^^^^^^^^ FURB129
-57 |         pass
+55 |         pass
    |
    = help: Remove `readlines()`
 
 ℹ Safe fix
-53 53 | 
-54 54 | 
-55 55 | with o("FURB129.py") as f:
-56    |-    for line in f.readlines():
-   56 |+    for line in f:
-57 57 |         pass
-58 58 | 
-59 59 |
+51 51 | from builtins import open as o
+52 52 | 
+53 53 | with o("FURB129.py") as f:
+54    |-    for line in f.readlines():
+   54 |+    for line in f:
+55 55 |         pass
+56 56 | 
+57 57 | 
+
+FURB129.py:93:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+   |
+91 | # https://github.com/astral-sh/ruff/issues/18231
+92 | with open("furb129.py") as f:
+93 |     for line in (f).readlines():
+   |                 ^^^^^^^^^^^^^^^ FURB129
+94 |         pass
+   |
+   = help: Remove `readlines()`
+
+ℹ Safe fix
+90 90 | 
+91 91 | # https://github.com/astral-sh/ruff/issues/18231
+92 92 | with open("furb129.py") as f:
+93    |-    for line in (f).readlines():
+   93 |+    for line in (f):
+94 94 |         pass
+95 95 | 
+96 96 | with open("furb129.py") as f:
+
+FURB129.py:97:23: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+   |
+96 | with open("furb129.py") as f:
+97 |     [line for line in (f).readlines()]
+   |                       ^^^^^^^^^^^^^^^ FURB129
+   |
+   = help: Remove `readlines()`
+
+ℹ Safe fix
+94 94 |         pass
+95 95 | 
+96 96 | with open("furb129.py") as f:
+97    |-    [line for line in (f).readlines()]
+   97 |+    [line for line in (f)]
+98 98 | 
+99 99 | 
+100 100 | with open("furb129.py") as f:
+
+FURB129.py:101:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+    |
+100 | with open("furb129.py") as f:
+101 |     for line in (((f))).readlines():
+    |                 ^^^^^^^^^^^^^^^^^^^ FURB129
+102 |         pass
+103 |     for line in(f).readlines():
+    |
+    = help: Remove `readlines()`
+
+ℹ Safe fix
+98  98  | 
+99  99  | 
+100 100 | with open("furb129.py") as f:
+101     |-    for line in (((f))).readlines():
+    101 |+    for line in (((f))):
+102 102 |         pass
+103 103 |     for line in(f).readlines():
+104 104 |         pass
+
+FURB129.py:103:16: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+    |
+101 |     for line in (((f))).readlines():
+102 |         pass
+103 |     for line in(f).readlines():
+    |                ^^^^^^^^^^^^^^^ FURB129
+104 |         pass
+    |
+    = help: Remove `readlines()`
+
+ℹ Safe fix
+100 100 | with open("furb129.py") as f:
+101 101 |     for line in (((f))).readlines():
+102 102 |         pass
+103     |-    for line in(f).readlines():
+    103 |+    for line in(f):
+104 104 |         pass