Don't emit FURB135/FURB148 when using _ in comprehensions (#312):

Closes #311.

See #311 (comment)
for a detailed explaination of the problem and how I went about fixing it.

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "refurb"
-version = "1.24.0"
+version = "1.25.0"
 description = "A tool for refurbish and modernize Python codebases"
 authors = ["dosisod"]
 license = "GPL-3.0-only"

refurb/checks/builtin/no_ignored_dict_items.py

@@ -12,7 +12,7 @@
     Var,
 )
 
-from refurb.checks.common import check_for_loop_like, is_name_unused_in_contexts, is_placeholder
+from refurb.checks.common import check_for_loop_like, is_name_unused_in_contexts
 from refurb.error import Error
 
 
@@ -78,10 +78,10 @@ def check_dict_items_call(
 def check_unused_key_or_value(
     key: NameExpr, value: NameExpr, contexts: list[Node], errors: list[Error]
 ) -> None:
-    if is_placeholder(key) or is_name_unused_in_contexts(key, contexts):
+    if is_name_unused_in_contexts(key, contexts):
         errors.append(
             ErrorInfo.from_node(key, "Key is unused, use `for value in d.values()` instead")
         )
 
-    if is_placeholder(value) or is_name_unused_in_contexts(value, contexts):
+    if is_name_unused_in_contexts(value, contexts):
         errors.append(ErrorInfo.from_node(value, "Value is unused, use `for key in d` instead"))

refurb/checks/builtin/no_ignored_enumerate.py

@@ -11,7 +11,7 @@
     Var,
 )
 
-from refurb.checks.common import check_for_loop_like, is_name_unused_in_contexts, is_placeholder
+from refurb.checks.common import check_for_loop_like, is_name_unused_in_contexts
 from refurb.error import Error
 
 
@@ -75,10 +75,10 @@ def check_enumerate_call(
 def check_unused_index_or_value(
     index: NameExpr, value: NameExpr, contexts: list[Node], errors: list[Error]
 ) -> None:
-    if is_placeholder(index) or is_name_unused_in_contexts(index, contexts):
+    if is_name_unused_in_contexts(index, contexts):
         errors.append(ErrorInfo.from_node(index, "Index is unused, use `for x in y` instead"))
 
-    if is_placeholder(value) or is_name_unused_in_contexts(value, contexts):
+    if is_name_unused_in_contexts(value, contexts):
         errors.append(
             ErrorInfo.from_node(value, "Value is unused, use `for x in range(len(y))` instead")
         )

refurb/checks/common.py

@@ -71,7 +71,7 @@ def check_for_loop_like(
 ) -> None:
     match node:
         case ForStmt(index=index, expr=expr):
-            func(index, expr, [], errors)
+            func(index, expr, [node.body], errors)
 
         case GeneratorExpr(
             indices=[index],
@@ -228,14 +228,7 @@ def was_read(self) -> int:
         return self.read_count > 0
 
 
-def is_placeholder(name: NameExpr) -> bool:
-    return unmangle_name(name.name) == "_"
-
-
 def is_name_unused_in_contexts(name: NameExpr, contexts: list[Node]) -> bool:
-    if not contexts:
-        return False
-
     for ctx in contexts:
         visitor = ReadCountVisitor(name)
         visitor.accept(ctx)

test/data/err_135.py

@@ -30,9 +30,21 @@ def f5():
     {v: "" for k, v in d.items()}  # "k" is unused, warn
 
 
+def f6():
+    k=v=0
+
+    # don't warn because we can't know if "k" or "v" are unused simply by
+    # looking at the for block, we need to account for the surrounding context,
+    # which is not possible currently.
+    for k, v in d.items():
+        pass
+
+    print(k, v)
+
+
 # these should not
 
-def f6():
+def f7():
     for k, v in d.items():
         print(k, v)
 
@@ -41,24 +53,13 @@ class Shelf:
     def items(self) -> list[tuple[str, int]]:
         return [("bagels", 123)]
 
-def f7():
+def f8():
     shelf = Shelf()
 
     for name, count in shelf.items():
         pass
 
 
-def f8():
-    k=v=0
-
-    # don't warn because we can't know if "k" or "v" are unused simply by
-    # looking at the for block, we need to account for the surrounding context,
-    # which is not possible currently.
-    for k, v in d.items():
-        pass
-
-    print(k, v)
-
 def f9():
     {k: "" for k, v in d.items() if v}
     {v: "" for k, v in d.items() if k}

test/data/err_135.txt

@@ -8,3 +8,5 @@ test/data/err_135.py:26:15 [FURB135]: Value is unused, use `for key in d` instea
 test/data/err_135.py:27:12 [FURB135]: Key is unused, use `for value in d.values()` instead
 test/data/err_135.py:29:19 [FURB135]: Value is unused, use `for key in d` instead
 test/data/err_135.py:30:16 [FURB135]: Key is unused, use `for value in d.values()` instead
+test/data/err_135.py:39:9 [FURB135]: Key is unused, use `for value in d.values()` instead
+test/data/err_135.py:39:12 [FURB135]: Value is unused, use `for key in d` instead

test/data/err_148.py

@@ -25,12 +25,11 @@
 
 _ = (index for index, _ in enumerate(nums3))
 
-
-# these should not
-
 for index, num in enumerate(nums):
     pass
 
+# these should not
+
 # "count" is an infinite generator. In general, we only want to warn on
 # sequence types because you can call len() on them without exhausting some
 # iterator.
@@ -39,3 +38,9 @@
     pass
 
 _ = (num for index, num in enumerate(nums) if index)
+
+for index, _ in enumerate(nums):
+    print(index, _)
+
+_ = ((index, _) for index, _ in enumerate(nums))
+_ = ((_, num) for _, num in enumerate(nums))

test/data/err_148.txt

@@ -9,3 +9,5 @@ test/data/err_148.py:19:19 [FURB148]: Value is unused, use `for x in range(len(y
 test/data/err_148.py:21:25 [FURB148]: Index is unused, use `for x in y` instead
 test/data/err_148.py:21:32 [FURB148]: Value is unused, use `for x in range(len(y))` instead
 test/data/err_148.py:26:23 [FURB148]: Value is unused, use `for x in range(len(y))` instead
+test/data/err_148.py:28:5 [FURB148]: Index is unused, use `for x in y` instead
+test/data/err_148.py:28:12 [FURB148]: Value is unused, use `for x in range(len(y))` instead