[`refurb`] Mark `FURB129` autofix as unsafe if there's comments in the `readlines` call (#18858)

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

@@ -105,3 +105,12 @@ with open("furb129.py") as f:
 
     # Test case for issue #17683 (missing space before keyword)
     print([line for line in f.readlines()if True])
+
+# https://github.com/astral-sh/ruff/issues/18843
+with open("file.txt") as fp:
+    for line in (  # 1
+        fp.  # 3  # 2
+        readlines(  # 4
+        )  # 5
+    ):
+        ...

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

@@ -1,3 +1,4 @@
+use ruff_diagnostics::Applicability;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::{Comprehension, Expr, StmtFor};
@@ -31,6 +32,19 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
 ///         ...
 /// ```
 ///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe if there's comments in the
+/// `readlines()` call, as comments may be removed.
+///
+/// For example, the fix would be marked as unsafe in the following case:
+/// ```python
+/// with open("file.txt") as fp:
+///     for line in (  # comment
+///         fp.readlines()  # comment
+///     ):
+///         ...
+/// ```
+///
 /// ## References
 /// - [Python documentation: `io.IOBase.readlines`](https://docs.python.org/3/library/io.html#io.IOBase.readlines)
 #[derive(ViolationMetadata)]
@@ -105,5 +119,12 @@ fn readlines_in_iter(checker: &Checker, iter_expr: &Expr) {
     };
 
     let mut diagnostic = checker.report_diagnostic(ReadlinesInFor, expr_call.range());
-    diagnostic.set_fix(Fix::safe_edit(edit));
+    diagnostic.set_fix(Fix::applicable_edit(
+        edit,
+        if checker.comment_ranges().intersects(iter_expr.range()) {
+            Applicability::Unsafe
+        } else {
+            Applicability::Safe
+        },
+    ));
 }

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

@@ -325,6 +325,8 @@ FURB129.py:107:29: FURB129 [*] Instead of calling `readlines()`, iterate over fi
 106 |     # Test case for issue #17683 (missing space before keyword)
 107 |     print([line for line in f.readlines()if True])
     |                             ^^^^^^^^^^^^^ FURB129
+108 |
+109 | # https://github.com/astral-sh/ruff/issues/18843
     |
     = help: Remove `readlines()`
 
@@ -334,3 +336,30 @@ FURB129.py:107:29: FURB129 [*] Instead of calling `readlines()`, iterate over fi
 106 106 |     # Test case for issue #17683 (missing space before keyword)
 107     |-    print([line for line in f.readlines()if True])
     107 |+    print([line for line in f if True])
+108 108 | 
+109 109 | # https://github.com/astral-sh/ruff/issues/18843
+110 110 | with open("file.txt") as fp:
+
+FURB129.py:112:9: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
+    |
+110 |   with open("file.txt") as fp:
+111 |       for line in (  # 1
+112 | /         fp.  # 3  # 2
+113 | |         readlines(  # 4
+114 | |         )  # 5
+    | |_________^ FURB129
+115 |       ):
+116 |           ...
+    |
+    = help: Remove `readlines()`
+
+ℹ Unsafe fix
+109 109 | # https://github.com/astral-sh/ruff/issues/18843
+110 110 | with open("file.txt") as fp:
+111 111 |     for line in (  # 1
+112     |-        fp.  # 3  # 2
+113     |-        readlines(  # 4
+114     |-        )  # 5
+    112 |+        fp  # 5
+115 113 |     ):
+116 114 |         ...