Skip to content

Commit

Permalink
Add better type deduction to use-dict-union and no-copy-with-merge
Browse files Browse the repository at this point in the history
  • Loading branch information
dosisod committed Feb 13, 2024
1 parent 0e70a8e commit 835e7a8
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 24 deletions.
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
13 changes: 5 additions & 8 deletions refurb/checks/readability/no_copy_with_merge.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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))

Expand Down
13 changes: 6 additions & 7 deletions refurb/checks/readability/use_dict_union.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 8 additions & 4 deletions test/data/err_173.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
2 changes: 2 additions & 0 deletions test/data/err_173.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`
16 changes: 11 additions & 5 deletions test/data/err_185.py
Original file line number Diff line number Diff line change
Expand Up @@ -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() | {}
1 change: 1 addition & 0 deletions test/data/err_185.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`

0 comments on commit 835e7a8

Please sign in to comment.