Fix a crash when __all__ exists but cannot be inferred
#8741
Conversation
jacobtylerwalls
added
Crash π₯
jacobtylerwalls
changed the title
__all__ exists but cannot be inferred__all__ exists but cannot be inferred
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8741 +/- ##
=======================================
Coverage 95.82% 95.82%
=======================================
Files 173 173
Lines 18381 18384 +3
=======================================
+ Hits 17613 17616 +3
Misses 768 768
|
|
|
||
| Other tests for undefined-all-variable in tests/functional/n/names_in__all__.py""" | ||
|
|
||
| __all__ += [] # [undefined-variable] |
There was a problem hiding this comment.
I'm not well-versed in using __all__. Is it the case that it doesn't exist until it's defined explicitly? (As opposed to being an always-defined global)
There was a problem hiding this comment.
I probably shouldn't have used the word "exists".
When __all__ isn't defined, you lose the ability to do:
>>> from pylint import *
>>> version
'3.0.0b1'Is it the case that it doesn't exist until it's defined explicitly? (As opposed to being an always-defined global)
Indeed!
>>> import astroid
>>> astroid.__all__
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 'astroid' has no attribute '__all__'. Did you mean: '__file__'?There was a problem hiding this comment.
Versus
>>> pylint.__all__
['__version__', 'version', 'modify_sys_path', 'run_pylint', 'run_symilar', 'run_pyreverse']
|
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit e770b48 |
| @@ -3046,7 +3046,10 @@ def _check_module_attrs( | |||
| def _check_all( | |||
| self, node: nodes.Module, not_consumed: dict[str, list[nodes.NodeNG]] | |||
| ) -> None: | |||
| assigned = next(node.igetattr("__all__")) | |||
| try: | |||
| assigned = next(node.igetattr("__all__")) | |||
There was a problem hiding this comment.
Why don't we use safe_infer here?
There was a problem hiding this comment.
Do you know of a way to use a helper with igetattr (or to use something besides igetattr, here)?
There was a problem hiding this comment.
Ah, of course.. Well, seems like we need safe_igetattr π
There was a problem hiding this comment.
Thank you @jacobtylerwalls. Just wondering if an internal checker for unsafe inference wouldn't be more practical than typing astroid properly π
(cherry picked from commit 331e48f)
(cherry picked from commit 331e48f)
(cherry picked from commit 331e48f) Co-authored-by: Jacob Walls <[email protected]>
Type of Changes
Description
Closes #8740