diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py index 52b4591a4e..b75a4af952 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py +++ b/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 + ): + ... diff --git a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs index aad840bcf1..6bdbf94184 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs +++ b/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 + }, + )); } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap index f466c31940..6b67b28958 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap +++ b/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 | ...