mirror of https://github.com/astral-sh/ruff
[`refurb`] Correctly handle lengths of literal strings in `slice-to-remove-prefix-or-suffix` (`FURB188`) (#16237)
Fixes false negative when slice bound uses length of string literal.
We were meant to check the following, for example. Given:
```python
text[:bound] if text.endswith(suffix) else text
```
We want to know whether:
- `suffix` is a string literal and `bound` is a number literal
- `suffix` is an expression and `bound` is
exactly `-len(suffix)` (as AST nodes, prior to evaluation.)
The issue is that negative number literals like `-10` are stored as
unary operators applied to a number literal in the AST. So when `suffix`
was a string literal but `bound` was `-len(suffix)` we were getting
caught in the match arm where `bound` needed to be a number. This is now
fixed with a guard.
Closes #16231
This commit is contained in:
parent
1907e60fab
commit
a23e489c79
|
|
@ -198,3 +198,10 @@ def handle_surrogates():
|
||||||
if text.startswith("\ud800\udc00"):
|
if text.startswith("\ud800\udc00"):
|
||||||
text = text[1:]
|
text = text[1:]
|
||||||
|
|
||||||
|
# Regression test for
|
||||||
|
# https://github.com/astral-sh/ruff/issues/16231
|
||||||
|
def func():
|
||||||
|
a = "sjdfaskldjfakljklfoo"
|
||||||
|
if a.endswith("foo"):
|
||||||
|
a = a[: -len("foo")]
|
||||||
|
print(a)
|
||||||
|
|
|
||||||
|
|
@ -365,7 +365,7 @@ fn affix_matches_slice_bound(data: &RemoveAffixData, semantic: &SemanticModel) -
|
||||||
range: _,
|
range: _,
|
||||||
value: string_val,
|
value: string_val,
|
||||||
}),
|
}),
|
||||||
) => operand.as_number_literal_expr().is_some_and(
|
) if operand.is_number_literal_expr() => operand.as_number_literal_expr().is_some_and(
|
||||||
|ast::ExprNumberLiteral { value, .. }| {
|
|ast::ExprNumberLiteral { value, .. }| {
|
||||||
// Only support prefix removal for size at most `u32::MAX`
|
// Only support prefix removal for size at most `u32::MAX`
|
||||||
value
|
value
|
||||||
|
|
|
||||||
|
|
@ -313,3 +313,23 @@ FURB188.py:193:5: FURB188 [*] Prefer `str.removeprefix()` over conditionally rep
|
||||||
195 194 |
|
195 194 |
|
||||||
196 195 | # should not be linted
|
196 195 | # should not be linted
|
||||||
197 196 | text = "\ud800\udc00heythere"
|
197 196 | text = "\ud800\udc00heythere"
|
||||||
|
|
||||||
|
FURB188.py:205:5: FURB188 [*] Prefer `str.removesuffix()` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
203 | def func():
|
||||||
|
204 | a = "sjdfaskldjfakljklfoo"
|
||||||
|
205 | / if a.endswith("foo"):
|
||||||
|
206 | | a = a[: -len("foo")]
|
||||||
|
| |____________________________^ FURB188
|
||||||
|
207 | print(a)
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of assignment conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
202 202 | # https://github.com/astral-sh/ruff/issues/16231
|
||||||
|
203 203 | def func():
|
||||||
|
204 204 | a = "sjdfaskldjfakljklfoo"
|
||||||
|
205 |- if a.endswith("foo"):
|
||||||
|
206 |- a = a[: -len("foo")]
|
||||||
|
205 |+ a = a.removesuffix("foo")
|
||||||
|
207 206 | print(a)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue