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

New check: Use single-statement dict initialization #2876

Closed
nickdrozd opened this issue Apr 19, 2019 · 4 comments · Fixed by #7794
Closed

New check: Use single-statement dict initialization #2876

nickdrozd opened this issue Apr 19, 2019 · 4 comments · Fixed by #7794
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors
Milestone

Comments

@nickdrozd
Copy link
Collaborator

nickdrozd commented Apr 19, 2019

Here's an ugly pattern, commonly found in eg config files:

config = {}
config['dir'] = 'bin'
config['user'] = 'me'
config['workers'] = 5

An empty dict is initialized and then mutated several times. It would
be better to use declarative literal syntax:

config = {
    'dir': 'bin',
    'user': 'me',
    'workers': 5,
}

No mutation! No repetition!

I'm imagining a check that would look for an empty dict initialization
and then see if any following lines are bare dict assignments. It
could get tricky though, as this pattern horrible pattern is sometimes
recursive:

config = {}
config['dir'] = 'bin'
config['user'] = 'me'
config['workers'] = 5
config['options'] = {}
config['options']['debug'] = False
config['options']['verbose'] = True

Yuck!

And I know, a check like this gets released, CIs start showing failing
builds, and everyone thinks the sky is falling. Maybe make it
optional? Or a warning or something? Anything to discourage this
pattern.

@PCManticore
Copy link
Contributor

Sounds like a good improvement @nickdrozd This can be an optional check for now, in the form of an extension pylint.checkers.extensions, which needs to be explicitly enabled in the configuration file. Maybe we can enable it by default in pylint 3.0.

@PCManticore PCManticore added Good first issue Friendly and approachable by new contributors Enhancement ✨ Improvement to a component labels Apr 19, 2019
@nickdrozd
Copy link
Collaborator Author

@PCManticore I'd like to work on this one, but I'm not sure exactly where to start. What kind of node would this check attach to? One approach would be to look for assign statements with {} on the right side, then collect all immediately following assign statements where the right side indexes into the variable assigned to by the first statement. Does that sound right? Are there any existing checks that do something similar?

@PCManticore
Copy link
Contributor

@nickdrozd That approach sounds right to me. You should find some similar checks in checkers.refactoring, consider-using-get and simplifiable-if-statement come to mind.

@Pierre-Sassoulas Pierre-Sassoulas added the Help wanted 🙏 Outside help would be appreciated, good for new contributors label Mar 2, 2021
@nickdrozd nickdrozd changed the title New check: Use dict literal syntax New check: Use single-statement dict initialization Apr 16, 2021
@nickdrozd
Copy link
Collaborator Author

I changed the title of this issue to accommodate #4365

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
4 participants