Skip to content

Fix a crash when __all__ exists but cannot be inferred #8741

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 6, 2023
Merged

Fix a crash when __all__ exists but cannot be inferred #8741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8740.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a crash when ``__all__`` exists but cannot be inferred.

Closes #8740
5 changes: 4 additions & 1 deletion pylint/checkers/variables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we use safe_infer here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you know of a way to use a helper with igetattr (or to use something besides igetattr, here)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, of course.. Well, seems like we need safe_igetattr 😄

except astroid.InferenceError:
return
if isinstance(assigned, util.UninferableBase):
return
if assigned.pytype() not in {"builtins.list", "builtins.tuple"}:
Expand Down
5 changes: 5 additions & 0 deletions tests/functional/u/undefined/undefined_all_variable_edge_case.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""Edge case: __all__ exists in module's locals, but cannot be inferred.

Other tests for undefined-all-variable in tests/functional/n/names_in__all__.py"""

__all__ += [] # [undefined-variable]
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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__'?

Copy link
Member Author

Choose a reason for hiding this comment

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

Versus

>>> pylint.__all__
['__version__', 'version', 'modify_sys_path', 'run_pylint', 'run_symilar', 'run_pyreverse']

1 change: 1 addition & 0 deletions tests/functional/u/undefined/undefined_all_variable_edge_case.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
undefined-variable:5:0:5:7::Undefined variable '__all__':UNDEFINED