[refurb] Make fix unsafe if it deletes comments (FURB188) (#22665)

<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

<!-- How was it tested? -->
This commit is contained in:
chiri
2026-01-20 19:51:36 +03:00
committed by GitHub
parent fa42b09e3f
commit 8b5fbc240a
3 changed files with 58 additions and 10 deletions

View File

@@ -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
)

View File

@@ -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,
));
}
}
}

View File

@@ -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