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

There was already handling for the singleton `x[:]` case but not the
tuple case.

Closes https://github.com/astral-sh/ruff/issues/13508
This commit is contained in:
Zanie Blue 2024-09-26 09:20:03 -05:00 committed by GitHub
parent e83388dcea
commit 58a8e9c511
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 122 additions and 1 deletions

View file

@ -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]

View file

@ -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)))

View file

@ -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))