[refurb] Fix misbehavior of operator.itemgetter when getter param…

… is a tuple (#11774)
astral-sh · Jun 7, 2024 · ea27445 · ea27445
1 parent 540d768
commit ea27445
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 12 deletions.
diff --git 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
@@ -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/...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap b/...ter/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,15 +751,17 @@ 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
    |
 31 | op_itemgetter = lambda x: (x[0], x[1], x[2])
 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`