From 740fe538db927ab993c35b6f399831b4c556472b Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 25 Sep 2024 19:02:13 -0500 Subject: [PATCH] Fix handling of slices in tuples for FURB118, e.g., `x[:, 1]` There was already handling for the singleton `x[:]` case but not the tuple case.\nCloses https://github.com/astral-sh/ruff/issues/13508 --- .../resources/test/fixtures/refurb/FURB118.py | 4 ++ .../refurb/rules/reimplemented_operator.rs | 16 ++++--- ...es__refurb__tests__FURB118_FURB118.py.snap | 45 +++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py index 562efacf852302..140819684f5944 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py @@ -83,3 +83,7 @@ class Class: @staticmethod def add(x, y): return x + y + +# See https://github.com/astral-sh/ruff/issues/13508 +op_itemgetter = lambda x: x[:, 1] +op_itemgetter = lambda x: x[1, :] diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs index 8d550e22d18cbe..f770a6f64b6aff 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -213,11 +213,17 @@ fn subscript_slice_to_string<'a>(expr: &Expr, locator: &Locator<'a>) -> Cow<'a, if let Expr::Slice(expr_slice) = expr { Cow::Owned(slice_expr_to_slice_call(expr_slice, locator)) } else if let Expr::Tuple(tuple) = expr { - if tuple.parenthesized { - Cow::Borrowed(locator.slice(expr)) - } else { - Cow::Owned(format!("({})", locator.slice(tuple))) - } + let inner = tuple + .elts + .iter() + .map(|expr| match expr { + Expr::Slice(expr) => Cow::Owned(slice_expr_to_slice_call(expr, locator)), + _ => Cow::Borrowed(locator.slice(expr)), + }) + .collect::>() + .join(", "); + + Cow::Owned(format!("({inner})")) } else { Cow::Borrowed(locator.slice(expr)) } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap index fdfe0dbfbe8187..4fbaf992e3526c 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap @@ -847,3 +847,48 @@ FURB118.py:42:5: FURB118 Use `operator.add` instead of defining a function 43 | return x + y | = help: Replace with `operator.add` + +FURB118.py:88:17: FURB118 [*] Use `operator.itemgetter((slice(None), 1))` instead of defining a lambda + | +87 | # See https://github.com/astral-sh/ruff/issues/13508 +88 | op_itemgetter = lambda x: x[:, 1] + | ^^^^^^^^^^^^^^^^^ FURB118 +89 | op_itemgetter = lambda x: x[1, :] + | + = help: Replace with `operator.itemgetter((slice(None), 1))` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +85 86 | return x + y +86 87 | +87 88 | # See https://github.com/astral-sh/ruff/issues/13508 +88 |-op_itemgetter = lambda x: x[:, 1] + 89 |+op_itemgetter = operator.itemgetter((slice(None), 1)) +89 90 | op_itemgetter = lambda x: x[1, :] + +FURB118.py:89:17: FURB118 [*] Use `operator.itemgetter((1, slice(None)))` instead of defining a lambda + | +87 | # See https://github.com/astral-sh/ruff/issues/13508 +88 | op_itemgetter = lambda x: x[:, 1] +89 | op_itemgetter = lambda x: x[1, :] + | ^^^^^^^^^^^^^^^^^ FURB118 + | + = help: Replace with `operator.itemgetter((1, slice(None)))` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +86 87 | +87 88 | # See https://github.com/astral-sh/ruff/issues/13508 +88 89 | op_itemgetter = lambda x: x[:, 1] +89 |-op_itemgetter = lambda x: x[1, :] + 90 |+op_itemgetter = operator.itemgetter((1, slice(None)))