From ea27445479a07ff76f831a86d16920d1b7ab1524 Mon Sep 17 00:00:00 2001 From: Embers-of-the-Fire Date: Fri, 7 Jun 2024 11:10:52 +0800 Subject: [PATCH] [`refurb`] Fix misbehavior of `operator.itemgetter` when getter param is a tuple (#11774) --- .../resources/test/fixtures/refurb/FURB118.py | 2 + .../refurb/rules/reimplemented_operator.rs | 6 ++ ...es__refurb__tests__FURB118_FURB118.py.snap | 78 ++++++++++++++++--- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py index cf4e79b13a6e2..562efacf85230 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py @@ -31,6 +31,8 @@ op_itemgetter = lambda x: (x[0], x[1], x[2]) op_itemgetter = lambda x: (x[1:], x[2]) op_itemgetter = lambda x: x[:] +op_itemgetter = lambda x: x[0, 1] +op_itemgetter = lambda x: x[(0, 1)] def op_not2(x): 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 d2c98b8d20727..f33b903e68f68 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -212,6 +212,12 @@ fn slice_expr_to_slice_call(slice: &ExprSlice, locator: &Locator) -> String { fn subscript_slice_to_string<'a>(expr: &Expr, locator: &Locator<'a>) -> Cow<'a, str> { 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))) + } } 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 daab20b991ae3..fdfe0dbfbe818 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 @@ -725,7 +725,7 @@ FURB118.py:31:17: FURB118 [*] Use `operator.itemgetter(0, 1, 2)` instead of defi 32 |+op_itemgetter = operator.itemgetter(0, 1, 2) 32 33 | op_itemgetter = lambda x: (x[1:], x[2]) 33 34 | op_itemgetter = lambda x: x[:] -34 35 | +34 35 | op_itemgetter = lambda x: x[0, 1] FURB118.py:32:17: FURB118 [*] Use `operator.itemgetter(slice(1, None), 2)` instead of defining a lambda | @@ -734,6 +734,7 @@ FURB118.py:32:17: FURB118 [*] Use `operator.itemgetter(slice(1, None), 2)` inste 32 | op_itemgetter = lambda x: (x[1:], x[2]) | ^^^^^^^^^^^^^^^^^^^^^^^ FURB118 33 | op_itemgetter = lambda x: x[:] +34 | op_itemgetter = lambda x: x[0, 1] | = help: Replace with `operator.itemgetter(slice(1, None), 2)` @@ -750,8 +751,8 @@ FURB118.py:32:17: FURB118 [*] Use `operator.itemgetter(slice(1, None), 2)` inste 32 |-op_itemgetter = lambda x: (x[1:], x[2]) 33 |+op_itemgetter = operator.itemgetter(slice(1, None), 2) 33 34 | op_itemgetter = lambda x: x[:] -34 35 | -35 36 | +34 35 | op_itemgetter = lambda x: x[0, 1] +35 36 | op_itemgetter = lambda x: x[(0, 1)] FURB118.py:33:17: FURB118 [*] Use `operator.itemgetter(slice(None))` instead of defining a lambda | @@ -759,6 +760,8 @@ FURB118.py:33:17: FURB118 [*] Use `operator.itemgetter(slice(None))` instead of 32 | op_itemgetter = lambda x: (x[1:], x[2]) 33 | op_itemgetter = lambda x: x[:] | ^^^^^^^^^^^^^^ FURB118 +34 | op_itemgetter = lambda x: x[0, 1] +35 | op_itemgetter = lambda x: x[(0, 1)] | = help: Replace with `operator.itemgetter(slice(None))` @@ -774,22 +777,73 @@ FURB118.py:33:17: FURB118 [*] Use `operator.itemgetter(slice(None))` instead of 32 33 | op_itemgetter = lambda x: (x[1:], x[2]) 33 |-op_itemgetter = lambda x: x[:] 34 |+op_itemgetter = operator.itemgetter(slice(None)) -34 35 | -35 36 | -36 37 | def op_not2(x): +34 35 | op_itemgetter = lambda x: x[0, 1] +35 36 | op_itemgetter = lambda x: x[(0, 1)] +36 37 | -FURB118.py:36:5: FURB118 Use `operator.not_` instead of defining a function +FURB118.py:34:17: FURB118 [*] Use `operator.itemgetter((0, 1))` instead of defining a lambda | -36 | def op_not2(x): +32 | op_itemgetter = lambda x: (x[1:], x[2]) +33 | op_itemgetter = lambda x: x[:] +34 | op_itemgetter = lambda x: x[0, 1] + | ^^^^^^^^^^^^^^^^^ FURB118 +35 | op_itemgetter = lambda x: x[(0, 1)] + | + = help: Replace with `operator.itemgetter((0, 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 +-------------------------------------------------------------------------------- +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) +33 34 | op_itemgetter = lambda x: x[:] +34 |-op_itemgetter = lambda x: x[0, 1] + 35 |+op_itemgetter = operator.itemgetter((0, 1)) +35 36 | op_itemgetter = lambda x: x[(0, 1)] +36 37 | +37 38 | + +FURB118.py:35:17: FURB118 [*] Use `operator.itemgetter((0, 1))` instead of defining a lambda + | +33 | op_itemgetter = lambda x: x[:] +34 | op_itemgetter = lambda x: x[0, 1] +35 | op_itemgetter = lambda x: x[(0, 1)] + | ^^^^^^^^^^^^^^^^^^^ FURB118 + | + = help: Replace with `operator.itemgetter((0, 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 +-------------------------------------------------------------------------------- +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) +33 34 | op_itemgetter = lambda x: x[:] +34 35 | op_itemgetter = lambda x: x[0, 1] +35 |-op_itemgetter = lambda x: x[(0, 1)] + 36 |+op_itemgetter = operator.itemgetter((0, 1)) +36 37 | +37 38 | +38 39 | def op_not2(x): + +FURB118.py:38:5: FURB118 Use `operator.not_` instead of defining a function + | +38 | def op_not2(x): | ^^^^^^^ FURB118 -37 | return not x +39 | return not x | = help: Replace with `operator.not_` -FURB118.py:40:5: FURB118 Use `operator.add` instead of defining a function +FURB118.py:42:5: FURB118 Use `operator.add` instead of defining a function | -40 | def op_add2(x, y): +42 | def op_add2(x, y): | ^^^^^^^ FURB118 -41 | return x + y +43 | return x + y | = help: Replace with `operator.add`