Skip to content

Some __str__ methods of nodes raise errors and warnings #1881

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

Closed
jacobtylerwalls opened this issue Nov 20, 2022 · 6 comments · Fixed by #2198
Closed

Some __str__ methods of nodes raise errors and warnings #1881

jacobtylerwalls opened this issue Nov 20, 2022 · 6 comments · Fixed by #2198
Assignees
Labels
Bug 🪳 Good first issue Friendly and approachable by new contributors
Milestone

Comments

@jacobtylerwalls
Copy link
Member

Steps to reproduce

>>> fd = astroid.nodes.FunctionDef()
>>> str(fd)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jacobwalls/astroid/astroid/nodes/node_ng.py", line 219, in __str__
    value = getattr(self, field)
            ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'FunctionDef' object has no attribute 'args'

Less severely, but still not great, launching python with warnings e.g. python -Wall:

>>> cd = astroid.nodes.ClassDef()
>>> str(cd)
/Users/.../astroid/astroid/nodes/scoped_nodes/scoped_nodes.py:2041: DeprecationWarning: The 'ClassDef.doc' attribute is deprecated, use 'ClassDef.doc_node' instead.

We should add a unittest that generates the str() and repr() of all node types.

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

2.12.13

@jacobtylerwalls jacobtylerwalls added Bug 🪳 Good first issue Friendly and approachable by new contributors labels Nov 20, 2022
@DanielNoord
Copy link
Collaborator

The first issue is caused by the fact that the str relies on postinit being called I think.

I ran into this while debugging astroid many times..

@jacobtylerwalls jacobtylerwalls added this to the 3.0.0a3 milestone May 13, 2023
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0a3, 3.0.0a4 May 14, 2023
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0a4, 3.0.0a5 Jun 6, 2023
crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 6, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. This commit adds unittests to find and reproduce this
behavior with the endgoal of fixing it.

Related to pylint-dev#1881
@crazybolillo
Copy link
Contributor

I made a quick unit test on #2198. Am I going in the right direction? Also, I guess fixing the errors themselves will be part of this issue right?

@DanielNoord
Copy link
Collaborator

I'd prefer using pytest semantics instead of unittests but that's more a review comment.

I think the bigger issue is that the reliance on __postinit__ makes this very hard to solve. We would need to revisit either 1) what we put in the __str__ or 2) how we instantiate nodes. Both are not optimal I think.

@jacobtylerwalls
Copy link
Member Author

I labeled this as a good first issue because I was hoping it wasn't requiring knowledge of any of that, just placing a good blank placeholder in the str/repr when the info is not available.

@DanielNoord
Copy link
Collaborator

That would require a fix here:

value = getattr(self, field)

I guess that would work? We don't really need a regression test for it though (I think).

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented Jun 6, 2023

LGTM. There are bunch of overridden __str__ methods that might have similar problems, so I was thinking the unit tests would be nice. But if we find no other problems in the other methods, then such a test could be overly paranoid. EDIT: but writing the test would be the fastest way to find out!

crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 7, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes  pylint-dev#1881
crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 7, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes  pylint-dev#1881
crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 8, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes  pylint-dev#1881
crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 8, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes  pylint-dev#1881
crazybolillo added a commit to crazybolillo/astroid that referenced this issue Jun 12, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes  pylint-dev#1881
jacobtylerwalls pushed a commit that referenced this issue Jun 12, 2023
Calling str() or repr() on certain nodes fails either with errors
or warnings. A unittest was added to verify this behaviour and find
the offending nodes. Code has been corrected, mainly by
accesing node's attributes safely and using placeholders if necessary.

Closes #1881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪳 Good first issue Friendly and approachable by new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants