Skip to content

Commit

Permalink
Add better detection and error messages for len compatible types in F…
Browse files Browse the repository at this point in the history
…URB115 (#331):

Previously `dict_keys()` and `dict_values()` objects where not type deducable
by Refurb, and now they are. In addition, chained calls like `len(list(d))`
are able to be simplified down to just `d`.

Closes #330.
  • Loading branch information
dosisod authored Mar 2, 2024
1 parent d8ec9f4 commit 43c63c8
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
11 changes: 10 additions & 1 deletion refurb/checks/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -752,4 +752,13 @@ def is_sized(node: Expression) -> bool:

# TODO: support any Sized subclass
def is_sized_type(ty: Type | SymbolNode | None) -> bool:
return is_mapping_type(ty) or is_same_type(ty, list, tuple, set, frozenset, str)
return is_mapping_type(ty) or is_same_type(
ty,
frozenset,
list,
set,
str,
tuple,
"_collections_abc.dict_keys",
"_collections_abc.dict_values",
)
20 changes: 19 additions & 1 deletion refurb/checks/readability/no_len_cmp.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@
ConditionalExpr,
DictExpr,
DictionaryComprehension,
Expression,
GeneratorExpr,
IfStmt,
IntExpr,
ListExpr,
MatchStmt,
MemberExpr,
NameExpr,
Node,
OpExpr,
UnaryExpr,
WhileStmt,
)

from refurb.checks.common import is_sized, stringify
from refurb.checks.common import is_mapping, is_sized, stringify
from refurb.error import Error
from refurb.visitor import METHOD_NODE_MAPPINGS, TraverserVisitor

Expand Down Expand Up @@ -77,6 +79,20 @@ def is_len_call(node: CallExpr) -> bool:
}


def simplify_len_call(expr: Expression) -> Expression:
match expr:
case CallExpr(callee=NameExpr(fullname="builtins.list"), args=[arg]):
return simplify_len_call(arg)

case CallExpr(
callee=MemberExpr(expr=arg, name="keys" | "values"),
args=[],
) if is_mapping(arg):
return simplify_len_call(arg)

return expr


class LenComparisonVisitor(TraverserVisitor):
errors: list[Error]

Expand Down Expand Up @@ -109,6 +125,8 @@ def visit_comparison_expr(self, node: ComparisonExpr) -> None:
if is_truthy is None:
return

arg = simplify_len_call(arg)

old = stringify(node)
new = stringify(arg)

Expand Down
8 changes: 8 additions & 0 deletions test/data/err_115.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ def mapping_check(m: Mapping[str, str]):
pass


assert len(list(authors.keys())) == 0
assert len(list(authors.values())) == 0
assert len(list(authors)) == 0
assert len(authors.keys()) == 0
assert len(authors.values()) == 0
assert len(authors) == 0


# these should not

if len(nums) == 1: ...
Expand Down
6 changes: 6 additions & 0 deletions test/data/err_115.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,9 @@ 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`
test/data/err_115.py:89:8 [FURB115]: Replace `len(m) == 0` with `not m`
test/data/err_115.py:93:8 [FURB115]: Replace `len(list(authors.keys())) == 0` with `not authors`
test/data/err_115.py:94:8 [FURB115]: Replace `len(list(authors.values())) == 0` with `not authors`
test/data/err_115.py:95:8 [FURB115]: Replace `len(list(authors)) == 0` with `not authors`
test/data/err_115.py:96:8 [FURB115]: Replace `len(authors.keys()) == 0` with `not authors`
test/data/err_115.py:97:8 [FURB115]: Replace `len(authors.values()) == 0` with `not authors`
test/data/err_115.py:98:8 [FURB115]: Replace `len(authors) == 0` with `not authors`

0 comments on commit 43c63c8

Please sign in to comment.