Add FURB187 use-reverse

docs/checks.md

@@ -2300,4 +2300,31 @@ Good:
 names = ["Bob", "Alice", "Charlie"]
 
 names.sort()
+```
+
+## FURB187: `use-reverse`
+
+Categories: `performance` `readability`
+
+Don't use `x[::-1]` or `reversed(x)` to reverse a list and reassign it to
+itself, use the faster in-place `.reverse()` method instead.
+
+Bad:
+
+```python
+names = ["Bob", "Alice", "Charlie"]
+
+names = reverse(names)
+# or
+names = list(reverse(names))
+# or
+names = names[::-1]
+```
+
+Good:
+
+```python
+names = ["Bob", "Alice", "Charlie"]
+
+names.reverse()
 ```

refurb/checks/common.py

@@ -3,6 +3,7 @@
 
 from mypy.nodes import (
     ArgKind,
+    AssignmentStmt,
     Block,
     BytesExpr,
     CallExpr,
@@ -105,7 +106,7 @@ def check_for_loop_like(
 
 
 def unmangle_name(name: str | None) -> str:
-    return (name or "").replace("'", "")
+    return (name or "").rstrip("'*")
 
 
 def is_equivalent(lhs: Node | None, rhs: Node | None) -> bool:
@@ -396,6 +397,10 @@ def _stringify(node: Node) -> str:
 
             return f"{{{inner}}}"
 
+        # TODO: support multiple lvalues
+        case AssignmentStmt(lvalues=[lhs], rvalue=rhs):
+            return f"{stringify(lhs)} = {stringify(rhs)}"
+
     raise ValueError
 
 

refurb/checks/readability/use_reverse.py

@@ -0,0 +1,105 @@
+from dataclasses import dataclass
+
+from mypy.nodes import (
+    AssignmentStmt,
+    CallExpr,
+    IndexExpr,
+    IntExpr,
+    NameExpr,
+    SliceExpr,
+    UnaryExpr,
+    Var,
+)
+
+from refurb.checks.common import stringify, unmangle_name
+from refurb.error import Error
+
+
+@dataclass
+class ErrorInfo(Error):
+    """
+    Don't use `x[::-1]` or `reversed(x)` to reverse a list and reassign it to
+    itself, use the faster in-place `.reverse()` method instead.
+
+    Bad:
+
+    ```
+    names = ["Bob", "Alice", "Charlie"]
+
+    names = reverse(names)
+    # or
+    names = list(reverse(names))
+    # or
+    names = names[::-1]
+    ```
+
+    Good:
+
+    ```
+    names = ["Bob", "Alice", "Charlie"]
+
+    names.reverse()
+    ```
+    """
+
+    name = "use-reverse"
+    categories = ("performance", "readability")
+    code = 187
+
+
+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.reversed"),
+                args=[NameExpr(fullname=reverse_name, node=Var(type=ty))],
+            ),
+        ) if (
+            unmangle_name(assign_name) == unmangle_name(reverse_name)
+            and str(ty).startswith("builtins.list[")
+        ):
+            pass
+
+        case AssignmentStmt(
+            lvalues=[NameExpr(fullname=assign_name) as assign_ref],
+            rvalue=CallExpr(
+                callee=NameExpr(fullname="builtins.list"),
+                args=[
+                    CallExpr(
+                        callee=NameExpr(fullname="builtins.reversed"),
+                        args=[NameExpr(fullname=reverse_name, node=Var(type=ty))],
+                    ),
+                ],
+            ),
+        ) if (
+            unmangle_name(assign_name) == unmangle_name(reverse_name)
+            and str(ty).startswith("builtins.list[")
+        ):
+            pass
+
+        case AssignmentStmt(
+            lvalues=[NameExpr(fullname=assign_name) as assign_ref],
+            rvalue=IndexExpr(
+                base=NameExpr(fullname=reverse_name, node=Var(type=ty)),
+                index=SliceExpr(
+                    begin_index=None,
+                    end_index=None,
+                    stride=UnaryExpr(op="-", expr=IntExpr(value=1)),
+                ),
+            ),
+        ) if (
+            unmangle_name(assign_name) == unmangle_name(reverse_name)
+            and str(ty).startswith("builtins.list[")
+        ):
+            pass
+
+        case _:
+            return
+
+    old = stringify(node)
+    new = f"{stringify(assign_ref)}.reverse()"
+
+    msg = f"Replace `{old}` with `{new}`"
+
+    errors.append(ErrorInfo.from_node(node, msg))

test/data/err_187.py

@@ -0,0 +1,43 @@
+# these should match
+
+# using functions to ensure `l` doesn't change type
+def a():
+    l = []
+    l = reversed(l)
+
+def b():
+    l = []
+    l = list(reversed(l))
+
+def c():
+    l = []
+    l = l[::-1]
+
+
+# these should not
+
+def d():
+    l = []
+    _ = reversed(l)
+
+def e():
+    l = []
+    l = l[::-2]
+    l = l[1:]
+    l = l[1::-1]
+    l = l[:1:-1]
+
+def f():
+    d = {}
+
+    # dont warn since d is a dict and does not have a .reverse() method
+    d = reversed(d)
+
+def g():
+    l = "abc"[::-1]
+
+def h():
+    l = reversed([1, 2, 3])
+
+def i():
+    l = list(reversed([1, 2, 3]))

test/data/err_187.txt

@@ -0,0 +1,3 @@
+test/data/err_187.py:6:5 [FURB187]: Replace `l = reversed(l)` with `l.reverse()`
+test/data/err_187.py:10:5 [FURB187]: Replace `l = list(reversed(l))` with `l.reverse()`
+test/data/err_187.py:14:5 [FURB187]: Replace `l = l[::-1]` with `l.reverse()`