-
-
Notifications
You must be signed in to change notification settings - Fork 300
Fix false positive no-member in except * handler #2786
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
Conversation
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2786 +/- ##
=======================================
Coverage 93.30% 93.30%
=======================================
Files 91 91
Lines 11083 11091 +8
=======================================
+ Hits 10341 10349 +8
Misses 742 742
Flags with carried forward coverage won't be shown. Click here to find out more.
π New features to boost your workflow:
|
astroid/protocols.py
Outdated
|
||
if isinstance(self.parent, node_classes.TryStar): | ||
# except * handler has assigned ExceptionGroup | ||
eg = nodes.ClassDef( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can just get this from builtins
and infer it from there? Why do we need to create the ClassDef
ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK ExceptionGroup
constructed and infered from builtins
module...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the merge request. Are pylint and Ruff disagreeing about next ? It seems making pylint happy makes Ruff sad. Maybe there's another way to make pylint happy, I'm on mobile so I was not able to check the pipeline for the commit where pylint is sad.
The issue is following code: eg = list(
node_classes.unpack_infer(
extract_node(
"""
from builtins import ExceptionGroup
ExceptionGroup
"""
)
)
)[0] ruff does not like list with zero index, it prefers eg = next(
node_classes.unpack_infer(
extract_node(
"""
from builtins import ExceptionGroup
ExceptionGroup
"""
)
)
) without handled If I understand code correctly, it is guaranteed that at least one item is returned by EDIT: Or we can add assertion: try:
...
except StopIteration:
assert False, "Should not happen" |
Let's noqa pylint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests and expected results LGTM ! Let's wait for another review before merging.
# pylint: disable-next=stop-iteration-return | ||
eg = next( | ||
node_classes.unpack_infer( | ||
extract_node( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's clever but I'm not sure it's the right approach (maybe there's something more efficient ?). Maybe another maintainer will have ideas about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used it since it seemed to be standard way how to do it - e.g. here: https://github.com/pylint-dev/astroid/blob/8e19277598d5764f5afd821d7bb6104b701ac88f/astroid/brain/brain_collections.py#L120C32-L120C55
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pierre-Sassoulas Shall we merge this?
Sure, I was simply waiting for another opinion :) |
Thnx! I put autocomplete but I expected that it will be merged when both reviewers will approve π |
It's okay and I had added a thumbsups on your answer too |
Type of Changes
Description
This PR allows astroid to understand that
except *
producesExceptionGroup
instead of specified exception.Note
This PR is mainly interested in
ExceptionGroup.exceptions
attribute. Other attributes/methods are left for future improvements.Refs pylint-dev/pylint#9056