Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive for use-yield-from for generators yielding from iterators #9696

Closed
jakelishman opened this issue Jun 5, 2024 · 0 comments · Fixed by #9700 or #9702
Closed

False positive for use-yield-from for generators yielding from iterators #9696

jakelishman opened this issue Jun 5, 2024 · 0 comments · Fixed by #9700 or #9702
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@jakelishman
Copy link
Contributor

jakelishman commented Jun 5, 2024

Bug description

Within a generator expression, pylint issues use-yield-from for constructs such as:

def my_generator[T, S](iterator: Iterable[T]) -> Generator[T, S, S]:
    for item in iterator:
        state = yield iterator
    return state

When iterator is an Iterator and not a Generator, it will not have the generator methods (e.g. send), so applying Pylint's suggestion will modify the type of my_generator from Generator[T, S, S] to Generator[T, None, None] (and will generally cause AttributeError to be raised on calls to send).

Example file generator.py:

import typing

iterable: list[int] = [1, 2, 3]

def gen1():
    for item in iterable:
        # Implicitly suggesting that `send` will be used.
        _ = yield item

# ... and ...

# Explicitly requiring that `send` must exist.
def gen2() -> typing.Generator[int, object, None]:
    for item in iterable:
        yield item

Configuration

No response

Command used

pylint generator.py

Pylint output

************* Module generator
generator.py:6:4: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)
generator.py:14:4: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)

Expected behavior

In many cases, I suspect that the message is worth the risk of the false positive:

def gen():
    for item in iterator:
        yield item

probably should emit the message, but imo neither of my examples in generator.py should, maybe unless iterable could be inferred to be a compatible type (which it explicitly isn't in my example).

Pylint version

pylint 3.2.2
astroid 3.2.2
Python 3.12.0rc3 (main, Sep 21 2023, 17:27:59) [Clang 16.0.6 ]

OS / Environment

No response

Additional dependencies

No response

@jakelishman jakelishman added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 5, 2024
jakelishman added a commit to jakelishman/pylint that referenced this issue Jun 6, 2024
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 pylint-dev#9696
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 6, 2024
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.2.3 milestone Jun 6, 2024
Pierre-Sassoulas pushed a commit that referenced this issue Jun 6, 2024
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
github-actions bot pushed a commit that referenced this issue Jun 6, 2024
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

(cherry picked from commit ea73bae)
Pierre-Sassoulas pushed a commit that referenced this issue Jun 6, 2024
…) (#9701)

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

(cherry picked from commit ea73bae)

Co-authored-by: Jake Lishman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
2 participants