From 835e7a89fbf64e488d15234bda9fa42da8afd5b1 Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Mon, 12 Feb 2024 18:14:19 -0800 Subject: [PATCH] Add better type deduction to `use-dict-union` and `no-copy-with-merge` --- pyproject.toml | 3 +++ refurb/checks/readability/no_copy_with_merge.py | 13 +++++-------- refurb/checks/readability/use_dict_union.py | 13 ++++++------- test/data/err_173.py | 12 ++++++++---- test/data/err_173.txt | 2 ++ test/data/err_185.py | 16 +++++++++++----- test/data/err_185.txt | 1 + 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 63d4282..80a773b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -126,6 +126,9 @@ target-version = "py310" "refurb/visitor/traverser.py" = ["ALL"] "test/e2e/gbk.py" = ["FURB105"] +[tool.typos.default.extend-words] +nd = "nd" + [build-system] requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" diff --git a/refurb/checks/readability/no_copy_with_merge.py b/refurb/checks/readability/no_copy_with_merge.py index 9422455..c8a4f58 100644 --- a/refurb/checks/readability/no_copy_with_merge.py +++ b/refurb/checks/readability/no_copy_with_merge.py @@ -1,8 +1,8 @@ from dataclasses import dataclass -from mypy.nodes import CallExpr, Expression, MemberExpr, OpExpr, RefExpr, Var +from mypy.nodes import CallExpr, Expression, MemberExpr, OpExpr -from refurb.checks.common import is_same_type, stringify +from refurb.checks.common import get_mypy_type, is_same_type, stringify from refurb.error import Error @@ -43,13 +43,10 @@ def check_expr(expr: Expression, errors: list[Error]) -> None: match expr: case CallExpr( - callee=MemberExpr( - expr=RefExpr(node=Var(type=ty)) as ref, - name="copy", - ), + callee=MemberExpr(expr=lhs, name="copy"), args=[], - ) if is_same_type(ty, dict, set): - msg = f"Replace `{stringify(ref)}.copy()` with `{stringify(ref)}`" + ) if is_same_type(get_mypy_type(lhs), dict, set): + msg = f"Replace `{stringify(lhs)}.copy()` with `{stringify(lhs)}`" errors.append(ErrorInfo.from_node(expr, msg)) diff --git a/refurb/checks/readability/use_dict_union.py b/refurb/checks/readability/use_dict_union.py index ff2ceb2..92e2d7a 100644 --- a/refurb/checks/readability/use_dict_union.py +++ b/refurb/checks/readability/use_dict_union.py @@ -1,9 +1,9 @@ from dataclasses import dataclass from itertools import groupby -from mypy.nodes import ArgKind, CallExpr, DictExpr, Expression, RefExpr, Var +from mypy.nodes import ArgKind, CallExpr, DictExpr, Expression, RefExpr -from refurb.checks.common import is_same_type, stringify +from refurb.checks.common import get_mypy_type, is_same_type, stringify from refurb.error import Error from refurb.settings import Settings @@ -49,11 +49,7 @@ def add_defaults(settings: dict[str, str]) -> dict[str, str]: def is_builtin_mapping(expr: Expression) -> bool: - match expr: - case RefExpr(node=Var(type=ty)): - return is_same_type(ty, *MAPPING_TYPES) - - return False + return is_same_type(get_mypy_type(expr), *MAPPING_TYPES) def check(node: DictExpr | CallExpr, errors: list[Error], settings: Settings) -> None: @@ -125,6 +121,9 @@ def check(node: DictExpr | CallExpr, errors: list[Error], settings: Settings) -> return if kind == ArgKind.ARG_STAR2: + if not is_builtin_mapping(arg): + return + stringified_arg = stringify(arg) if len(node.args) == 1: diff --git a/test/data/err_173.py b/test/data/err_173.py index 9339e24..49ad3ea 100644 --- a/test/data/err_173.py +++ b/test/data/err_173.py @@ -40,6 +40,12 @@ _ = dict(x, a=1) _ = dict(**x, a=1, b=2) _ = dict(**x, **y, a=1, b=2) +_ = dict(**x, **{}) + +class Wrapper: + d: dict + +_ = {**Wrapper().d, **x} # these should not @@ -66,11 +72,9 @@ def __getitem__(self, key: str) -> Any: _ = {"k": "v", **c} -# TODO: support more expr types -_ = {"k": "v", **{}} - - _ = dict(x) # noqa: FURB123 _ = dict(*({},)) _ = dict() # noqa: FURB112 _ = dict(a=1, b=2) + +_ = dict(**x, **[]) # type: ignore diff --git a/test/data/err_173.txt b/test/data/err_173.txt index 54617d6..45fde7c 100644 --- a/test/data/err_173.txt +++ b/test/data/err_173.txt @@ -20,3 +20,5 @@ 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}` +test/data/err_173.py:43:5 [FURB173]: Replace `dict(**x, **{})` with `x | {}` +test/data/err_173.py:48:5 [FURB173]: Replace `{**Wrapper().d, **x}` with `Wrapper().d | x` diff --git a/test/data/err_185.py b/test/data/err_185.py index 4472504..5e3cf06 100644 --- a/test/data/err_185.py +++ b/test/data/err_185.py @@ -11,13 +11,19 @@ _ = x.copy() | {} | x.copy() - class C: - def copy(self) -> dict: - return {} + d: dict[str, str] -c = C() + +_ = C().d.copy() | {} # these should not -_ = c.copy() | {} + +class NotADict: + def copy(self) -> dict: + return {} + +nd = NotADict() + +_ = nd.copy() | {} diff --git a/test/data/err_185.txt b/test/data/err_185.txt index 085487e..5337438 100644 --- a/test/data/err_185.txt +++ b/test/data/err_185.txt @@ -4,3 +4,4 @@ test/data/err_185.py:8:5 [FURB185]: Replace `y.copy()` with `y` test/data/err_185.py:9:13 [FURB185]: Replace `y.copy()` with `y` test/data/err_185.py:11:5 [FURB185]: Replace `x.copy()` with `x` test/data/err_185.py:11:21 [FURB185]: Replace `x.copy()` with `x` +test/data/err_185.py:18:5 [FURB185]: Replace `C().d.copy()` with `C().d`