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.

Sign up for GitHub

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

Jump to bottom

Warn about dict.fromkeys(keys, mutable) #4613

Closed
janosh opened this issue May 23, 2023 · 4 comments · Fixed by #9597
Closed

Warn about dict.fromkeys(keys, mutable) #4613

janosh opened this issue May 23, 2023 · 4 comments · Fixed by #9597
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule

Comments

@janosh
Copy link

janosh commented May 23, 2023

ruff should error and suggest the following fix when passing a mutable 2nd arg to dict.fromkeys() since modifications will affect all keys which is a common pitfall.

# bad
dict.fromkeys((1, 2), {})
dict.fromkeys((1, 2), [])
dict.fromkeys((1, 2), set())
# ...

# good
{key: {} for key in (1, 2)}
{key: [] for key in (1, 2)}
{key: set() for key in (1, 2)}

Examples:
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label May 24, 2023
@charliermarsh
Copy link
Member

Oh interesting, I've never seen that!

@janosh janosh changed the title Error on dict.fromkeys(keys, mutable) Warn about dict.fromkeys(keys, mutable) May 24, 2023
@Skylion007
Copy link
Contributor

Flake8-bugbear might be interested in adding this rule too, you should open an issue there as well.

@janosh janosh mentioned this issue May 28, 2023
Open
@charliermarsh charliermarsh added the needs-decision Awaiting a decision from a maintainer label Jul 10, 2023
@tjkuson tjkuson mentioned this issue Jan 20, 2024
Closed
@charliermarsh charliermarsh added accepted Ready for implementation and removed needs-decision Awaiting a decision from a maintainer labels Jan 20, 2024
@charliermarsh
Copy link
Member

Marking as accepted, makes sense to me.

@tjkuson
Copy link
Contributor

tjkuson commented Jan 20, 2024

I'll implement this tomorrow (as a precursor to #9592)!

@tjkuson tjkuson mentioned this issue Jan 21, 2024
Merged
charliermarsh pushed a commit that referenced this issue Jan 22, 2024
@tjkuson
[ruff] Implement mutable-fromkeys-value (RUF024) (#9597)
1e4b421 
## Summary

Implement rule `mutable-fromkeys-value` (`RUF023`).

Autofixes

```python
dict.fromkeys(foo, [])
```

to

```python
{key: [] for key in foo}
```

The fix is marked as unsafe as it changes runtime behaviour. It also
uses `key` as the comprehension variable, which may not always be
desired.

Closes #4613.

## Test Plan

`cargo test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ruff] Implement mutable-fromkeys-value (RUF024) tjkuson/ruff
4 participants
@charliermarsh @Skylion007 @tjkuson @janosh