Skip to content

Commit

Permalink
Add better type deduction for no-len-cmp
Browse files Browse the repository at this point in the history
  • Loading branch information
@dosisod
dosisod committed Feb 12, 2024
1 parent 4098c21 commit 0e70a8e
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 78 deletions.
62 changes: 23 additions & 39 deletions refurb/checks/readability/no_len_cmp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,11 @@
NameExpr,
Node,
OpExpr,
StrExpr,
TupleExpr,
TypeInfo,
UnaryExpr,
Var,
WhileStmt,
)
from mypy.types import Type

from refurb.checks.common import is_same_type
from refurb.checks.common import get_mypy_type, is_same_type, stringify
from refurb.error import Error
from refurb.visitor import METHOD_NODE_MAPPINGS, TraverserVisitor

Expand Down Expand Up @@ -66,22 +61,8 @@ class ErrorInfo(Error):
categories = ("iterable", "truthy")


def is_builtin_container_type(ty: Type | TypeInfo) -> bool:
return is_same_type(ty, list, tuple, dict, set, frozenset, str)


def is_builtin_container_like(node: Expression) -> bool:
match node:
case NameExpr(node=Var(type=ty)) if ty and is_builtin_container_type(ty):
return True

case CallExpr(callee=NameExpr(node=TypeInfo() as ty)) if is_builtin_container_type(ty):
return True

case DictExpr() | ListExpr() | StrExpr() | TupleExpr():
return True

return False
return is_same_type(get_mypy_type(node), list, tuple, dict, set, frozenset, str)


def is_len_call(node: CallExpr) -> bool:
Expand Down Expand Up @@ -129,39 +110,42 @@ def visit_comparison_expr(self, node: ComparisonExpr) -> None:
match node:
case ComparisonExpr(
operators=[oper],
operands=[CallExpr() as call, IntExpr(value=num)],
operands=[CallExpr(args=[arg]) as call, IntExpr(value=num)],
) if is_len_call(call):
is_truthy = IS_INT_COMPARISON_TRUTHY.get((oper, num))

if is_truthy is None:
return

expr = "x" if is_truthy else "not x"
old = stringify(node)
new = stringify(arg)

if not is_truthy:
new = f"not {new}"

msg = f"Replace `{old}` with `{new}`"

self.errors.append(
ErrorInfo.from_node(node, f"Replace `len(x) {oper} {num}` with `{expr}`")
)
self.errors.append(ErrorInfo.from_node(node, msg))

case ComparisonExpr(
operators=["==" | "!=" as oper],
operands=[
NameExpr() as name,
(ListExpr() | DictExpr()) as expr,
],
) if is_builtin_container_like(name):
if expr.items: # type: ignore
return
operands=[lhs, (ListExpr(items=[]) | DictExpr(items=[]))],
) if is_builtin_container_like(lhs):
old = stringify(node)
new = stringify(lhs)

if oper == "==":
new = f"not {new}"

old_expr = "[]" if isinstance(expr, ListExpr) else "{}"
expr = "not x" if oper == "==" else "x"
msg = f"Replace `{old}` with `{new}`"

self.errors.append(
ErrorInfo.from_node(node, f"Replace `x {oper} {old_expr}` with `{expr}`")
)
self.errors.append(ErrorInfo.from_node(node, msg))

def visit_call_expr(self, node: CallExpr) -> None:
if is_len_call(node):
self.errors.append(ErrorInfo.from_node(node, "Replace `len(x)` with `x`"))
msg = f"Replace `{stringify(node)}` with `{stringify(node.args[0])}`"

self.errors.append(ErrorInfo.from_node(node, msg))


ConditionLikeNode = (
Expand Down
9 changes: 9 additions & 0 deletions test/data/err_115.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@
assert len(nums) or False


class C:
l: list[int]

assert C().l == []

c = C()
assert c.l == []


# these should not

if len(nums) == 1: ...
Expand Down
78 changes: 40 additions & 38 deletions test/data/err_115.txt
View file
Original file line number Diff line number Diff line change
@@ -1,38 +1,40 @@
test/data/err_115.py:11:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:12:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:13:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:14:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:15:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:16:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:18:4 [FURB115]: Replace `len(x) <= 0` with `not x`
test/data/err_115.py:19:4 [FURB115]: Replace `len(x) > 0` with `x`
test/data/err_115.py:20:4 [FURB115]: Replace `len(x) != 0` with `x`
test/data/err_115.py:21:4 [FURB115]: Replace `len(x) >= 1` with `x`
test/data/err_115.py:23:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:24:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:25:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:26:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:27:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:28:4 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:30:13 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:33:15 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:36:23 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:37:23 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:38:28 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:40:10 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:42:7 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:45:8 [FURB115]: Replace `len(x) == 0` with `not x`
test/data/err_115.py:50:4 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:53:15 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:56:23 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:57:23 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:58:28 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:60:10 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:62:7 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:65:8 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:67:8 [FURB115]: Replace `x == []` with `not x`
test/data/err_115.py:68:8 [FURB115]: Replace `x != []` with `x`
test/data/err_115.py:70:8 [FURB115]: Replace `x == {}` with `not x`
test/data/err_115.py:71:8 [FURB115]: Replace `x != {}` with `x`
test/data/err_115.py:73:8 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:74:8 [FURB115]: Replace `len(x)` with `x`
test/data/err_115.py:11:4 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:12:4 [FURB115]: Replace `len(authors) == 0` with `not authors`
test/data/err_115.py:13:4 [FURB115]: Replace `len(primes) == 0` with `not primes`
test/data/err_115.py:14:4 [FURB115]: Replace `len(data) == 0` with `not data`
test/data/err_115.py:15:4 [FURB115]: Replace `len(name) == 0` with `not name`
test/data/err_115.py:16:4 [FURB115]: Replace `len(fruits) == 0` with `not fruits`
test/data/err_115.py:18:4 [FURB115]: Replace `len(nums) <= 0` with `not nums`
test/data/err_115.py:19:4 [FURB115]: Replace `len(nums) > 0` with `nums`
test/data/err_115.py:20:4 [FURB115]: Replace `len(nums) != 0` with `nums`
test/data/err_115.py:21:4 [FURB115]: Replace `len(nums) >= 1` with `nums`
test/data/err_115.py:23:4 [FURB115]: Replace `len([]) == 0` with `not []`
test/data/err_115.py:24:4 [FURB115]: Replace `len({}) == 0` with `not {}`
test/data/err_115.py:25:4 [FURB115]: Replace `len(()) == 0` with `not ()`
test/data/err_115.py:26:4 [FURB115]: Replace `len("") == 0` with `not ""`
test/data/err_115.py:27:4 [FURB115]: Replace `len(set(())) == 0` with `not set(())`
test/data/err_115.py:28:4 [FURB115]: Replace `len(frozenset(())) == 0` with `not frozenset(())`
test/data/err_115.py:30:13 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:33:15 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:36:23 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:37:23 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:38:28 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:40:10 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:42:7 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:45:8 [FURB115]: Replace `len(nums) == 0` with `not nums`
test/data/err_115.py:50:4 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:53:15 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:56:23 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:57:23 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:58:28 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:60:10 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:62:7 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:65:8 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:67:8 [FURB115]: Replace `nums == []` with `not nums`
test/data/err_115.py:68:8 [FURB115]: Replace `nums != []` with `nums`
test/data/err_115.py:70:8 [FURB115]: Replace `authors == {}` with `not authors`
test/data/err_115.py:71:8 [FURB115]: Replace `authors != {}` with `authors`
test/data/err_115.py:73:8 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:74:8 [FURB115]: Replace `len(nums)` with `nums`
test/data/err_115.py:80:8 [FURB115]: Replace `C().l == []` with `not C().l`
test/data/err_115.py:83:8 [FURB115]: Replace `c.l == []` with `not c.l`
2 changes: 1 addition & 1 deletion test/test_gen.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

def test_folder_not_in_cwd_is_ignored():
with patch("pathlib.Path.cwd", lambda: Path("/some/random/path")):
assert folders_needing_init_file(Path("./some/path")) == []
assert not folders_needing_init_file(Path("./some/path"))


def test_relative_path_works():
Expand Down

0 comments on commit 0e70a8e

Please sign in to comment.