diff --git a/refurb/checks/common.py b/refurb/checks/common.py index b5c5666..c39b908 100644 --- a/refurb/checks/common.py +++ b/refurb/checks/common.py @@ -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", + ) diff --git a/refurb/checks/readability/no_len_cmp.py b/refurb/checks/readability/no_len_cmp.py index 43cccce..39a587b 100644 --- a/refurb/checks/readability/no_len_cmp.py +++ b/refurb/checks/readability/no_len_cmp.py @@ -7,11 +7,13 @@ ConditionalExpr, DictExpr, DictionaryComprehension, + Expression, GeneratorExpr, IfStmt, IntExpr, ListExpr, MatchStmt, + MemberExpr, NameExpr, Node, OpExpr, @@ -19,7 +21,7 @@ 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 @@ -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] @@ -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) diff --git a/test/data/err_115.py b/test/data/err_115.py index 4b7dd09..cbfcf5f 100644 --- a/test/data/err_115.py +++ b/test/data/err_115.py @@ -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: ... diff --git a/test/data/err_115.txt b/test/data/err_115.txt index c803e5c..f7f5af7 100644 --- a/test/data/err_115.txt +++ b/test/data/err_115.txt @@ -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`