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

Exhaustiveness checks for patten matching #2569

Closed
hcarty opened this issue Nov 15, 2021 · 18 comments
Closed

Exhaustiveness checks for patten matching #2569

hcarty opened this issue Nov 15, 2021 · 18 comments
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request

Comments

@hcarty
Copy link

hcarty commented Nov 15, 2021

Is your feature request related to a problem? Please describe.

I would like to be able to be able to enable an error when a match does not cover all cases.

Describe the solution you'd like

I would like an option to generate an error when a match statement is not exhaustive. For example, this match statement would error with such an option enabled because the int case is not handled.

IntOrStr = int | str

def handle(x : IntOrStr) -> None:
    match x:
        case str(s):
            print(f"Handled {s}")
@hcarty hcarty added the enhancement request New feature or request label Nov 15, 2021
@erictraut
Copy link
Collaborator

erictraut commented Nov 16, 2021

Structural pattern matching has many use cases that don't involve exhaustive checks, so I don't think it makes sense to assume that every use of a match statement will cover all cases.

The same is true for if/elif/else chains. In that case, the recommended approach when you want to validate exhaustive checks is to use an assert_never function like this:

from typing import NoReturn

def assert_never(x: NoReturn) -> NoReturn:
    assert False, "Unhandled type: {}".format(type(x).__name__)

IntOrStr = int | str

def handle(x: IntOrStr) -> None:
    if isinstance(x, str):
        print(f"Handled {x}")
    else:
        assert_never(x)  # Error: "int" is incompatible with "NoReturn"

This works because NoReturn is effectively the same as an empty union (or a Never type). By convention, it is treated by Python type checkers as being compatible with only itself. If you attempt to pass any other type to it, the type checker will emit an error.

If you want a static type checker enforce exhaustiveness within a match statement, I recommend using the same approach:

def handle_not_exhaustive(x : IntOrStr) -> None:
    match x:
        case str(s):
            print(f"Handled {s}")

        case _:
            assert_never(x)  # Error: "int" is incompatible with "NoReturn"

def handle_exhaustive(x: IntOrStr) -> None:
    match x:
        case str(s):
            print(f"Handled {s}")

        case int(s):
            print(f"Handled {s}")

        case _:
            assert_never(x)  # No error

@hcarty
Copy link
Author

hcarty commented Nov 17, 2021

If you want a static type checker enforce exhaustiveness within a match statement, I recommend using the same approach

That's fair. The issue I have with that approach is that it requires users to remember to add a catch-all case to every match. Stylistically I'd prefer to maintain code where non-exhaustive matches are made explicit and exhaustiveness is expected by default. That way, for example, I can't forget to update a match when the matched value evolves to include new cases.

@erictraut
Copy link
Collaborator

Yeah, as I said, there are many legitimate uses for match statements that don't involve exhaustiveness, so there needs to be some way for a developer to say "I want to enforce exhaustiveness in this case". This seems like a straightforward way to do so.

@hcarty
Copy link
Author

hcarty commented Nov 18, 2021

Where is match useful in place of if in that case? If there is no way to have implicit exhaustiveness checks for match statements then I'm curious to know where they help versus using if with the same pattern.

@erictraut
Copy link
Collaborator

Match statements provide a different (more concise) syntax that some developers prefer over if/elif/else statements. You can generally use them interchangeably and for the same purposes.

@hcarty
Copy link
Author

hcarty commented Nov 18, 2021

Thanks - and thank you generally for your prompt responses to issues and feature requests on here. It's very appreciated!

@rouge8
Copy link

rouge8 commented Dec 8, 2021

PEP 622 seems to suggest that type checkers should enforce exhaustiveness checks with pattern matching: https://www.python.org/dev/peps/pep-0622/#exhaustiveness-checks. Are there no plans for pyright to support this beyond the assert_never() pattern?

@erictraut
Copy link
Collaborator

Pyright does provide the type narrowing support required for enforcing exhaustiveness checks, but you need to specify whether exhaustiveness is what you intend. There are many legitimate uses of match that do not involve exhaustive pattern matching, so it wouldn't be appropriate to assume that exhaustive matching is intended in every case.

There are a few ways to specify that exhaustive matching is intended. The first example in PEP 622 that you linked to is already covered by the return type check in pyright.

Screen Shot 2021-12-07 at 9 31 51 PM

In other cases (such as in the second example), you can use the assert_never() pattern.

Screen Shot 2021-12-07 at 9 34 25 PM

@TomasPuverle
Copy link

TomasPuverle commented Jan 23, 2022

I made a counter-argument in the same thread: python/mypy#12010 (comment)

(Btw, I love PyRight - thank you! ❤️ )

@erictraut erictraut reopened this Jan 23, 2022
@erictraut
Copy link
Collaborator

There seems to be sufficient interest in enforcing exhaustive matching, so I've added a new diagnostic check ("reportMatchNotExhaustive") that emits a diagnostic in cases where exhaustion cannot be proven through static analysis. The check is off by default with "basic" type checking mode but on by default with "strict" typing checking.

This will be included in the next release of pyright.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Jan 23, 2022
@hcarty
Copy link
Author

hcarty commented Jan 23, 2022

Thank you @erictraut!

@TomasPuverle
Copy link

Thank you!

@erictraut
Copy link
Collaborator

This is included in pyright 1.1.215, which I just published. It will also be included in the next release of pylance.

@TomasPuverle
Copy link

Hi @erictraut , I wanted to follow up and thank you for implementing this. I use the 3.10 match statements on a daily basis and the exhaustiveness support has been a real pleasure to work with.

@dogweather
Copy link

reportMatchNotExhaustive is incredible, thank you!

@Hubro
Copy link

Hubro commented Aug 3, 2022

Is it possible to enable this check for individual match statements?

If not, would it be useful to add? Something like:

# pyright: exhaustive-match
match foo:
    ...

For those that don't want to enable this check globally, this seems much nicer than using assert_never(foo), since you have to define that function yourself and import it when needed.

@johnthagen
Copy link

johnthagen commented Sep 4, 2022

For those that don't want to enable this check globally, this seems much nicer than using assert_never(foo), since you have to define that function yourself and import it when needed.

FYI, assert_never() is included in Python 3.11 and also in recent typing-extension package releases. So in those cases you can avoid having to define it yourself.

from typing_extensions import assert_never

dusty-phillips added a commit to dusty-phillips/match-variant that referenced this issue Sep 29, 2022
Noticed this while documenting another project; enums need the value to
be NoReturn to create type exhaustion. Variants still need to be
specialcased upstream for this to work, though.

Reference microsoft/pyright#2569
@dogweather
Copy link

I wrote up my positive experience with this check and made a repo with some demo cases to play with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants