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

Deduction of a possible None return although NoReturn seems to be the only way out #967

Closed
PatrickBourdon opened this issue Feb 19, 2021 · 2 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@PatrickBourdon
Copy link

Environment data

  • Windows-10-10.0.18362-SP0
  • python: 3.9.1 (tags/v3.9.1:1e5d33e, Dec 7 2020, 17:08:21) [MSC v.1927 64 bit (AMD64)]
  • Visual Code Studio: 1.53.2
  • extension pylance: v2021.2.3
  • type checking mode: strict

Expected behaviour

no typing problem reported.

Actual behaviour

I isolated the issue I encountered in the short following example.
It includes 4 use cases progressively more complex: fa1, fa2, fa3, fa4.
Only the last fa4 fails with the following message: Function with declared type of "NoReturn" cannot return "None"
The issue seems closely related to NoReturn since there is no reported problem when the example is a bit modified and all functions return int.

Code Snippet / Additional information

from typing import NoReturn, Callable, Union

def f() -> NoReturn:
    raise TypeError

class B(object):
    def fb(self) -> NoReturn:
        f()

class C(object):
    def fb(self) -> NoReturn:
        f()

class A(object):
    def __init__(self):
        self._f: Callable[[], NoReturn] = f
        self._B: B = B()
        self._B_or_C: Union[B, C] = B()

    def fa1(self) -> NoReturn:
        f()

    def fa2(self) -> NoReturn: 
        self._f()

    def fa3(self) -> NoReturn: 
        self._B.fb()

    def fa4(self) -> NoReturn:
        self._B_or_C.fb()
@erictraut
Copy link
Contributor

Thanks for the bug report. This will be fixed in the next release.

In case you're interested in the details... The code flow engine in pylance/pyright needs to determine whether each call expression is invoking a call that has a NoReturn type. This affects the code flow graph because code after such a call is unreachable. Determining whether self._B_or_C.fb() is a NoReturn call requires the code flow engine to evaluate the type of self. To do this, it needs to perform code flow analysis on self using the code flow graph. This presents a difficult chicken-and-egg problem because the code flow graph cannot be fully built without first understanding which call expressions are invoking NoReturn calls. To break this cycle, the type analyzer has a bunch of special-case code that handles only common expression patterns used in call expressions. This code was not smart enough to handle the union that you used in the declaration for _B_or_C. I think it's reasonable for this code to handle unions, so my fix is to add that additional logic. Probably more details than you cared to know. :)

Here is the PR. I used a portion of your sample in the unit test case.

This will be fixed in the next release of pylance.

@erictraut erictraut added bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed triage labels Feb 19, 2021
@jakebailey
Copy link
Member

This issue has been fixed in version 2021.2.4, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202124-24-february-2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

3 participants