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

Add a rule for defaultdict(default_factory=list) #9509

Closed
hauntsaninja opened this issue Jan 14, 2024 · 6 comments
Closed

Add a rule for defaultdict(default_factory=list) #9509

hauntsaninja opened this issue Jan 14, 2024 · 6 comments
Assignees
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule

Comments

@hauntsaninja
Copy link
Contributor

Kwargs in the defaultdict constructor are forwarded to the dict constructor, so this creates a defaultdict with {"default_factory": list}. Since this is a valid call, type checkers may or may not catch this (and even when they catch it, the error might be a little confusing). ruff could lint this specific situation and mention use of collections.defaultdict(list) instead.

@hauntsaninja hauntsaninja changed the title Add a rule for collections.defaultdict(default_factory=list) Add a rule for defaultdict(default_factory=list) Jan 14, 2024
@charliermarsh charliermarsh added rule Implementing or modifying a lint rule accepted Ready for implementation labels Jan 14, 2024
@charliermarsh
Copy link
Member

Seems like a good rule.

@Skylion007
Copy link

@hauntsaninja You should also open the issue in the flake8-bugbear repo, they may be interested in it.

@mikeleppane
Copy link
Contributor

I can take this.

@mikeleppane
Copy link
Contributor

@hauntsaninja did you use 'list' as an example or I assume this applies to any callable? I tried default_factory with mypy 1.7.1 and it truly seems to be confused with different input types (list, int, dict, lambda, ...).

@hauntsaninja
Copy link
Contributor Author

As an example! :-) Like I said, type checkers can't catch the call itself since it's valid. They may be able to surface an issue later because they'll infer a different type for the defaultdict.

charliermarsh pushed a commit that referenced this issue Jan 26, 2024
… (`RUF026`) (#9651)

## Summary

Add a rule for defaultdict(default_factory=callable). Instead suggest
using defaultdict(callable).

See: #9509

If a user tries to bind a "non-callable" to default_factory, the rule
ignores it. Another option would be to warn that it's probably not what
you want. Because Python allows the following:

```python 
from collections import defaultdict

defaultdict(default_factory=1)
```
this raises after you actually try to use it:

```python
dd = defaultdict(default_factory=1)
dd[1]
```
=> 
```bash
KeyError: 1
```

Instead using callable directly in the constructor it will raise (not
being a callable):

```python 
from collections import defaultdict

defaultdict(1)
```
=> 
```bash
TypeError: first argument must be callable or None
```




## Test Plan

```bash
cargo test
```
@charliermarsh
Copy link
Member

Added in #9651, thanks @mikeleppane!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants