Skip to content

Commit

Permalink
Add use-sort check
Browse files Browse the repository at this point in the history
  • Loading branch information
dosisod committed Jan 7, 2024
1 parent 661f756 commit 5986070
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 1 deletion.
2 changes: 1 addition & 1 deletion refurb/checks/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ def _stringify(node: Node) -> str:
return f"{_stringify(expr)}.{name}"

case NameExpr(name=name):
return name
return unmangle_name(name)

case BytesExpr(value=value):
# TODO: use same formatting as source line
Expand Down
74 changes: 74 additions & 0 deletions refurb/checks/readability/use_sort.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
from dataclasses import dataclass

from mypy.nodes import ArgKind, AssignmentStmt, CallExpr, NameExpr, Var

from refurb.checks.common import stringify, unmangle_name
from refurb.error import Error


@dataclass
class ErrorInfo(Error):
"""
Don't use `sorted()` to sort a list and reassign it to itself, use the
faster in-place `.sort()` method instead.
Bad:
```
names = ["Bob", "Alice", "Charlie"]
names = sorted(names)
```
Good:
```
names = ["Bob", "Alice", "Charlie"]
names.sort()
```
"""

name = "use-sort"
categories = ("performance", "readability")
code = 186


def check(node: AssignmentStmt, errors: list[Error]) -> None:
match node:
case AssignmentStmt(
lvalues=[NameExpr(fullname=assign_name) as assign_ref],
rvalue=CallExpr(
callee=NameExpr(fullname="builtins.sorted"),
args=[
NameExpr(fullname=sort_name, node=Var(type=ty)),
*rest,
],
arg_names=[_, *arg_names],
arg_kinds=[_, *arg_kinds],
),
) if (
unmangle_name(assign_name) == unmangle_name(sort_name)
and str(ty).startswith("builtins.list[")
and all(arg_kind == ArgKind.ARG_NAMED for arg_kind in arg_kinds)
):
old_args: list[str] = []
new_args: list[str] = []

name = stringify(assign_ref)

old_args.append(name)

if rest:
for arg_name, expr in zip(arg_names, rest):
arg = f"{arg_name}={stringify(expr)}"

old_args.append(arg)
new_args.append(arg)

old = f"{name} = sorted({', '.join(old_args)})"
new = f"{name}.sort({', '.join(new_args)})"

msg = f"Replace `{old}` with `{new}`"

errors.append(ErrorInfo.from_node(node, msg))
22 changes: 22 additions & 0 deletions test/data/err_186.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
l = []

# these should match

l = sorted(l)
l = sorted(l, key=lambda x: x > 0)
l = sorted(l, reverse=True)
l = sorted(l, key=lambda x: x > 0, reverse=True)


# these should not

l2 = sorted(l)
l2 = sorted(l, key=lambda x: x > 0)
l2 = sorted(l, reverse=True)

d = {}

# dont warn since d is a dict and does not have a .sort() method
d = sorted(d)

l = sorted(l, lambda x: x) # type: ignore
4 changes: 4 additions & 0 deletions test/data/err_186.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
test/data/err_186.py:5:1 [FURB186]: Replace `l = sorted(l)` with `l.sort()`
test/data/err_186.py:6:1 [FURB186]: Replace `l = sorted(l, key=lambda x: x > 0)` with `l.sort(key=lambda x: x > 0)`
test/data/err_186.py:7:1 [FURB186]: Replace `l = sorted(l, reverse=True)` with `l.sort(reverse=True)`
test/data/err_186.py:8:1 [FURB186]: Replace `l = sorted(l, key=lambda x: x > 0, reverse=True)` with `l.sort(key=lambda x: x > 0, reverse=True)`

0 comments on commit 5986070

Please sign in to comment.