Fix FURB118 operator.contains false positive, improve error messages (

#317): * Ensure operands are reversed for `operator.contains` * Include lambda code in error message for FURB118
dosisod · Dec 19, 2023 · b15ad49 · b15ad49
1 parent adfd2f1
commit b15ad49
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 34 deletions.
diff --git a/refurb/checks/common.py b/refurb/checks/common.py
@@ -2,6 +2,7 @@
 from itertools import chain, combinations, starmap
 
 from mypy.nodes import (
+    ArgKind,
     Block,
     BytesExpr,
     CallExpr,
@@ -13,12 +14,14 @@
     GeneratorExpr,
     IndexExpr,
     IntExpr,
+    LambdaExpr,
     ListExpr,
     MemberExpr,
     MypyFile,
     NameExpr,
     Node,
     OpExpr,
+    ReturnStmt,
     SetExpr,
     SliceExpr,
     StarExpr,
@@ -271,9 +274,17 @@ def is_type_none_call(node: Expression) -> bool:
 
 
 def stringify(node: Node) -> str:
+    try:
+        return _stringify(node)
+
+    except ValueError:  # pragma: no cover
+        return "x"
+
+
+def _stringify(node: Node) -> str:
     match node:
         case MemberExpr(expr=expr, name=name):
-            return f"{stringify(expr)}.{name}"
+            return f"{_stringify(expr)}.{name}"
 
         case NameExpr(name=name):
             return name
@@ -287,10 +298,41 @@ def stringify(node: Node) -> str:
             return str(value)
 
         case CallExpr():
-            name = stringify(node.callee)
+            name = _stringify(node.callee)
 
-            args = ", ".join(stringify(arg) for arg in node.args)
+            args = ", ".join(_stringify(arg) for arg in node.args)
 
             return f"{name}({args})"
 
-    return "x"  # pragma: no cover
+        case OpExpr(left=left, op=op, right=right):
+            return f"{_stringify(left)} {op} {_stringify(right)}"
+
+        case ComparisonExpr():
+            parts: list[str] = []
+
+            for op, operand in zip(node.operators, node.operands):
+                parts.extend((_stringify(operand), op))
+
+            parts.append(_stringify(node.operands[-1]))
+
+            return " ".join(parts)
+
+        case UnaryExpr(op=op, expr=expr):
+            return f"{op} {_stringify(expr)}"
+
+        case LambdaExpr(
+            arg_names=arg_names,
+            arg_kinds=arg_kinds,
+            body=Block(body=[ReturnStmt(expr=Expression() as expr)]),
+        ) if (all(kind == ArgKind.ARG_POS for kind in arg_kinds) and all(arg_names)):
+            if arg_names:
+                args = " "
+                args += ", ".join(arg_names)  # type: ignore
+            else:
+                args = ""
+
+            body = _stringify(expr)
+
+            return f"lambda{args}: {body}"
+
+    raise ValueError
diff --git a/refurb/checks/readability/use_operators.py b/refurb/checks/readability/use_operators.py
@@ -12,6 +12,7 @@
     UnaryExpr,
 )
 
+from refurb.checks.common import _stringify
 from refurb.error import Error
 
 
@@ -82,7 +83,7 @@ class ErrorInfo(Error):
 
 
 def check(node: FuncItem, errors: list[Error]) -> None:
-    func_type = "lambda" if isinstance(node, LambdaExpr) else "function"
+    func_type = get_function_type(node)
 
     match node:
         case FuncItem(
@@ -106,8 +107,12 @@ def check(node: FuncItem, errors: list[Error]) -> None:
                     )
                 ]
             ),
-        ) if lhs_name == expr_lhs and rhs_name == expr_rhs:
-            if func_name := BINARY_OPERATORS.get(op):
+        ) if func_name := BINARY_OPERATORS.get(op):
+            if func_name == "contains":
+                # operator.contains has reversed parameters
+                expr_lhs, expr_rhs = expr_rhs, expr_lhs
+
+            if lhs_name == expr_lhs and rhs_name == expr_rhs:
                 errors.append(
                     ErrorInfo.from_node(
                         node,
@@ -136,3 +141,14 @@ def check(node: FuncItem, errors: list[Error]) -> None:
                         f"Replace {func_type} with `operator.{func_name}`",
                     )
                 )
+
+
+def get_function_type(node: FuncItem) -> str:
+    if isinstance(node, LambdaExpr):
+        try:
+            return f"`{_stringify(node)}`"
+
+        except ValueError:
+            return "lambda"
+
+    return "function"
diff --git a/test/data/err_118.py b/test/data/err_118.py
@@ -9,7 +9,7 @@
 lambda x, y: x ** y
 lambda x, y: x is y
 lambda x, y: x is not y
-lambda x, y: x in y
+lambda x, y: y in x
 lambda x, y: x & y
 lambda x, y: x | y
 lambda x, y: x ^ y

