Skip to content
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

typing_extensions assert_never's Never not supported #7565

Closed
henryiii opened this issue Oct 3, 2022 · 6 comments · Fixed by #9591 or #9940
Closed

typing_extensions assert_never's Never not supported #7565

henryiii opened this issue Oct 3, 2022 · 6 comments · Fixed by #9591 or #9940
Labels
Enhancement ✨ Improvement to a component python 3.11
Milestone

Comments

@henryiii
Copy link

henryiii commented Oct 3, 2022

Bug description

Python 3.11 added Never as an alias of NoReturn. The signature of assert_never is assert_never(__x: Never) -> Never (personally, I wish had been assert_never(__x: Never) -> NoReturn, but that's what it is). This was backported to typing_extensions 4.1.0, as well.

I tried the following, but it seems typing_extensions.assert_never is uninferable.

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index ed1b7a938..21b9b2b07 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -1907,9 +1907,9 @@ class RefactoringChecker(checkers.BaseTokenChecker):
         if isinstance(node, nodes.FunctionDef) and node.returns:
             return (
                 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

If it's defined locally, however:

def assert_never(value: Never) -> Never:
    assert False, f"Unhandled value: {value} ({type(value).__name__})"

Then the above patch fixes it, so there's that, at least. Maybe that would at least fix it on Python 3.11? Maybe typing_extensions.assert_never could be added to the list of no return functions if it can't be inferred? However, when I stick an print statement in here, it seems the node is "Uninferrable", and adding "typing_extensions.assert_never" to refactoring.never-returning-functions does not work.

This is using cibuildwheel's source, see pypa/cibuildwheel#1291.

It looks like this was partially addressed in #7311, but not for all places NoReturn shows up.

Configuration

No response

Command used

pylint src

Pylint output

cibuildwheel/architecture.py:117:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

Expected behavior

Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Pylint version

pylint 2.15.3
astroid 2.12.10
Python 3.10.7 (main, Sep 15 2022, 01:51:29) [Clang 14.0.0 (clang-1400.0.29.102)]

OS / Environment

No response

Additional dependencies

typing_extensions==4.3.0

@henryiii henryiii added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Oct 3, 2022
@henryiii henryiii changed the title typing_extensions assert_never not supported typing_extensions assert_never's Never not supported Oct 3, 2022
@Pierre-Sassoulas Pierre-Sassoulas added python 3.11 Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 3, 2022
@svenpanne
Copy link

I'd like to point out that this is still a bug in pylint, which is quite annoying when you use assert_never relatively often. The patch above fixes the problem, so it would be nice to have it in the code.

In addition, handling self._never_returning_function after the first if seems wrong: If one explicitly specifies that a function never returns, this should override any inference by pylint, so this test should come first. At the current state, this configuration option is quite useless, one can't e.g. specify that assert_never never returns.

@Pierre-Sassoulas
Copy link
Member

Do you want to open a PR with the patch @svenpanne ?

If one explicitly specifies that a function never returns, this should override any inference by pylint

This is not the case right now, see #4813. Changing that will be quite a journey

At the current state, this configuration option is quite useless, one can't e.g. specify that assert_never never returns.

What configuration options are we talking about ?

@svenpanne
Copy link

Do you want to open a PR with the patch @svenpanne ?

It's basically the patch above, let's see when I find some time...

If one explicitly specifies that a function never returns, this should override any inference by pylint

This is not the case right now, see #4813. Changing that will be quite a journey

Actually pylint already figures out enough, verified by print()ing in the above method. It's just that pylint doesn't handle the Never type introduced in Python 3.11 last year, see e.g. https://typing.readthedocs.io/en/latest/source/unreachable.html. In addition to checking for NoReturn, it needs to check fo Never, too. This would already solve the problem for assert_never.

At the current state, this configuration option is quite useless, one can't e.g. specify that assert_never never returns.

What configuration options are we talking about ?

https://pylint.readthedocs.io/en/latest/user_guide/configuration/all-options.html#never-returning-functions My point is: If the user explicitly configures that some functions never return, this should override any logic in pylint, so the order of tests in the function above should be reversed. If this was the case, adding typing.assert_never to that configuration option would work even without the Never change above, but currently it doesn't.

So in effect, there are 2 bugs in the method.

@finite-state-machine
Copy link

finite-state-machine commented Jan 31, 2024

Repro for the second issue @svenpanne describes:

bug7565.py:

# pylint: disable=missing-function-docstring,missing-module-docstring
from __future__ import annotations

from typing_extensions import (
        assert_never,
        Union,
        )

def some_func(arg: Union[int, str]) ->  int:

    if isinstance(arg, int):
        return 1
    if isinstance(arg, str):
        return 2

    assert_never(arg)

pylint --rcfile=/dev/null --never-returning-functions=typing_extensions.assert_never bug7565.py:

************* Module bug7565
bug7565.py:9:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

-----------------------------------
Your code has been rated at 8.75/10

@antoniogamizbadger
Copy link
Contributor

Any update on this?

svenpanne added a commit to svenpanne/pylint that referenced this issue Apr 3, 2024
This fixes 2 bugs in the computation:
* `Never` is handled in addition to `NoReturn`.
* Give priority to the explicit `--never-returning-functions` option.

Closes pylint-dev#7565
@svenpanne
Copy link

OK, I've uploaded a PR for this, fixing both problems: Never was unknown and explicitly stating non-returning functions had the wrong priority compared to the incomplete inference of astroid/pylint.

With this change, things work out-of-the-box for Python 3.11 and 3.12 with both typing and typing_extensions. Older Python versions have to use the latter, which exposes its own implementation of assert_never then. Somehow astroid can't infer anything about that then, but this is a different problem. For a home-grown & visible version, things work, too, even for such old Pythons.

svenpanne added a commit to svenpanne/pylint that referenced this issue Apr 8, 2024
This fixes 2 bugs in the computation:
* `Never` is handled in addition to `NoReturn`.
* Give priority to the explicit `--never-returning-functions` option.

Closes pylint-dev#7565
svenpanne added a commit to svenpanne/pylint that referenced this issue Apr 12, 2024
This fixes 2 bugs in the computation:
* `Never` is handled in addition to `NoReturn`.
* Give priority to the explicit `--never-returning-functions` option.

Closes pylint-dev#7565
svenpanne added a commit to svenpanne/pylint that referenced this issue Apr 12, 2024
This fixes 2 bugs in the computation:
* `Never` is handled in addition to `NoReturn`.
* Give priority to the explicit `--never-returning-functions` option.

Closes pylint-dev#7565
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels May 3, 2024
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.2.0 milestone May 3, 2024
@jacobtylerwalls jacobtylerwalls modified the milestones: 3.2.0, 3.3.0 May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment