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

Fix unhashable-member crash when lambda used as a dict key #7454

Merged
merged 2 commits into from Sep 12, 2022

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes #7453

@jacobtylerwalls jacobtylerwalls added Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer Crash πŸ’₯ A bug that makes pylint crash labels Sep 11, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.15.3 milestone Sep 11, 2022
@jacobtylerwalls jacobtylerwalls changed the title Fix unhashable-member crash when lambda used as a dict key Fix unhashable-member crash when lambda used as a dict key Sep 11, 2022
@coveralls
Copy link

coveralls commented Sep 11, 2022

Pull Request Test Coverage Report for Build 3033283713

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0005%) to 95.315%

Totals Coverage Status
Change from base Build 3027465461: 0.0005%
Covered Lines: 17030
Relevant Lines: 17867

πŸ’› - Coveralls

@github-actions

This comment has been minimized.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick fix !

@@ -38,6 +38,15 @@
nodes.DictComp,
nodes.GeneratorExp,
)
WITH_IGETATTR = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to use has_attr("igetattr" and have something generic instead ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just figured this was more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old python is more duck-type-y, and modern python seems to be more explicit. πŸ€·πŸ»β€β™‚οΈ

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was more explicit.

Sure but if another class with igetattr appear we'd need to remember to add it there, which is unlikely to be caught in review, right ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, will change.

@github-actions
Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit a2450c6

@@ -1950,6 +1950,8 @@ def is_hashable(node: nodes.NodeNG) -> bool:
for inferred in node.infer():
if inferred is astroid.Uninferable:
return True
if not hasattr(inferred, "igetattr"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy will complain about this when we add py.typed to astroid, but that is a problem to fix then I think.

@DanielNoord DanielNoord merged commit aef4055 into pylint-dev:main Sep 12, 2022
@jacobtylerwalls jacobtylerwalls deleted the crash-lambda-key branch September 12, 2022 11:42
@Pierre-Sassoulas Pierre-Sassoulas added Backported and removed Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer labels Sep 16, 2022
Pierre-Sassoulas pushed a commit to Pierre-Sassoulas/pylint that referenced this pull request Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported Crash πŸ’₯ A bug that makes pylint crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash with .AstroidError when parsing a dict with a lambda as the key
4 participants