diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py index 51a3cf73a5..c5c0307098 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py @@ -57,7 +57,7 @@ with o("FURB129.py") as f: pass -# False positives +# Ok def func(f): for _line in f.readlines(): pass diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index f64821f80d..9973eada40 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -116,3 +116,8 @@ pub(crate) const fn is_allow_nested_roots_enabled(settings: &LinterSettings) -> pub(crate) const fn is_check_file_level_directives_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/17644 +pub(crate) const fn is_readlines_in_for_fix_safe(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 830581d49f..1793e0e964 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -61,6 +61,24 @@ mod tests { Ok(()) } + #[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))] + fn preview(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview__{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("refurb").join(path).as_path(), + &settings::LinterSettings { + preview: settings::types::PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test] fn write_whole_file_python_39() -> Result<()> { let diagnostics = test_path( 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 e06626da9c..23b6e337db 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 crate::preview::is_readlines_in_for_fix_safe; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{Comprehension, Expr, StmtFor}; @@ -85,8 +86,14 @@ fn readlines_in_iter(checker: &Checker, iter_expr: &Expr) { } let mut diagnostic = Diagnostic::new(ReadlinesInFor, expr_call.range()); - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_deletion( - expr_call.range().add_start(expr_attr.value.range().len()), - ))); + diagnostic.set_fix(if is_readlines_in_for_fix_safe(checker.settings) { + Fix::safe_edit(Edit::range_deletion( + expr_call.range().add_start(expr_attr.value.range().len()), + )) + } else { + Fix::unsafe_edit(Edit::range_deletion( + expr_call.range().add_start(expr_attr.value.range().len()), + )) + }); checker.report_diagnostic(diagnostic); } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__preview__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__preview__FURB129_FURB129.py.snap new file mode 100644 index 0000000000..f0fdde19a8 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__preview__FURB129_FURB129.py.snap @@ -0,0 +1,243 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB129.py:7:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +5 | # Errors +6 | with open("FURB129.py") as f: +7 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +8 | pass +9 | a = [line.lower() for line in f.readlines()] + | + = help: Remove `readlines()` + +ℹ Safe fix +4 4 | +5 5 | # Errors +6 6 | with open("FURB129.py") as f: +7 |- for _line in f.readlines(): + 7 |+ for _line in f: +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 10 | b = {line.upper() for line in f.readlines()} + +FURB129.py:9:35: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | + 7 | for _line in f.readlines(): + 8 | pass + 9 | a = [line.lower() for line in f.readlines()] + | ^^^^^^^^^^^^^ FURB129 +10 | b = {line.upper() for line in f.readlines()} +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | + = help: Remove `readlines()` + +ℹ Safe fix +6 6 | with open("FURB129.py") as f: +7 7 | for _line in f.readlines(): +8 8 | pass +9 |- a = [line.lower() for line in f.readlines()] + 9 |+ a = [line.lower() for line in f] +10 10 | b = {line.upper() for line in f.readlines()} +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | + +FURB129.py:10:35: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | + 8 | pass + 9 | a = [line.lower() for line in f.readlines()] +10 | b = {line.upper() for line in f.readlines()} + | ^^^^^^^^^^^^^ FURB129 +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | + = help: Remove `readlines()` + +ℹ Safe fix +7 7 | for _line in f.readlines(): +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 |- b = {line.upper() for line in f.readlines()} + 10 |+ b = {line.upper() for line in f} +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | +13 13 | with Path("FURB129.py").open() as f: + +FURB129.py:11:49: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | + 9 | a = [line.lower() for line in f.readlines()] +10 | b = {line.upper() for line in f.readlines()} +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | ^^^^^^^^^^^^^ FURB129 +12 | +13 | with Path("FURB129.py").open() as f: + | + = help: Remove `readlines()` + +ℹ Safe fix +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 10 | b = {line.upper() for line in f.readlines()} +11 |- c = {line.lower(): line.upper() for line in f.readlines()} + 11 |+ c = {line.lower(): line.upper() for line in f} +12 12 | +13 13 | with Path("FURB129.py").open() as f: +14 14 | for _line in f.readlines(): + +FURB129.py:14:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +13 | with Path("FURB129.py").open() as f: +14 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +15 | pass + | + = help: Remove `readlines()` + +ℹ Safe fix +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | +13 13 | with Path("FURB129.py").open() as f: +14 |- for _line in f.readlines(): + 14 |+ for _line in f: +15 15 | pass +16 16 | +17 17 | for _line in open("FURB129.py").readlines(): + +FURB129.py:17:14: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +15 | pass +16 | +17 | for _line in open("FURB129.py").readlines(): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 +18 | pass + | + = help: Remove `readlines()` + +ℹ Safe fix +14 14 | for _line in f.readlines(): +15 15 | pass +16 16 | +17 |-for _line in open("FURB129.py").readlines(): + 17 |+for _line in open("FURB129.py"): +18 18 | pass +19 19 | +20 20 | for _line in Path("FURB129.py").open().readlines(): + +FURB129.py:20:14: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +18 | pass +19 | +20 | for _line in Path("FURB129.py").open().readlines(): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 +21 | pass + | + = help: Remove `readlines()` + +ℹ Safe fix +17 17 | for _line in open("FURB129.py").readlines(): +18 18 | pass +19 19 | +20 |-for _line in Path("FURB129.py").open().readlines(): + 20 |+for _line in Path("FURB129.py").open(): +21 21 | pass +22 22 | +23 23 | + +FURB129.py:26:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +24 | def func(): +25 | f = Path("FURB129.py").open() +26 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +27 | pass +28 | f.close() + | + = help: Remove `readlines()` + +ℹ Safe fix +23 23 | +24 24 | def func(): +25 25 | f = Path("FURB129.py").open() +26 |- for _line in f.readlines(): + 26 |+ for _line in f: +27 27 | pass +28 28 | f.close() +29 29 | + +FURB129.py:32:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +31 | def func(f: io.BytesIO): +32 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +33 | pass + | + = help: Remove `readlines()` + +ℹ Safe fix +29 29 | +30 30 | +31 31 | def func(f: io.BytesIO): +32 |- for _line in f.readlines(): + 32 |+ for _line in f: +33 33 | pass +34 34 | +35 35 | + +FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +36 | def func(): +37 | with (open("FURB129.py") as f, foo as bar): +38 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +39 | pass +40 | for _line in bar.readlines(): + | + = help: Remove `readlines()` + +ℹ Safe fix +35 35 | +36 36 | def func(): +37 37 | with (open("FURB129.py") as f, foo as bar): +38 |- for _line in f.readlines(): + 38 |+ for _line in f: +39 39 | pass +40 40 | for _line in bar.readlines(): +41 41 | pass + +FURB129.py:48: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(): + | ^^^^^^^^^^^^^ FURB129 +49 | pass + | + = help: Remove `readlines()` + +ℹ Safe fix +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 +50 50 | +51 51 | + +FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +55 | with o("FURB129.py") as f: +56 | for line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +57 | 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 |