diff --git a/test/data/err_118.txt b/test/data/err_118.txt
@@ -1,28 +1,28 @@
-test/data/err_118.py:3:1 [FURB118]: Replace lambda with `operator.add`
-test/data/err_118.py:4:1 [FURB118]: Replace lambda with `operator.sub`
-test/data/err_118.py:5:1 [FURB118]: Replace lambda with `operator.truediv`
-test/data/err_118.py:6:1 [FURB118]: Replace lambda with `operator.floordiv`
-test/data/err_118.py:7:1 [FURB118]: Replace lambda with `operator.mul`
-test/data/err_118.py:8:1 [FURB118]: Replace lambda with `operator.matmul`
-test/data/err_118.py:9:1 [FURB118]: Replace lambda with `operator.pow`
-test/data/err_118.py:10:1 [FURB118]: Replace lambda with `operator.is_`
-test/data/err_118.py:11:1 [FURB118]: Replace lambda with `operator.is_not`
-test/data/err_118.py:12:1 [FURB118]: Replace lambda with `operator.contains`
-test/data/err_118.py:13:1 [FURB118]: Replace lambda with `operator.and_`
-test/data/err_118.py:14:1 [FURB118]: Replace lambda with `operator.or_`
-test/data/err_118.py:15:1 [FURB118]: Replace lambda with `operator.xor`
-test/data/err_118.py:16:1 [FURB118]: Replace lambda with `operator.lshift`
-test/data/err_118.py:17:1 [FURB118]: Replace lambda with `operator.rshift`
-test/data/err_118.py:18:1 [FURB118]: Replace lambda with `operator.mod`
-test/data/err_118.py:19:1 [FURB118]: Replace lambda with `operator.lt`
-test/data/err_118.py:20:1 [FURB118]: Replace lambda with `operator.le`
-test/data/err_118.py:21:1 [FURB118]: Replace lambda with `operator.eq`
-test/data/err_118.py:22:1 [FURB118]: Replace lambda with `operator.ne`
-test/data/err_118.py:23:1 [FURB118]: Replace lambda with `operator.ge`
-test/data/err_118.py:24:1 [FURB118]: Replace lambda with `operator.gt`
-test/data/err_118.py:26:1 [FURB118]: Replace lambda with `operator.invert`
-test/data/err_118.py:27:1 [FURB118]: Replace lambda with `operator.neg`
-test/data/err_118.py:28:1 [FURB118]: Replace lambda with `operator.not_`
-test/data/err_118.py:29:1 [FURB118]: Replace lambda with `operator.pos`
+test/data/err_118.py:3:1 [FURB118]: Replace `lambda x, y: x + y` with `operator.add`
+test/data/err_118.py:4:1 [FURB118]: Replace `lambda x, y: x - y` with `operator.sub`
+test/data/err_118.py:5:1 [FURB118]: Replace `lambda x, y: x / y` with `operator.truediv`
+test/data/err_118.py:6:1 [FURB118]: Replace `lambda x, y: x // y` with `operator.floordiv`
+test/data/err_118.py:7:1 [FURB118]: Replace `lambda x, y: x * y` with `operator.mul`
+test/data/err_118.py:8:1 [FURB118]: Replace `lambda x, y: x @ y` with `operator.matmul`
+test/data/err_118.py:9:1 [FURB118]: Replace `lambda x, y: x ** y` with `operator.pow`
+test/data/err_118.py:10:1 [FURB118]: Replace `lambda x, y: x is y` with `operator.is_`
+test/data/err_118.py:11:1 [FURB118]: Replace `lambda x, y: x is not y` with `operator.is_not`
+test/data/err_118.py:12:1 [FURB118]: Replace `lambda x, y: y in x` with `operator.contains`
+test/data/err_118.py:13:1 [FURB118]: Replace `lambda x, y: x & y` with `operator.and_`
+test/data/err_118.py:14:1 [FURB118]: Replace `lambda x, y: x | y` with `operator.or_`
+test/data/err_118.py:15:1 [FURB118]: Replace `lambda x, y: x ^ y` with `operator.xor`
+test/data/err_118.py:16:1 [FURB118]: Replace `lambda x, y: x << y` with `operator.lshift`
+test/data/err_118.py:17:1 [FURB118]: Replace `lambda x, y: x >> y` with `operator.rshift`
+test/data/err_118.py:18:1 [FURB118]: Replace `lambda x, y: x % y` with `operator.mod`
+test/data/err_118.py:19:1 [FURB118]: Replace `lambda x, y: x < y` with `operator.lt`
+test/data/err_118.py:20:1 [FURB118]: Replace `lambda x, y: x <= y` with `operator.le`
+test/data/err_118.py:21:1 [FURB118]: Replace `lambda x, y: x == y` with `operator.eq`
+test/data/err_118.py:22:1 [FURB118]: Replace `lambda x, y: x != y` with `operator.ne`
+test/data/err_118.py:23:1 [FURB118]: Replace `lambda x, y: x >= y` with `operator.ge`
+test/data/err_118.py:24:1 [FURB118]: Replace `lambda x, y: x > y` with `operator.gt`
+test/data/err_118.py:26:1 [FURB118]: Replace `lambda x: ~ x` with `operator.invert`
+test/data/err_118.py:27:1 [FURB118]: Replace `lambda x: - x` with `operator.neg`
+test/data/err_118.py:28:1 [FURB118]: Replace `lambda x: not x` with `operator.not_`
+test/data/err_118.py:29:1 [FURB118]: Replace `lambda x: + x` with `operator.pos`
 test/data/err_118.py:31:1 [FURB118]: Replace function with `operator.add`
 test/data/err_118.py:34:1 [FURB118]: Replace function with `operator.neg`