[refurb] Skip slice-to-remove-prefix-or-suffix (FURB188) when nontrivial slice step is present (#13405)

Author: dylwil3

crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py

@@ -151,4 +151,22 @@ def remove_prefix_comparable_literal_expr() -> None:
 def shadow_builtins(filename: str, extension: str) -> None:
     from builtins import len as builtins_len
 
-    return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
+    return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
+
+def okay_steps():
+    text = "!x!y!z"
+    if text.startswith("!"):
+        text = text[1::1]
+    if text.startswith("!"):
+        text = text[1::True]
+    if text.startswith("!"):
+        text = text[1::None]
+    print(text)
+
+
+# this should be skipped
+def ignore_step():
+    text = "!x!y!z"
+    if text.startswith("!"):
+        text = text[1::2]
+    print(text)

crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs

@@ -246,6 +246,27 @@ fn affix_removal_data<'a>(
         return None;
     }
     let slice = slice.as_slice_expr()?;
+
+    // Exit early if slice step is...
+    if slice
+        .step
+        .as_deref()
+        // present and
+        .is_some_and(|step| match step {
+            // not equal to 1
+            ast::Expr::NumberLiteral(ast::ExprNumberLiteral {
+                value: ast::Number::Int(x),
+                ..
+            }) => x.as_u8() != Some(1),
+            // and not equal to `None` or `True`
+            ast::Expr::NoneLiteral(_)
+            | ast::Expr::BooleanLiteral(ast::ExprBooleanLiteral { value: true, .. }) => false,
+            _ => true,
+        })
+    {
+        return None;
+    };
+
     let compr_test_expr = ast::comparable::ComparableExpr::from(
         &test.as_call_expr()?.func.as_attribute_expr()?.value,
     );

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap

@@ -166,6 +166,8 @@ FURB188.py:154:12: FURB188 [*] Prefer `removesuffix` over conditionally replacin
 153 | 
 154 |     return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
     |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
+155 | 
+156 | def okay_steps():
     |
     = help: Use removesuffix instead of ternary expression conditional upon endswith.
 
@@ -175,3 +177,77 @@ FURB188.py:154:12: FURB188 [*] Prefer `removesuffix` over conditionally replacin
 153 153 | 
 154     |-    return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
     154 |+    return filename.removesuffix(extension)
+155 155 | 
+156 156 | def okay_steps():
+157 157 |     text = "!x!y!z"
+
+FURB188.py:158:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
+    |
+156 |   def okay_steps():
+157 |       text = "!x!y!z"
+158 |       if text.startswith("!"):
+    |  _____^
+159 | |         text = text[1::1]
+    | |_________________________^ FURB188
+160 |       if text.startswith("!"):
+161 |           text = text[1::True]
+    |
+    = help: Use removeprefix instead of assignment conditional upon startswith.
+
+ℹ Safe fix
+155 155 | 
+156 156 | def okay_steps():
+157 157 |     text = "!x!y!z"
+158     |-    if text.startswith("!"):
+159     |-        text = text[1::1]
+    158 |+    text = text.removeprefix("!")
+160 159 |     if text.startswith("!"):
+161 160 |         text = text[1::True]
+162 161 |     if text.startswith("!"):
+
+FURB188.py:160:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
+    |
+158 |       if text.startswith("!"):
+159 |           text = text[1::1]
+160 |       if text.startswith("!"):
+    |  _____^
+161 | |         text = text[1::True]
+    | |____________________________^ FURB188
+162 |       if text.startswith("!"):
+163 |           text = text[1::None]
+    |
+    = help: Use removeprefix instead of assignment conditional upon startswith.
+
+ℹ Safe fix
+157 157 |     text = "!x!y!z"
+158 158 |     if text.startswith("!"):
+159 159 |         text = text[1::1]
+160     |-    if text.startswith("!"):
+161     |-        text = text[1::True]
+    160 |+    text = text.removeprefix("!")
+162 161 |     if text.startswith("!"):
+163 162 |         text = text[1::None]
+164 163 |     print(text)
+
+FURB188.py:162:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
+    |
+160 |       if text.startswith("!"):
+161 |           text = text[1::True]
+162 |       if text.startswith("!"):
+    |  _____^
+163 | |         text = text[1::None]
+    | |____________________________^ FURB188
+164 |       print(text)
+    |
+    = help: Use removeprefix instead of assignment conditional upon startswith.
+
+ℹ Safe fix
+159 159 |         text = text[1::1]
+160 160 |     if text.startswith("!"):
+161 161 |         text = text[1::True]
+162     |-    if text.startswith("!"):
+163     |-        text = text[1::None]
+    162 |+    text = text.removeprefix("!")
+164 163 |     print(text)
+165 164 | 
+166 165 |