From ea73baecdd94deea5336c062647697448513a933 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Thu, 6 Jun 2024 13:54:09 +0100 Subject: [PATCH] Fix false positive in `use-yield-from` when using `yield` return (#9700) If the return value from `yield` is inspected inline, such as by (augmented) assignment, changing the looped `yield` to `yield from` is very likely to change the semantics of the generator, since there is an implicit use of `generator.send`. Closes #9696 --- doc/whatsnew/fragments/9696.false_positive | 3 +++ .../checkers/refactoring/refactoring_checker.py | 15 +++++++++------ tests/functional/u/use/use_yield_from.py | 10 ++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 doc/whatsnew/fragments/9696.false_positive diff --git a/doc/whatsnew/fragments/9696.false_positive b/doc/whatsnew/fragments/9696.false_positive new file mode 100644 index 0000000000..5d6e8cbf11 --- /dev/null +++ b/doc/whatsnew/fragments/9696.false_positive @@ -0,0 +1,3 @@ +Fix a false positive for ``use-yield-from`` when using the return value from the ``yield`` atom. + +Closes #9696 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 94e722b177..9ffcecce12 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -1169,21 +1169,24 @@ def visit_yield(self, node: nodes.Yield) -> None: if not isinstance(node.value, nodes.Name): return - parent = node.parent.parent + loop_node = node.parent.parent if ( - not isinstance(parent, nodes.For) - or isinstance(parent, nodes.AsyncFor) - or len(parent.body) != 1 + not isinstance(loop_node, nodes.For) + or isinstance(loop_node, nodes.AsyncFor) + or len(loop_node.body) != 1 + # Avoid a false positive if the return value from `yield` is used, + # (such as via Assign, AugAssign, etc). + or not isinstance(node.parent, nodes.Expr) ): return - if parent.target.name != node.value.name: + if loop_node.target.name != node.value.name: return if isinstance(node.frame(), nodes.AsyncFunctionDef): return - self.add_message("use-yield-from", node=parent, confidence=HIGH) + self.add_message("use-yield-from", node=loop_node, confidence=HIGH) @staticmethod def _has_exit_in_scope(scope: nodes.LocalsDictNodeNG) -> bool: diff --git a/tests/functional/u/use/use_yield_from.py b/tests/functional/u/use/use_yield_from.py index 2ccbb6d77e..f4dcdc6b9e 100644 --- a/tests/functional/u/use/use_yield_from.py +++ b/tests/functional/u/use/use_yield_from.py @@ -57,3 +57,13 @@ async def async_for_yield(agen): async def async_yield(agen): for item in agen: yield item + + +# If the return from `yield` is used inline, don't suggest delegation. + +def yield_use_send(): + for item in (1, 2, 3): + _ = yield item + total = 0 + for item in (1, 2, 3): + total += yield item