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

pyreverse should check that klass is still ClassDef #9797

Open
bjmc opened this issue Jul 12, 2024 · 3 comments
Open

pyreverse should check that klass is still ClassDef #9797

bjmc opened this issue Jul 12, 2024 · 3 comments
Labels
Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation pyreverse Related to pyreverse component

Comments

@bjmc
Copy link

bjmc commented Jul 12, 2024

Ran into this when trying to use pyreverse, but I think this might be an astroid bug. It seems like the issue is that is_stdlib_module() expects a string module name, but in cases where the node is Uninferable then the special UninferableBase object returns self for almost all attribute access, including .name. This leads to confusing exceptions that are hard for a user to interpret.

Steps to reproduce

from astroid.util import Uninferable
from astroid.modutils import is_stdlib_module

# Example from pyreverse/diadefslib.py line 75:
# https://github.com/pylint-dev/pylint/blob/main/pylint/pyreverse/diadefslib.py#L75
node = Uninferable
is_stdlib_module(node.root().name)

Current behavior

Traceback (most recent call last):
  File "/home/bjmc/Sandbox/example.py", line 7, in <module>
    is_stdlib_module(node.root().name)
  File "/home/bjmc/.cache/pypoetry/virtualenvs/example-zPbyUnCb-py3.10/lib/python3.10/site-packages/astroid/modutils.py", line 524, in is_stdlib_module
    return modname.split(".")[0] in stdlib_module_names
TypeError: 'UninferableBase' object is not subscriptable

Expected behavior

return False (UninferableBase is not known to be in stdlib)

Astroid version

$ python -c "from astroid import __pkginfo__; print(__pkginfo__.version)"
3.2.3
@DanielNoord
Copy link
Collaborator

This bug makes sense. Where did you run into this? Can we just escape early in that path when we encounter an UniferableBase? Instead of adding more special cases to that __getattribute__.

@bjmc
Copy link
Author

bjmc commented Jul 15, 2024

I ran into it trying to run pyreverse on our codebase to generate some PlantUML.

The full traceback is as follows:

$ pyreverse -a 0 -o plantuml src/ska_oso_pdm/ -c ska_oso_pdm.sb_definition.DishAllocation
Traceback (most recent call last):
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/bin/pyreverse", line 8, in <module>
    sys.exit(run_pyreverse())
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/__init__.py", line 56, in run_pyreverse
    PyreverseRun(argv or sys.argv[1:])
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/main.py", line 297, in __init__
    sys.exit(self.run(args))
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/main.py", line 316, in run
    diadefs = handler.get_diadefs(project, linker)
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/diadefslib.py", line 230, in get_diadefs
    diagrams.append(generator.class_diagram(project, klass))
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/diadefslib.py", line 202, in class_diagram
    self.extract_classes(klass, anc_level, association_level)
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/diadefslib.py", line 117, in extract_classes
    if self.classdiagram.has_node(klass_node) or not self.show_node(klass_node):
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pylint/pyreverse/diadefslib.py", line 76, in show_node
    if is_stdlib_module(node.root().name):
  File "/home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/astroid/modutils.py", line 524, in is_stdlib_module
    return modname.split(".")[0] in stdlib_module_names
TypeError: 'UninferableBase' object is not subscriptable

Can we just escape early in that path when we encounter an UniferableBase?

On the face of it, that sounds like a decent idea to me, but I wouldn't have any idea where to start making those changes. I was hoping to treat pyreverse as a black box tool.

Instead of adding more special cases to that __getattribute__

Another alternative to avoid added complexity in __getattribute__ might be to define an UninferableBase.name property? My understanding is that __getattribute__() is only called as a fallback when an attribute doesn't exist on the object.

@DanielNoord
Copy link
Collaborator

The fix should be made here:

anc_level, association_level = self._get_levels()

pyreverse incorrectly assumes that klass = next(module.ilookup(klass)) still gives klass a type of ClassDef while that is clearly not the case.

I'm moving this issue to the pylint repository and renaming it.

@DanielNoord DanielNoord changed the title UninferableBase.name should be a string pyreverse should check that klass is still ClassDef Jul 15, 2024
@DanielNoord DanielNoord transferred this issue from pylint-dev/astroid Jul 15, 2024
@DanielNoord DanielNoord added Help wanted 🙏 Outside help would be appreciated, good for new contributors pyreverse Related to pyreverse component Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation pyreverse Related to pyreverse component
Projects
None yet
Development

No branches or pull requests

2 participants