Skip to content

Commit

Permalink
Add no-sorted-min-max check (FURB192):
Browse files Browse the repository at this point in the history
Closes #332.
  • Loading branch information
dosisod committed Mar 20, 2024
1 parent 589afd6 commit d5c69b3
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 0 deletions.
25 changes: 25 additions & 0 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
```
74 changes: 74 additions & 0 deletions refurb/checks/builtin/no_sorted_min_max.py
Original file line number Diff line number Diff line change
@@ -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))
25 changes: 25 additions & 0 deletions test/data/err_192.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions test/data/err_192.txt
Original file line number Diff line number Diff line change
@@ -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)`

0 comments on commit d5c69b3

Please sign in to comment.