diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py index 8f58cb9d8f..9556dfb36f 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py @@ -205,3 +205,12 @@ def func(): if a.endswith("foo"): a = a[: -len("foo")] print(a) + + +def example(filename: str, text: str): + filename = ( + filename[:-4] # text + if filename.endswith(".txt") # text + else filename + ) + diff --git a/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs b/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs index 10c0f8d8a3..1bee43e782 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, PythonVersion}; use ruff_python_semantic::SemanticModel; @@ -37,6 +38,9 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// filename = filename.removesuffix(".txt") /// text = text.removeprefix("pre") /// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the expression contains comments. #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "0.9.0")] pub(crate) struct SliceToRemovePrefixOrSuffix { @@ -89,11 +93,16 @@ pub(crate) fn slice_to_remove_affix_expr(checker: &Checker, if_expr: &ast::ExprI let replacement = generate_removeaffix_expr(text, &removal_data.affix_query, checker.locator()); - diagnostic.set_fix(Fix::safe_edit(Edit::replacement( - replacement, - if_expr.start(), - if_expr.end(), - ))); + let applicability = if checker.comment_ranges().intersects(if_expr.range) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.set_fix(Fix::applicable_edit( + Edit::replacement(replacement, if_expr.start(), if_expr.end()), + applicability, + )); } } } @@ -122,11 +131,16 @@ pub(crate) fn slice_to_remove_affix_stmt(checker: &Checker, if_stmt: &ast::StmtI checker.locator(), ); - diagnostic.set_fix(Fix::safe_edit(Edit::replacement( - replacement, - if_stmt.start(), - if_stmt.end(), - ))); + let applicability = if checker.comment_ranges().intersects(if_stmt.range) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.set_fix(Fix::applicable_edit( + Edit::replacement(replacement, if_stmt.start(), if_stmt.end()), + applicability, + )); } } } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap index 21e50d3130..4889b6e7fb 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap @@ -317,3 +317,28 @@ help: Use removesuffix instead of assignment conditional upon endswith. - a = a[: -len("foo")] 205 + a = a.removesuffix("foo") 206 | print(a) +207 | +208 | + +FURB188 [*] Prefer `str.removesuffix()` over conditionally replacing with slice. + --> FURB188.py:212:9 + | +210 | def example(filename: str, text: str): +211 | filename = ( +212 | / filename[:-4] # text +213 | | if filename.endswith(".txt") # text +214 | | else filename + | |_____________________^ +215 | ) + | +help: Use removesuffix instead of ternary expression conditional upon endswith. +209 | +210 | def example(filename: str, text: str): +211 | filename = ( + - filename[:-4] # text + - if filename.endswith(".txt") # text + - else filename +212 + filename.removesuffix(".txt") +213 | ) +214 | +note: This is an unsafe fix and may change runtime behavior