diff --git a/refurb/checks/common.py b/refurb/checks/common.py index 39b8494..1105326 100644 --- a/refurb/checks/common.py +++ b/refurb/checks/common.py @@ -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 diff --git a/refurb/checks/readability/use_sort.py b/refurb/checks/readability/use_sort.py new file mode 100644 index 0000000..f0a7f52 --- /dev/null +++ b/refurb/checks/readability/use_sort.py @@ -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)) diff --git a/test/data/err_186.py b/test/data/err_186.py new file mode 100644 index 0000000..5ff04a1 --- /dev/null +++ b/test/data/err_186.py @@ -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 diff --git a/test/data/err_186.txt b/test/data/err_186.txt new file mode 100644 index 0000000..36dc140 --- /dev/null +++ b/test/data/err_186.txt @@ -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)`