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

B008 complains about function call when constructing a default parameter value as a exception #8378

Closed
RonnyPfannschmidt opened this issue Oct 31, 2023 · 7 comments · Fixed by #8501
Labels
needs-decision Awaiting a decision from a maintainer

Comments

@RonnyPfannschmidt
Copy link

https://play.ruff.rs/54edf8a4-4c2f-4c0e-a5d8-b7b4afa7a19e

a definition like

def pain( error: Exception | None = ValueError("Hosts weren't successfully added")):
    pass

triggers

B008: Do not perform function call `ValueError` in argument defaults`
@charliermarsh
Copy link
Member

I'm definitely open-minded about improving the rule, but can you say a bit more about why you think this case should be excluded? Alternatively, why prefer this over the in-body initialization pattern?

if error is None:
    error = ValueError("Hosts weren't successfully added")

@charliermarsh charliermarsh added the needs-info More information is needed from the issue author label Oct 31, 2023
@RonnyPfannschmidt
Copy link
Author

In my case none is a valid meaningful parameter variant

Creating sentinels as replacement is pretty painful

@charliermarsh
Copy link
Member

Ahh I see, so passing in error=None has meaningfully different behavior. And I assume you don't want to define the sentinel in the module body.

The challenge here is that I don't know how to avoid this without disabling the rule altogether. Even here, error isn't immutable, so calls to pain could modify the shared error value...

@RonnyPfannschmidt
Copy link
Author

The rule is about function calls,I believe it's fair game to allow construction of built-in exceptions

@charliermarsh charliermarsh added needs-decision Awaiting a decision from a maintainer and removed needs-info More information is needed from the issue author labels Oct 31, 2023
@RonnyPfannschmidt
Copy link
Author

https://github.com/PyCQA/flake8-bugbear/blob/907e0dd29a99818591a604d4557c70ea33204712/bugbear.py#L1660-L1667 has more details in the error

is rather barebones

i would recommend to extend the error with the suggestion

@charliermarsh
Copy link
Member

Does that suggestion solve the problem for you though? Do you consider that a reasonable workaround?

@RonnyPfannschmidt
Copy link
Author

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants