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

Avoid duplicate inference results for List[int] #2185

Merged

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Fix a regression in 3.0.0.a3 that became necessary once we fixed the cache key in 0740a0d.

If we re-run the inference function for inferring a slice like List[int], we risk creating so many duplicates, especially if used in several files, that we hit the limit for max inference and end up introducing an Uninferable, which can cause some pylint checks to not fire.

This explains (and resolves!) the failures seen in pylint-dev/pylint#8685 (comment), which we were only seeing when running the test suite as a whole.

Skipping news because we're not writing changelogs specific to each alpha.

This should unblock pylint-dev/pylint#8685.

This became necessary once we fixed the cache key in 0740a0d.
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #2185 (3b91e01) into main (12c3d15) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2185   +/-   ##
=======================================
  Coverage   92.61%   92.61%           
=======================================
  Files          94       94           
  Lines       10812    10813    +1     
=======================================
+ Hits        10013    10014    +1     
  Misses        799      799           
Flag Coverage Ξ”
linux 92.37% <100.00%> (+<0.01%) ⬆️
pypy 87.57% <100.00%> (+<0.01%) ⬆️
windows 92.20% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Ξ”
astroid/brain/brain_typing.py 86.11% <100.00%> (+0.09%) ⬆️

@@ -316,6 +316,9 @@ def infer_typing_alias(
# This is an issue in cases where the aliased class implements it,
# but the typing alias isn't subscriptable. E.g., `typing.ByteString` for PY39+
_forbid_class_getitem_access(class_def)

# Avoid re-instantiating this class every time it's seen
node._explicit_inference = lambda node, context: iter([class_def])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels like something we might need to do for a lot of brains.

We us a similar pattern for the enum brain. πŸ˜“

Copy link
Member Author

Choose a reason for hiding this comment

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

You're quite right. I traced the bulk of the primer noise to needing to do this in other places in the typing brain.

@DanielNoord DanielNoord merged commit 92fe829 into pylint-dev:main May 19, 2023
@jacobtylerwalls jacobtylerwalls deleted the avoid-multiple-fake-classes branch May 19, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants