From f110d80279f95d5a9d798b2991c895331311c20f Mon Sep 17 00:00:00 2001 From: Dylan <53534755+dylwil3@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:47:17 -0500 Subject: [PATCH] [refurb] Skip `slice-to-remove-prefix-or-suffix (FURB188)` when nontrivial slice step is present (#13405) --- .../resources/test/fixtures/refurb/FURB188.py | 20 ++++- .../rules/slice_to_remove_prefix_or_suffix.rs | 21 +++++ ...es__refurb__tests__FURB188_FURB188.py.snap | 76 +++++++++++++++++++ 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py index 3437d5c56bec4..45a39257f3255 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py +++ b/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 \ No newline at end of file + 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) \ No newline at end of file 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 1bc491a0fb53b..e61cb1dc13696 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 @@ -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, ); 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 3103e5f2723ad..89a0c17633e70 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 @@ -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 |