From 661f7568472d975d14d0fd510fd366df45a8ee8b Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Fri, 5 Jan 2024 21:27:47 -0800 Subject: [PATCH] Add more cases to FURB173 --- .../checks/readability/no_unnecessary_cast.py | 4 +- refurb/checks/readability/use_dict_union.py | 62 ++++++++++++++++++- test/data/err_123.py | 1 + test/data/err_173.py | 14 +++++ test/data/err_173.txt | 6 ++ 5 files changed, 84 insertions(+), 3 deletions(-) diff --git a/refurb/checks/readability/no_unnecessary_cast.py b/refurb/checks/readability/no_unnecessary_cast.py index bf85168..f8dd34f 100644 --- a/refurb/checks/readability/no_unnecessary_cast.py +++ b/refurb/checks/readability/no_unnecessary_cast.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from mypy.nodes import ( + ArgKind, BytesExpr, CallExpr, ComplexExpr, @@ -83,7 +84,8 @@ def check(node: CallExpr, errors: list[Error]) -> None: case CallExpr( callee=NameExpr(fullname=fullname, name=name), args=[arg], - ) if fullname in FUNC_NAMES: + arg_kinds=[arg_kind], + ) if arg_kind != ArgKind.ARG_STAR2 and fullname in FUNC_NAMES: node_type, msg = FUNC_NAMES[fullname] if type(arg) == node_type: diff --git a/refurb/checks/readability/use_dict_union.py b/refurb/checks/readability/use_dict_union.py index deb62a1..07bbe4f 100644 --- a/refurb/checks/readability/use_dict_union.py +++ b/refurb/checks/readability/use_dict_union.py @@ -1,8 +1,9 @@ from dataclasses import dataclass from itertools import groupby -from mypy.nodes import DictExpr, Expression, RefExpr, Var +from mypy.nodes import ArgKind, CallExpr, DictExpr, Expression, RefExpr, Var +from refurb.checks.common import stringify from refurb.error import Error from refurb.settings import Settings @@ -55,7 +56,7 @@ def is_builtin_mapping(expr: Expression) -> bool: return False -def check(node: DictExpr, errors: list[Error], settings: Settings) -> None: +def check(node: DictExpr | CallExpr, errors: list[Error], settings: Settings) -> None: if settings.get_python_version() < (3, 9): return # pragma: no cover @@ -107,3 +108,60 @@ def check(node: DictExpr, errors: list[Error], settings: Settings) -> None: msg = f"Replace `{{{old_msg}}}` with `{new_msg}`" errors.append(ErrorInfo.from_node(node, msg)) + + case CallExpr(callee=RefExpr(fullname="builtins.dict")): + old = [] + args: list[str] = [] + kwargs: dict[str, str] = {} + + # ignore dict(x) since that is covered by FURB123 + match node.arg_kinds: + case []: + return + + case [ArgKind.ARG_POS]: + return + + # TODO: move dict(a=1, b=2) to FURB112 + if all(x == ArgKind.ARG_NAMED for x in node.arg_kinds): + return + + for arg, name, kind in zip(node.args, node.arg_names, node.arg_kinds): + # ignore dict(*x) + if kind == ArgKind.ARG_STAR: + return + + if kind == ArgKind.ARG_STAR2: + old.append(f"**{stringify(arg)}") + + stringified_arg = stringify(arg) + + if len(node.args) == 1: + # TODO: dict(**x) can be replaced with x.copy() if we know x has a copy() + # method. + stringified_arg = f"{{**{stringified_arg}}}" + + args.append(stringified_arg) + + elif name: + old.append(f"{name}={stringify(arg)}") + kwargs[name] = stringify(arg) + + else: + old.append(stringify(arg)) + args.append(stringify(arg)) + + inner = ", ".join(old) + old_msg = f"dict({inner})" + + if kwargs: + kwargs2 = ", ".join(f'"{name}": {expr}' for name, expr in kwargs.items()) + kwargs2 = f"{{{kwargs2}}}" + + args.append(kwargs2) + + new_msg = " | ".join(args) + + msg = f"Replace `{old_msg}` with `{new_msg}`" + + errors.append(ErrorInfo.from_node(node, msg)) diff --git a/test/data/err_123.py b/test/data/err_123.py index 3887536..2509b5d 100644 --- a/test/data/err_123.py +++ b/test/data/err_123.py @@ -46,3 +46,4 @@ _ = str(123) _ = tuple([1, 2, 3]) _ = int("0xFF") +_ = dict(**d) # noqa: FURB173 diff --git a/test/data/err_173.py b/test/data/err_173.py index 30e6392..9339e24 100644 --- a/test/data/err_173.py +++ b/test/data/err_173.py @@ -34,6 +34,14 @@ _ = {"k": "v", **userdict} +_ = dict(**x) +_ = dict(x, **y) +_ = dict(**x, **y) +_ = dict(x, a=1) +_ = dict(**x, a=1, b=2) +_ = dict(**x, **y, a=1, b=2) + + # these should not _ = {} @@ -60,3 +68,9 @@ def __getitem__(self, key: str) -> Any: # TODO: support more expr types _ = {"k": "v", **{}} + + +_ = dict(x) # noqa: FURB123 +_ = dict(*({},)) +_ = dict() # noqa: FURB112 +_ = dict(a=1, b=2) diff --git a/test/data/err_173.txt b/test/data/err_173.txt index cc8458b..58c5ec3 100644 --- a/test/data/err_173.txt +++ b/test/data/err_173.txt @@ -14,3 +14,9 @@ test/data/err_173.py:25:5 [FURB173]: Replace `{..., **x1}` with `{...} | x1` test/data/err_173.py:28:5 [FURB173]: Replace `{..., **x1}` with `{...} | x1` test/data/err_173.py:31:5 [FURB173]: Replace `{..., **x1}` with `{...} | x1` test/data/err_173.py:34:5 [FURB173]: Replace `{..., **x1}` with `{...} | x1` +test/data/err_173.py:37:5 [FURB173]: Replace `dict(**x)` with `{**x}` +test/data/err_173.py:38:5 [FURB173]: Replace `dict(x, **y)` with `x | y` +test/data/err_173.py:39:5 [FURB173]: Replace `dict(**x, **y)` with `x | y` +test/data/err_173.py:40:5 [FURB173]: Replace `dict(x, a=1)` with `x | {"a": 1}` +test/data/err_173.py:41:5 [FURB173]: Replace `dict(**x, a=1, b=2)` with `x | {"a": 1, "b": 2}` +test/data/err_173.py:42:5 [FURB173]: Replace `dict(**x, **y, a=1, b=2)` with `x | y | {"a": 1, "b": 2}`