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

assign list.sort() instead of sorted(list) #5722

Closed
orSolocate opened this issue Jan 25, 2022 · 4 comments · Fixed by #5738
Closed

assign list.sort() instead of sorted(list) #5722

orSolocate opened this issue Jan 25, 2022 · 4 comments · Fixed by #5738
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors
Milestone

Comments

@orSolocate
Copy link
Contributor

orSolocate commented Jan 25, 2022

Current problem

when you use the sort() method on a list object, it modifies the list and returns None.
A common mistake is to assign the return value of this assignment (i.e. None) to a variable thinking you assigned the sorted list instead.

lst=[3,2,1]
A=lst.sort()

Results with:
None in A and [1,2,3] in lst.

Desired solution

The checker should raise the WARNING message:
“A sort method of list object is assigned to a variable and equals None, consider using the sorted() function instead”.

Additional context

When i run pylint with all extensions on the example above I don't get any message.
But PyCharm's linter has a message for it so I am not sure if it was already implemented.

If not I could write it.

@orSolocate orSolocate added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 25, 2022
@DanielNoord
Copy link
Collaborator

I like the idea although I wonder how opinionated this might be. Personally I never use .sort() to return None, but others might actually rely on that behaviour.

@Pierre-Sassoulas Should this be a plugin or a standard check?

@Pierre-Sassoulas
Copy link
Member

I also like this. Don't we have 'assignment-from-none' already? Is it a false negative?

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Proposal 📨 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 26, 2022
@DanielNoord
Copy link
Collaborator

Yes, I think we can consider this a false-positive.

@Pierre-Sassoulas Pierre-Sassoulas added the Help wanted 🙏 Outside help would be appreciated, good for new contributors label Jan 26, 2022
@orSolocate
Copy link
Contributor Author

In the list.sort() method documentation they write:

Sort the list in ascending order and return None.

So I assume it always returns None.. therefore I don't see a case when someone relies on this value.
If they did, they should assign None to the variable/expression they want to evaluate..

@Pierre-Sassoulas I could try to expand the 'assignment-from-none' checker...

@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code and removed Proposal 📨 Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Jan 28, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants