From 43c63c8d6a514aad2e0cecf83130d9008303b1c3 Mon Sep 17 00:00:00 2001
From: Logan Hunt <39638017+dosisod@users.noreply.github.com>
Date: Sat, 2 Mar 2024 15:12:13 -0800
Subject: [PATCH] Add better detection and error messages for len compatible
 types in FURB115 (#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.
---
 refurb/checks/common.py                 | 11 ++++++++++-
 refurb/checks/readability/no_len_cmp.py | 20 +++++++++++++++++++-
 test/data/err_115.py                    |  8 ++++++++
 test/data/err_115.txt                   |  6 ++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

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`