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

Type Union with NoReturn #2008

Closed
jfmend opened this issue Oct 29, 2021 · 5 comments
Closed

Type Union with NoReturn #2008

jfmend opened this issue Oct 29, 2021 · 5 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

@jfmend
Copy link

jfmend commented Oct 29, 2021

Environment data

  • Language Server version: 2021.10.3
  • OS and version: linux x64
  • Python version (and distribution if applicable, e.g. Anaconda):
  • python.analysis.indexing: undefined
  • python.analysis.typeCheckingMode: basic

Expected behaviour

No type errors reported

Actual behaviour

Error resulting from Union[A, B, ..., NoReturn] not being considered equivalent to Union[A, B, ...] which I believe it should.
To be clear, a [variable, function_arg, return_type] of type A should not accept an expression of type NoReturn (since this is either useless or a bug) in which case a type error is correctly reported. However, a scenario where a type A is expected should accept an expression of type Union[A, NoReturn].

Notice that, in Python, the evaluation of any expression may result in an Exception being raised (ex: MemoryError). This means that an expression of some type A is always implicitly of type Union[A, NoReturn].

Code Snippet / Additional information

from typing import NoReturn


def raises(exception: Exception) -> NoReturn:
    '''Helper function to allow raising an exception in an expression context'''
    raise exception


# Example 1 

n = 1
n_str: str
# incorrect type ERROR (assigning str | NoReturn to var of type str):
n_str = str(n) if n > 0 else raises(
        Exception(f'Must be positive: N = {n}'))
print(n_str)


# Example 2

import os
from pathlib import Path


def find_productFile(product_dir: Path) -> Path:
    dir_files = os.listdir(product_dir)
    # incorrect type ERROR (returning Path | NoReturn where Path is expected):
    return ( product_dir/'product.xml'   if 'product.xml' in dir_files
        else product_dir/'manifest.safe' if 'manifest.safe' in dir_files
        else raises(Exception(
                'Product not identified as one of [Sentinel-1, RadarSat2]')))
@erictraut
Copy link
Contributor

Thanks for reporting the issue. I've created a tracking bug in the pyright issue tracker.

@erictraut
Copy link
Contributor

We already elide NoReturn from unions in several other cases, but this logic was not included for ternary expressions. It's trivial to add this case. This will be included in the next release.

@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 Oct 29, 2021
@jfmend
Copy link
Author

jfmend commented Oct 30, 2021

Thank you for the quick reply.
I am not sure if this should be another issue or not, but type check errors with Unions containing NoReturn also occurs in or expressions:

from typing import NoReturn


def raises(exception: Exception) -> NoReturn:
    '''Helper function to allow raising an exception in an expression context'''
    raise exception


n = 1
n_str: str
# incorrect type ERROR (assigning str | NoReturn to var of type str):
n_str = str(n) or raises(RuntimeError(f'Must be positive: N = {n}'))
print(n_str)

@erictraut
Copy link
Contributor

I've added support for or and and operators as well.

@heejaechang
Copy link
Contributor

fixed in 2021.11.0

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