Add future=True to all NodeNG.frame calls
#5621
Conversation
DanielNoord
added
the
Maintenance
|
Not sure if this should be the PR to update the |
Pull Request Test Coverage Report for Build 1663088065
💛 - Coveralls |
| try: | ||
| frame_name = node.frame().name | ||
| frame_name = node.frame(future=True).name | ||
| except AttributeError: | ||
| return False |
There was a problem hiding this comment.
Should we catch ParentMissingError here now too? Usually it should never happen unless we or a plugin screws up. In that case we would need to catch it for every other call as well which is just unnecessary.
There was a problem hiding this comment.
We could also just remove it?
We don't cover it right now: https://coveralls.io/builds/45308622/source?filename=pylint%2Fcheckers%2Fclasses%2Fclass_checker.py
The commit that added it also doesn't talk about a plugin doing this wrong or anything: 5309e1d
The code from the original issue did raise an AttributeError but the fact that we no longer cover it and those tests had been added means (I think) that we handle those cases better in an earlier function and we never reach this.
There was a problem hiding this comment.
Yeah, let's remove it! If a plugin screws up, it's better to surface the error than to ignore it silently.
|
@Pierre-Sassoulas I don't think this was completely ready to merge. #5621 (comment) wasn't resolved yet 😉 |
|
My bad I got overexcited with all those approved MR :D maybe we don't need to revert everything and we can do a follow-up instead ? |
|
Yeah no problem. Do we want to remove the |
|
See #5664 in case we want to go ahead with this! |
Type of Changes
Description
Follow up to changes in
astroid2.9.1.