New message use-yield-from (R1737) (#9419)

Suggest refactoring when yielding from a for loop can be replaced by 
'yield from'. This message is only emitted when there are no other 
statements in the containing for loop.

There are too many possible generators to use in a for loop and handling
all the cases to provide a human friendly display name in emitted
messages seems too cumbersome compared to the benefit it brings,
specially considering this message will have a doc page with examples.

It is invalid to use 'yield from' in async functions. Replacing yielding
from an 'async for' loop may not be the same as using 'yield from' so
the message won't be emitted for async loops as well.

Closes #9229

Author: crazybolillo

doc/data/messages/u/use-yield-from/bad.py

@@ -0,0 +1,3 @@
+def bad_yield_from(generator):
+    for item in generator:  # [use-yield-from]
+        yield item

doc/data/messages/u/use-yield-from/details.rst

@@ -0,0 +1,14 @@
+:code:`yield from` can be thought of as removing the intermediary (your for loop) between the function caller and the
+requested generator. This enables the caller to directly communicate with the generator (e.g. using :code:`send()`).
+This communication is not possible when manually yielding each element one by one in a loop.
+
+PEP 380 describes the possibility of adding optimizations specific to :code:`yield from`. It looks like they
+have not been implemented as of the time of writing. Even without said optimizations, the following snippet shows
+that :code:`yield from` is marginally faster.
+
+.. code-block:: sh
+
+  $ python3 -m timeit "def yield_from(): yield from range(100)" "for _ in yield_from(): pass"
+  100000 loops, best of 5: 2.44 usec per loop
+  $ python3 -m timeit "def yield_loop():" "    for item in range(100): yield item" "for _ in yield_loop(): pass"
+  100000 loops, best of 5: 2.49 usec per loop

doc/data/messages/u/use-yield-from/good.py

@@ -0,0 +1,2 @@
+def good_yield_from(generator):
+    yield from generator

doc/data/messages/u/use-yield-from/related.rst

@@ -0,0 +1 @@
+- `PEP 380 <https://peps.python.org/pep-0380/>`_

doc/user_guide/checkers/features.rst

@@ -906,6 +906,9 @@ Refactoring checker Messages
 :unnecessary-comprehension (R1721): *Unnecessary use of a comprehension, use %s instead.*
   Instead of using an identity comprehension, consider using the list, dict or
   set constructor. It is faster and simpler.
+:use-yield-from (R1737): *Use 'yield from' directly instead of yielding each element one by one*
+  Yielding directly from the iterator is faster and arguably cleaner code than
+  yielding each element one by one in the loop.
 :use-a-generator (R1729): *Use a generator instead '%s(%s)'*
   Comprehension inside of 'any', 'all', 'max', 'min' or 'sum' is unnecessary. A
   generator would be sufficient and faster.

doc/user_guide/messages/messages_overview.rst

@@ -544,6 +544,7 @@ All messages in the refactor category:
    refactor/use-dict-literal
    refactor/use-list-literal
    refactor/use-set-for-membership
+   refactor/use-yield-from
    refactor/useless-object-inheritance
    refactor/useless-option-value
    refactor/useless-return

doc/whatsnew/fragments/9229.feature

@@ -0,0 +1,3 @@
+New message `use-yield-from` added to the refactoring checker. This message is emitted when yielding from a loop can be replaced by `yield from`.
+
+Closes #9229.

pylint/checkers/refactoring/refactoring_checker.py

@@ -483,6 +483,12 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "value by index lookup. "
             "The value can be accessed directly instead.",
         ),
+        "R1737": (
+            "Use 'yield from' directly instead of yielding each element one by one",
+            "use-yield-from",
+            "Yielding directly from the iterator is faster and arguably cleaner code than yielding each element "
+            "one by one in the loop.",
+        ),
     }
     options = (
         (
@@ -1123,6 +1129,27 @@ def visit_call(self, node: nodes.Call) -> None:
         self._check_use_list_literal(node)
         self._check_use_dict_literal(node)
 
+    @utils.only_required_for_messages("use-yield-from")
+    def visit_yield(self, node: nodes.Yield) -> None:
+        if not isinstance(node.value, nodes.Name):
+            return
+
+        parent = node.parent.parent
+        if (
+            not isinstance(parent, nodes.For)
+            or isinstance(parent, nodes.AsyncFor)
+            or len(parent.body) != 1
+        ):
+            return
+
+        if parent.target.name != node.value.name:
+            return
+
+        if isinstance(node.frame(), nodes.AsyncFunctionDef):
+            return
+
+        self.add_message("use-yield-from", node=parent, confidence=HIGH)
+
     @staticmethod
     def _has_exit_in_scope(scope: nodes.LocalsDictNodeNG) -> bool:
         exit_func = scope.locals.get("exit")

tests/functional/b/bad_reversed_sequence.py

@@ -1,6 +1,6 @@
 """ Checks that reversed() receive proper argument """
 # pylint: disable=missing-docstring
-# pylint: disable=too-few-public-methods
+# pylint: disable=too-few-public-methods,use-yield-from
 from collections import deque, OrderedDict
 from enum import IntEnum
 

tests/functional/c/consider/consider_using_enumerate.py

@@ -1,6 +1,6 @@
 """Emit a message for iteration through range and len is encountered."""
 
-# pylint: disable=missing-docstring, import-error, unsubscriptable-object, too-few-public-methods, unnecessary-list-index-lookup
+# pylint: disable=missing-docstring, import-error, unsubscriptable-object, too-few-public-methods, unnecessary-list-index-lookup, use-yield-from
 
 def bad():
     iterable = [1, 2, 3]