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 checks: Use literals instead of calling the constructor #6542

Open
Sawbez opened this issue May 7, 2022 · 4 comments
Open

New checks: Use literals instead of calling the constructor #6542

Sawbez opened this issue May 7, 2022 · 4 comments
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@Sawbez
Copy link

Sawbez commented May 7, 2022

Current problem

Literals should be prioritized over calling their respective constructors because it is clearer.
Demonstrations of new checks:

"""Incorrect Code"""
mylist = list((1, 2, 3, 4, 5))  # [use-list-literal]
mytuple = tuple()  # [use-tuple-literal]
mytuple2 = tuple(['foo', 'bar'])  # [use-tuple-literal]
mydict = dict((('a', 1), ('b', 2)))  # [use-dict-literal]
myset = set(['py', 'lint'])  # [use-set-literal]

Should be (Correct code):

"""Correct Code"""
mylist = [1, 2, 3, 4, 5] 
mytuple = ()
mytuple2 = ('foo', 'bar')
mydict = {'a': 1, 'b': 2}
myset = {'py', 'lint'}

For both of the above pylint gives a rating of 10.0/10.0.

Desired solution

I propose a couple new checks, or they could all be merged into a use-literal instead.
The list example and dict example should be part of use-list-literal and use-dict-literal, and the set and tuple examples could be use-set-literal and use-tuple-literal. I can see some argument against the tuple literal, as it could be confusing in some parentheses-dense situations, but the other checks would almost always (if not always) make things clearer.

Additional context

Similar to issue #4365 but for set, tuple, and more complicated expressions.

@Sawbez Sawbez added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label May 7, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Proposal 📨 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels May 7, 2022
@Pierre-Sassoulas
Copy link
Member

Thank you for opening the issue. It definitely make sense to extends the check. I think we can keep the use-x-literal style as it permits to deactivate one or the other if you don't like, and will prevent some churn as two of the messages already exists.

@Sawbez
Copy link
Author

Sawbez commented May 8, 2022

@Pierre-Sassoulas I'm working on a version of this, but I'm not very familiar with astroid. I have the node with the call to the function, but I'm not sure how to check if the args and kwargs of the function are constants. Also, I'm not sure how to test my code, I don't see where I run the modified version of pylint.

@Pierre-Sassoulas
Copy link
Member

Also, I'm not sure how to test my code, I don't see where I run the modified version of pylint.

You should clone the repository and install pylint in a virtualenv with pip install -e . + pip install -r requirements_test.txt , then use pytest. There's more detailed information in the documentation. https://pylint.pycqa.org/en/latest/development_guide/testing.html

I have the node with the call to the function, but I'm not sure how to check if the args and kwargs of the function are constants.

I think you can do some check on the call's args/kwargs. It should return a list of nodes : https://pylint.pycqa.org/projects/astroid/en/latest/api/astroid.nodes.html#astroid.nodes.Call.args

@Sawbez
Copy link
Author

Sawbez commented May 8, 2022

@Pierre-Sassoulas Thank you! I got the code to work and the tests to run, and made a PR.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.0 milestone May 9, 2022
@DanielNoord DanielNoord removed this from the 2.15.0 milestone Aug 23, 2022
@DanielNoord DanielNoord added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants