Skip to content

Commit

Permalink
Add special case for lambda x: x[:] -> list.copy for FURB118:
Browse files Browse the repository at this point in the history
When `x` is a `list` type you can use `list.copy` instead to make a copy. This
is an improvement over using `operator.itemgetter` since it's more readable.
  • Loading branch information
dosisod committed Feb 7, 2024
1 parent 762aedb commit e178921
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
14 changes: 13 additions & 1 deletion refurb/checks/builtin/no_slice_copy.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
from dataclasses import dataclass

from mypy.nodes import AssignmentStmt, DelStmt, IndexExpr, MypyFile, RefExpr, SliceExpr, Var
from mypy.nodes import (
AssignmentStmt,
DelStmt,
IndexExpr,
LambdaExpr,
MypyFile,
RefExpr,
SliceExpr,
Var,
)

from refurb.checks.common import is_same_type, stringify
from refurb.error import Error
Expand Down Expand Up @@ -44,6 +53,9 @@ def __init__(self, errors: list[Error]) -> None:
def visit_assignment_stmt(self, node: AssignmentStmt) -> None:
self.accept(node.rvalue)

def visit_lambda_expr(self, node: LambdaExpr) -> None: # noqa: PLR6301, ARG002
return

def visit_del_stmt(self, node: DelStmt) -> None:
if not isinstance(node.expr, IndexExpr):
self.accept(node.expr)
Expand Down
25 changes: 17 additions & 8 deletions refurb/checks/readability/use_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
SliceExpr,
TupleExpr,
UnaryExpr,
Var,
)

from refurb.checks.common import _stringify, slice_expr_to_slice_call, stringify
from refurb.checks.common import _stringify, is_same_type, slice_expr_to_slice_call, stringify
from refurb.error import Error


Expand Down Expand Up @@ -163,16 +164,24 @@ def check(node: FuncItem, errors: list[Error]) -> None:
)

case IndexExpr(
base=NameExpr(name=item_name),
base=NameExpr(name=item_name, node=Var(type=ty)),
index=index,
) if item_name == name:
index_expr = (
slice_expr_to_slice_call(index)
if isinstance(index, SliceExpr)
else stringify(index)
)
match index:
case SliceExpr(
begin_index=None,
end_index=None,
stride=None,
) if is_same_type(ty, list):
new = "list.copy"

case SliceExpr():
new = f"operator.itemgetter({slice_expr_to_slice_call(index)})"

case _:
new = f"operator.itemgetter({stringify(index)})"

msg = f"Replace {func_type} with `operator.itemgetter({index_expr})`"
msg = f"Replace {func_type} with `{new}`"

errors.append(ErrorInfo.from_node(node, msg))

Expand Down
5 changes: 5 additions & 0 deletions test/data/err_118.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ def f2(x):
lambda x: (x[0], x[1], x[2])
lambda x: (x[1:], x[2])

# map() is needed for mypy to be able to deduce type of arg x
map(lambda x: x[:], [[]])

# since we cannot deduce the type of x we can't safely use list.copy, default to itemgetter
lambda x: x[:]

# these will not

Expand Down
2 changes: 2 additions & 0 deletions test/data/err_118.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,5 @@ test/data/err_118.py:34:1 [FURB118]: Replace function with `operator.neg`
test/data/err_118.py:37:1 [FURB118]: Replace `lambda x: x[0]` with `operator.itemgetter(0)`
test/data/err_118.py:39:1 [FURB118]: Replace `lambda x: (x[0], x[1], x[2])` with `operator.itemgetter(0, 1, 2)`
test/data/err_118.py:40:1 [FURB118]: Replace `lambda x: (x[1:], x[2])` with `operator.itemgetter(slice(1, None), 2)`
test/data/err_118.py:43:5 [FURB118]: Replace `lambda x: x[:]` with `list.copy`
test/data/err_118.py:46:1 [FURB118]: Replace `lambda x: x[:]` with `operator.itemgetter(slice(None, None))`
4 changes: 4 additions & 0 deletions test/data/err_145.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,7 @@ def __getitem__(self, key):

b = b"abc"
_ = b[:]


# special case that conflicts with FURB118, ignore
map(lambda x: x[:], [[]]) # noqa: FURB118

0 comments on commit e178921

Please sign in to comment.