-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix 'function never returning' computation #9591
base: main
Are you sure you want to change the base?
Conversation
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit eb30f7c |
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.
Hey thank you for taking over @antoniogamizbadger ! I feel like we lack some functional tests. (Shouldn't there be a Never
used somewhere ?) That could be taken from the example in the original issue directly. Also the tests are not passing in the CI right now. When a message is expected in functional tests it's indicated with # [message-name]
styile comment.
@antoniogamizbadger Would you like to continue working on this? |
return ( | ||
isinstance(node, (nodes.FunctionDef, astroid.BoundMethod)) | ||
and node.returns | ||
and ( | ||
isinstance(node.returns, nodes.Attribute) | ||
and node.returns.attrname == "NoReturn" | ||
and node.returns.attrname in {"NoReturn", "Never"} | ||
or isinstance(node.returns, nodes.Name) | ||
and node.returns.name == "NoReturn" | ||
and node.returns.name in {"NoReturn", "Never"} | ||
) | ||
try: | ||
return node.qname() in self._never_returning_functions | ||
except (TypeError, AttributeError): | ||
return False | ||
) |
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.
In #9714 the strategy is to start relying on utils.is_terminating_func()
instead of duplicating this logic in several places. Suggest exploring that here.
This fixes 2 bugs in the computation:
Never is handled in addition to NoReturn.
Give priority to the explicit --never-returning-functions option.
This PR was copied from #9535 (I cannot add commits there and we really need this).
Type of Changes
Description
Closes #7565