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

False negative assigned-from-none for value = dict.update({}, {}) #8714

Closed
jamesbraza opened this issue May 24, 2023 · 2 comments · Fixed by #8809 or #9093
Closed

False negative assigned-from-none for value = dict.update({}, {}) #8714

jamesbraza opened this issue May 24, 2023 · 2 comments · Fixed by #8809 or #9093
Assignees
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 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@jamesbraza
Copy link

jamesbraza commented May 24, 2023

Current problem

This mistake recently bit a coworker of mine:

some_dict = {0: 1}.update({2: 3})
# This will fail, as `some_dict` is actually `None`
assert isinstance(some_dict, dict)

Obviously this type of error one should catch in unit testing, but we didn't have tests. mypy can catch this only if one adds type annotations.

I am wondering, can pylint catch this in the future, without type annotations?

Desired solution

pylint would:

  1. Understand that some_dict is actually None
  2. Know that its usage downstream as a dict will fail

In other words the request: pylint catching errors associated with None returns later being used as if not None.

Additional context

No response

@jamesbraza jamesbraza added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label May 24, 2023
@Pierre-Sassoulas Pierre-Sassoulas added Help wanted 🙏 Outside help would be appreciated, good for new contributors False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels May 24, 2023
@Pierre-Sassoulas
Copy link
Member

Sounds great ! When switching between language you never know if a data structure is modified in place or if it's returned, so I think it's very useful to warn about it. Also I think it's a false negative of assignment-from-none.

@jamesbraza
Copy link
Author

jamesbraza commented May 24, 2023

Oh wow, I forgot about assignment-from-none, so it sounds like this request already is handled by pylint 🥳.

Would you like me to rename this issue to just be about the false negative?

@jamesbraza jamesbraza changed the title Request: detecting failures associated with methods returning None False negative assigned-from-none for value = dict.update({}, {}) May 26, 2023
@jacobtylerwalls jacobtylerwalls self-assigned this Jul 1, 2023
@jacobtylerwalls jacobtylerwalls added this to the 3.0.0a7 milestone Jul 1, 2023
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 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
3 participants