Fix handling of slices in tuples for FURB118, e.g., x[:, 1]

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

@@ -83,3 +83,13 @@ 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, :]
+
+# With a slice, trivia is dropped
+op_itemgetter = lambda x: x[1,          :]
+
+# Without a slice, trivia is retained
+op_itemgetter = lambda x: x[1,          2]

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

@@ -3,6 +3,7 @@ use std::fmt::{Debug, Display, Formatter};
 
 use anyhow::Result;
 
+use itertools::Itertools;
 use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::any_over_expr;
@@ -213,7 +214,18 @@ 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 {
+        if locator.slice(tuple).contains(':') {
+            // We cannot perform a trivial replacement if there's a `:` in the expression
+            let inner = tuple
+                .iter()
+                .map(|expr| match expr {
+                    Expr::Slice(expr) => Cow::Owned(slice_expr_to_slice_call(expr, locator)),
+                    _ => Cow::Borrowed(locator.slice(expr)),
+                })
+                .join(", ");
+
+            Cow::Owned(format!("({inner})"))
+        } else if tuple.parenthesized {
             Cow::Borrowed(locator.slice(expr))
         } else {
             Cow::Owned(format!("({})", locator.slice(tuple)))

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

@@ -847,3 +847,102 @@ 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, :]
+90 91 | 
+91 92 | # With a slice, trivia is dropped
+
+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
+90 | 
+91 | # With a slice, trivia is dropped
+   |
+   = 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)))
+90 91 | 
+91 92 | # With a slice, trivia is dropped
+92 93 | op_itemgetter = lambda x: x[1,          :]
+
+FURB118.py:92:17: FURB118 [*] Use `operator.itemgetter((1, slice(None)))` instead of defining a lambda
+   |
+91 | # With a slice, trivia is dropped
+92 | op_itemgetter = lambda x: x[1,          :]
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB118
+93 | 
+94 | # Without a slice, trivia is retained
+   |
+   = 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
+--------------------------------------------------------------------------------
+89 90 | op_itemgetter = lambda x: x[1, :]
+90 91 | 
+91 92 | # With a slice, trivia is dropped
+92    |-op_itemgetter = lambda x: x[1,          :]
+   93 |+op_itemgetter = operator.itemgetter((1, slice(None)))
+93 94 | 
+94 95 | # Without a slice, trivia is retained
+95 96 | op_itemgetter = lambda x: x[1,          2]
+
+FURB118.py:95:17: FURB118 [*] Use `operator.itemgetter((1,          2))` instead of defining a lambda
+   |
+94 | # Without a slice, trivia is retained
+95 | op_itemgetter = lambda x: x[1,          2]
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB118
+   |
+   = help: Replace with `operator.itemgetter((1,          2))`
+
+ℹ 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
+--------------------------------------------------------------------------------
+92 93 | op_itemgetter = lambda x: x[1,          :]
+93 94 | 
+94 95 | # Without a slice, trivia is retained
+95    |-op_itemgetter = lambda x: x[1,          2]
+   96 |+op_itemgetter = operator.itemgetter((1,          2))