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

B901: Misses yield sub-expresisons #14453

Open
MichaReiser opened this issue Nov 19, 2024 · 7 comments
Open

B901: Misses yield sub-expresisons #14453

MichaReiser opened this issue Nov 19, 2024 · 7 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Nov 19, 2024

A very contrived example but B901 misses yield expressions where they're not the expression of a ExprStmt.

def get_file_paths(file_types: Iterable[str] | None = None) -> Iterable[Path]:
    dir_path = Path(".")
    if file_types is None:
        return dir_path.glob("*")

    for file_type in file_types:
        (yield a for a in dir_path.glob(f"*.{file_type}"))

Note: we should make sure that we don't traverse into any lambda expressions...

@MichaReiser MichaReiser added bug Something isn't working good first issue Good for newcomers labels Nov 19, 2024
@AlexWaygood
Copy link
Member

def get_file_paths(file_types: Iterable[str] | None = None) -> Iterable[Path]:
    dir_path = Path(".")
    if file_types is None:
        return dir_path.glob("*")

    for file_type in file_types:
        (yield a for a in dir_path.glob(f"*.{file_type}"))

hmm, this isn't valid syntax:

>>> from typing import *
>>> from pathlib import Path
>>> def get_file_paths(file_types: Iterable[str] | None = None) -> Iterable[Path]:
...     dir_path = Path(".")
...     if file_types is None:
...         return dir_path.glob("*")
... 
...     for file_type in file_types:
...         (yield a for a in dir_path.glob(f"*.{file_type}"))
...         
  File "<python-input-2>", line 7
    (yield a for a in dir_path.glob(f"*.{file_type}"))
             ^^^
SyntaxError: invalid syntax

@MichaReiser
Copy link
Member Author

hmm, I only tested with our parser. nested yield expressions are definitely valid :)

@MichaReiser
Copy link
Member Author

Okay, found one

async def main():
    await (yield x)

@AlexWaygood
Copy link
Member

Here's another function that is a generator function with a return statement and is not flagged by B901:

def f():
    x = yield
    print(x)
    return 42

This is a generator function that you can send values into:

>>> def f():
...     x = yield
...     print(x)
...     return 42
...     
>>> y = f()
>>> next(y)
>>> y.send('foo')
foo
Traceback (most recent call last):
  File "<python-input-13>", line 1, in <module>
    y.send('foo')
    ~~~~~~^^^^^^^
StopIteration: 42

Sending valued into a generator is, like returning values from a generator, quite an advanced feature. So if I saw a function like this in code review, I would assume the user knew what they were doing. That doesn't feel like a very principled reason not to emit B901 on it, though; maybe we should. Not sure.

@rabelmervin
Copy link

Hi @MichaReiser @AlexWaygood can I help you fix this issue ?

@MichaReiser
Copy link
Member Author

@rabelmervin
Copy link

Thanks @MichaReiser excited to work on this !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants