diff --git a/docs/checks.md b/docs/checks.md index 662378e..f09c805 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -2435,4 +2435,29 @@ Good: ```python if isinstance(value, bool): pass +``` + +## FURB192: `no-sorted-min-max` + +Categories: `builtin` `performance` `readability` + +Don't use `sorted()` to get the min/max value out of an iterable element, +use `min()` or `max()`. + +Bad: + +```python +nums = [3, 1, 4, 1, 5] + +lowest = sorted(nums)[0] +highest = sorted(nums)[-1] +``` + +Good: + +```python +nums = [3, 1, 4, 1, 5] + +lowest = min(nums) +highest = max(nums) ``` \ No newline at end of file diff --git a/refurb/checks/builtin/no_sorted_min_max.py b/refurb/checks/builtin/no_sorted_min_max.py new file mode 100644 index 0000000..563f899 --- /dev/null +++ b/refurb/checks/builtin/no_sorted_min_max.py @@ -0,0 +1,74 @@ +from dataclasses import dataclass + +from mypy.nodes import CallExpr, IndexExpr, IntExpr, NameExpr, UnaryExpr + +from refurb.checks.common import stringify +from refurb.error import Error + + +@dataclass +class ErrorInfo(Error): + """ + Don't use `sorted()` to get the min/max value out of an iterable element, + use `min()` or `max()`. + + Bad: + + ``` + nums = [3, 1, 4, 1, 5] + + lowest = sorted(nums)[0] + highest = sorted(nums)[-1] + ``` + + Good: + + ``` + nums = [3, 1, 4, 1, 5] + + lowest = min(nums) + highest = max(nums) + ``` + """ + + name = "no-sorted-min-max" + code = 192 + categories = ("builtin", "performance", "readability") + + +def check(node: IndexExpr, errors: list[Error]) -> None: + match node: + case IndexExpr( + base=CallExpr( + callee=NameExpr(fullname="builtins.sorted"), + args=[arg1, *args], + arg_names=[_, *arg_names], + ), + index=IntExpr(value=0) | UnaryExpr(op="-", expr=IntExpr(value=1)) as index, + ): + is_zero_index = isinstance(index, IntExpr) + is_reversed = False + key = "" + + for arg_name, arg in zip(arg_names, args): + if arg_name == "reverse": + match arg: + case NameExpr(fullname="builtins.True"): + is_reversed = True + + case _: + return + + elif arg_name == "key": + key = f", key={stringify(arg)}" + + else: + return + + func = "max" if is_zero_index == is_reversed else "min" + + old = stringify(node) + new = f"{func}({stringify(arg1)}{key})" + msg = f"Replace `{old}` with `{new}`" + + errors.append(ErrorInfo.from_node(node, msg)) diff --git a/test/data/err_192.py b/test/data/err_192.py new file mode 100644 index 0000000..424e771 --- /dev/null +++ b/test/data/err_192.py @@ -0,0 +1,25 @@ +l = [] + +# these should match + +_ = sorted(l)[0] +_ = sorted(l)[-1] +_ = sorted(l, reverse=True)[0] +_ = sorted(l, reverse=True)[-1] + +_ = sorted(l, key=lambda x: x)[0] +_ = sorted(l, key=lambda x: x)[-1] +_ = sorted(l, key=lambda x: x, reverse=True)[0] +_ = sorted(l, key=lambda x: x, reverse=True)[-1] + + +# these should not + +_ = sorted()[0] # type: ignore +_ = sorted(l)[1] +_ = sorted(l)[-2] + +b = True + +_ = sorted(l, reverse=b)[0] +_ = sorted(l, invalid_kwarg=True)[0] # type: ignore diff --git a/test/data/err_192.txt b/test/data/err_192.txt new file mode 100644 index 0000000..0115c67 --- /dev/null +++ b/test/data/err_192.txt @@ -0,0 +1,8 @@ +test/data/err_192.py:5:5 [FURB192]: Replace `sorted(l)[0]` with `min(l)` +test/data/err_192.py:6:5 [FURB192]: Replace `sorted(l)[-1]` with `max(l)` +test/data/err_192.py:7:5 [FURB192]: Replace `sorted(l, reverse=True)[0]` with `max(l)` +test/data/err_192.py:8:5 [FURB192]: Replace `sorted(l, reverse=True)[-1]` with `min(l)` +test/data/err_192.py:10:5 [FURB192]: Replace `sorted(l, key=lambda x: x)[0]` with `min(l, key=lambda x: x)` +test/data/err_192.py:11:5 [FURB192]: Replace `sorted(l, key=lambda x: x)[-1]` with `max(l, key=lambda x: x)` +test/data/err_192.py:12:5 [FURB192]: Replace `sorted(l, key=lambda x: x, reverse=True)[0]` with `max(l, key=lambda x: x)` +test/data/err_192.py:13:5 [FURB192]: Replace `sorted(l, key=lambda x: x, reverse=True)[-1]` with `min(l, key=lambda x: x)`