Skip to content

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

matusvalo
Copy link

@matusvalo matusvalo commented Jul 22, 2025

Type of Changes

Type
βœ“ πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
πŸ“œ Docs

Description

This PR allows astroid to understand that except * produces ExceptionGroup 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

Copy link

codecov bot commented Jul 22, 2025

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 93.26%. Comparing base (9256b31) to head (0f362f5).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2786   +/-   ##
=======================================
  Coverage   93.26%   93.26%           
=======================================
  Files          91       91           
  Lines       11010    11017    +7     
=======================================
+ Hits        10268    10275    +7     
  Misses        742      742           
Flag Coverage Ξ”
linux 93.12% <100.00%> (+<0.01%) ⬆️
pypy 93.26% <100.00%> (+<0.01%) ⬆️
windows 93.24% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Ξ”
astroid/protocols.py 90.04% <100.00%> (+0.18%) ⬆️

... and 1 file with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


if isinstance(self.parent, node_classes.TryStar):
# except * handler has assigned ExceptionGroup
eg = nodes.ClassDef(
Copy link
Collaborator

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?

Copy link
Author

Choose a reason for hiding this comment

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

I will try that.

Copy link
Author

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...

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a 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.

@matusvalo
Copy link
Author

matusvalo commented Jul 27, 2025

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 next() (RUF015). On the other hand pylint does not like next() version:

        eg = next(
            node_classes.unpack_infer(
                extract_node(
                    """
from builtins import ExceptionGroup
ExceptionGroup
"""
                )
            )
        )

without handled StopIteration exception (stop-iteration-return).

If I understand code correctly, it is guaranteed that at least one item is returned by unpack_infer() hence I propose to use next() version of the code and disable stop-iteration-return. Let me know if you agree.

EDIT: Or we can add assertion:

try:
    ...
except StopIteration:
   assert False, "Should not happen"

@Pierre-Sassoulas
Copy link
Member

Let's noqa pylint

@Pierre-Sassoulas Pierre-Sassoulas added this to the 4.0.0 milestone Aug 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants