diff --git a/doc/data/messages/u/use-yield-from/bad.py b/doc/data/messages/u/use-yield-from/bad.py new file mode 100644 index 0000000000..012bc42f46 --- /dev/null +++ b/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 diff --git a/doc/data/messages/u/use-yield-from/details.rst b/doc/data/messages/u/use-yield-from/details.rst new file mode 100644 index 0000000000..037248a872 --- /dev/null +++ b/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 diff --git a/doc/data/messages/u/use-yield-from/good.py b/doc/data/messages/u/use-yield-from/good.py new file mode 100644 index 0000000000..1331a6fd29 --- /dev/null +++ b/doc/data/messages/u/use-yield-from/good.py @@ -0,0 +1,2 @@ +def good_yield_from(generator): + yield from generator diff --git a/doc/data/messages/u/use-yield-from/related.rst b/doc/data/messages/u/use-yield-from/related.rst new file mode 100644 index 0000000000..826aca35f2 --- /dev/null +++ b/doc/data/messages/u/use-yield-from/related.rst @@ -0,0 +1 @@ +- `PEP 380 `_ diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index 84b967a82c..7dce5c43b8 100644 --- a/doc/user_guide/checkers/features.rst +++ b/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. diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst index 8d9f197502..e928ac1a24 100644 --- a/doc/user_guide/messages/messages_overview.rst +++ b/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 diff --git a/doc/whatsnew/fragments/9229.feature b/doc/whatsnew/fragments/9229.feature new file mode 100644 index 0000000000..22d059ecdf --- /dev/null +++ b/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. diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 97e509025c..14b8adbc27 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/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") diff --git a/tests/functional/b/bad_reversed_sequence.py b/tests/functional/b/bad_reversed_sequence.py index c4380491e9..d9c3c68b3f 100644 --- a/tests/functional/b/bad_reversed_sequence.py +++ b/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 diff --git a/tests/functional/c/consider/consider_using_enumerate.py b/tests/functional/c/consider/consider_using_enumerate.py index 1b8bb4c6b1..b49ffd9f0b 100644 --- a/tests/functional/c/consider/consider_using_enumerate.py +++ b/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] diff --git a/tests/functional/ext/docparams/return/missing_return_doc_Sphinx.py b/tests/functional/ext/docparams/return/missing_return_doc_Sphinx.py index 41b0ce1ae5..1351d1cef8 100644 --- a/tests/functional/ext/docparams/return/missing_return_doc_Sphinx.py +++ b/tests/functional/ext/docparams/return/missing_return_doc_Sphinx.py @@ -1,7 +1,7 @@ """Tests for missing-return-doc and missing-return-type-doc for Sphinx style docstrings""" # pylint: disable=function-redefined, invalid-name, undefined-variable, missing-function-docstring # pylint: disable=unused-argument, disallowed-name, too-few-public-methods, missing-class-docstring -# pylint: disable=unnecessary-pass +# pylint: disable=unnecessary-pass, use-yield-from import abc diff --git a/tests/functional/i/iterable_context_py36.py b/tests/functional/i/iterable_context_py36.py index d50d3da981..ecb48a241f 100644 --- a/tests/functional/i/iterable_context_py36.py +++ b/tests/functional/i/iterable_context_py36.py @@ -1,4 +1,4 @@ -# pylint: disable=missing-docstring,too-few-public-methods,unused-variable,unnecessary-comprehension +# pylint: disable=missing-docstring,too-few-public-methods,unused-variable,unnecessary-comprehension,use-yield-from import asyncio class AIter: diff --git a/tests/functional/n/non/non_iterator_returned.py b/tests/functional/n/non/non_iterator_returned.py index 3bc24a23e0..7dffd711ec 100644 --- a/tests/functional/n/non/non_iterator_returned.py +++ b/tests/functional/n/non/non_iterator_returned.py @@ -1,6 +1,6 @@ """Check non-iterators returned by __iter__ """ -# pylint: disable=too-few-public-methods, missing-docstring, consider-using-with, import-error +# pylint: disable=too-few-public-methods, missing-docstring, consider-using-with, import-error, use-yield-from from uninferable import UNINFERABLE class FirstGoodIterator: diff --git a/tests/functional/s/stop_iteration_inside_generator.py b/tests/functional/s/stop_iteration_inside_generator.py index fcd20a6836..4b034e2506 100644 --- a/tests/functional/s/stop_iteration_inside_generator.py +++ b/tests/functional/s/stop_iteration_inside_generator.py @@ -2,7 +2,7 @@ Test that no StopIteration is raised inside a generator """ # pylint: disable=missing-docstring,invalid-name,import-error, try-except-raise, wrong-import-position -# pylint: disable=not-callable,raise-missing-from,broad-exception-raised +# pylint: disable=not-callable,raise-missing-from,broad-exception-raised,use-yield-from import asyncio class RebornStopIteration(StopIteration): diff --git a/tests/functional/u/undefined/undefined_variable.py b/tests/functional/u/undefined/undefined_variable.py index e1b66910fc..194de114d3 100644 --- a/tests/functional/u/undefined/undefined_variable.py +++ b/tests/functional/u/undefined/undefined_variable.py @@ -1,7 +1,7 @@ # pylint: disable=missing-docstring, multiple-statements, import-outside-toplevel # pylint: disable=too-few-public-methods, bare-except, broad-except # pylint: disable=using-constant-test, import-error, global-variable-not-assigned, unnecessary-comprehension -# pylint: disable=unnecessary-lambda-assignment +# pylint: disable=unnecessary-lambda-assignment, use-yield-from from typing import TYPE_CHECKING diff --git a/tests/functional/u/unpacking/unpacking_non_sequence.py b/tests/functional/u/unpacking/unpacking_non_sequence.py index feb465ecbe..0a13c656c8 100644 --- a/tests/functional/u/unpacking/unpacking_non_sequence.py +++ b/tests/functional/u/unpacking/unpacking_non_sequence.py @@ -1,6 +1,6 @@ """Check unpacking non-sequences in assignments. """ -# pylint: disable=too-few-public-methods, invalid-name, attribute-defined-outside-init, unused-variable +# pylint: disable=too-few-public-methods, invalid-name, attribute-defined-outside-init, unused-variable, use-yield-from # pylint: disable=using-constant-test, missing-docstring, wrong-import-order,wrong-import-position,no-else-return from os import rename as nonseq_func from functional.u.unpacking.unpacking import nonseq diff --git a/tests/functional/u/use/use_implicit_booleaness_not_len.py b/tests/functional/u/use/use_implicit_booleaness_not_len.py index 79547d99e1..1261aa3014 100644 --- a/tests/functional/u/use/use_implicit_booleaness_not_len.py +++ b/tests/functional/u/use/use_implicit_booleaness_not_len.py @@ -1,4 +1,4 @@ -# pylint: disable=too-few-public-methods,import-error, missing-docstring +# pylint: disable=too-few-public-methods,import-error, missing-docstring, use-yield-from # pylint: disable=useless-super-delegation,wrong-import-position,invalid-name, wrong-import-order, condition-evals-to-constant if len('TEST'): # [use-implicit-booleaness-not-len] diff --git a/tests/functional/u/use/use_yield_from.py b/tests/functional/u/use/use_yield_from.py new file mode 100644 index 0000000000..2ccbb6d77e --- /dev/null +++ b/tests/functional/u/use/use_yield_from.py @@ -0,0 +1,59 @@ +# pylint: disable=missing-docstring, import-error, yield-outside-function +import factory +from magic import shazam, turbogen + +yield 1 + +def bad(generator): + for item in generator: # [use-yield-from] + yield item + + +def out_of_names(): + for item in turbogen(): # [use-yield-from] + yield item + + +def good(generator): + for item in generator: + shazam() + yield item + + +def yield_something(): + yield 5 + + +def yield_attr(): + for item in factory.gen(): # [use-yield-from] + yield item + + +def yield_attr_nested(): + for item in factory.kiwi.gen(): # [use-yield-from] + yield item + + +def yield_expr(): + for item in [1, 2, 3]: # [use-yield-from] + yield item + + +def for_else_yield(gen, something): + for item in gen(): + if shazam(item): + break + else: + yield something + + +# yield from is not supported in async functions, so the following are fine + +async def async_for_yield(agen): + async for item in agen: + yield item + + +async def async_yield(agen): + for item in agen: + yield item diff --git a/tests/functional/u/use/use_yield_from.txt b/tests/functional/u/use/use_yield_from.txt new file mode 100644 index 0000000000..fd77d31eb3 --- /dev/null +++ b/tests/functional/u/use/use_yield_from.txt @@ -0,0 +1,5 @@ +use-yield-from:8:4:9:18:bad:Use 'yield from' directly instead of yielding each element one by one:HIGH +use-yield-from:13:4:14:18:out_of_names:Use 'yield from' directly instead of yielding each element one by one:HIGH +use-yield-from:28:4:29:18:yield_attr:Use 'yield from' directly instead of yielding each element one by one:HIGH +use-yield-from:33:4:34:18:yield_attr_nested:Use 'yield from' directly instead of yielding each element one by one:HIGH +use-yield-from:38:4:39:18:yield_expr:Use 'yield from' directly instead of yielding each element one by one:HIGH diff --git a/tests/functional/y/yield_assign.py b/tests/functional/y/yield_assign.py index e7a938c692..841fe5db31 100644 --- a/tests/functional/y/yield_assign.py +++ b/tests/functional/y/yield_assign.py @@ -1,3 +1,4 @@ +# pylint: disable=use-yield-from """https://www.logilab.org/ticket/8771"""