Skip to content

Commit

Permalink
Fix handling of slices in tuples for FURB118, e.g., x[:, 1]
Browse files Browse the repository at this point in the history
There was already handling for the singleton `x[:]` case but not the tuple case.\nCloses #13508
  • Loading branch information
zanieb committed Sep 26, 2024
1 parent 7c83af4 commit 740fe53
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 5 deletions.
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, :]
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
.join(", ");

Cow::Owned(format!("({inner})"))
} else {
Cow::Borrowed(locator.slice(expr))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

0 comments on commit 740fe53

Please sign in to comment.