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

Add [disallow-non-exhaustive-match] check #13597

Open
johnthagen opened this issue Sep 3, 2022 · 7 comments
Open

Add [disallow-non-exhaustive-match] check #13597

johnthagen opened this issue Sep 3, 2022 · 7 comments
Labels
feature topic-match-statement Python 3.10's match statement

Comments

@johnthagen
Copy link
Contributor

johnthagen commented Sep 3, 2022

Feature

Add a new check, [disallow-non-exhaustive-match], that enforces at type check time that no match statements implicitly fall through their bottom.

I would prefer if this check were on-by-default, or at least enabled when strict = true, but if this is controversial to the mypy team, off-by-default would be acceptable.

Pitch

The (superseded) PEP 622 has a great discussion on the motivation for this feature with several examples.

For brevity, below we list a very simple example demonstrating the kinds of correctness checks that motivate this feature request.

Consider we have the following code:

from enum import Enum, auto

class Color(Enum):
    Red = auto()
    Green = auto()

def print_color(color: Color) -> None:
    match color:
        case Color.Red:
            print("red")
        case Color.Green:
            print("green")

Now, consider in the future another Color variant is added:

from enum import Enum, auto

class Color(Enum):
    Red = auto()
    Green = auto()
    Blue = auto()

def print_color(color: Color) -> None:
    match color:
        case Color.Red:
            print("red")
        case Color.Green:
            print("green")

Oops! We forgot to add another case to print_color's match, Color.Blue is not handled, and this bug slips though into runtime.

Even if mypy is configured in strict mode, this bug isn't caught (because Python's default behavior is to allow match fall through).

[tool.mypy]
strict = true
$ mypy main.py
Success: no issues found in 1 source file

So how does a programmer defend against this bug? Currently, the best solution is to sprinkle assert_never clauses into every match.

from enum import Enum, auto

from typing_extensions import assert_never


class Color(Enum):
    Red = auto()
    Green = auto()
    Blue = auto()


def print_color(color: Color) -> None:
    match color:
        case Color.Red:
            print("red")
        case Color.Green:
            print("green")
        case _ as unreachable:
            assert_never(unreachable)

This will catch the error:

$ mypy main.py
main.py:19: error: Argument 1 to "assert_never" has incompatible type "Literal[Color.Blue]"; expected "NoReturn"

This boilerplate requires the programmer to:

  1. If running on Python <3.11, add typing_extensions to their dependencies
  2. For each module they are using match, import assert_never
  3. For each match statement, add the case _ clause

This hurts the ergonomics of trying to pervasively enforce this kind of correctness.

This also requires the programmer not to forget to add this boilerplate to every match clause, which can be error prone, especially for large projects wanting to guarantee correctness.

If using coverage, the boilerplate increases further as there will be no reasonable way to add tests to cover the extra unreachable cases. An extra # pragma: no cover comment must be added to every unreachable case:

def print_color(color: Color) -> None:
    match color:
        case Color.Red:
            print("red")
        case Color.Green:
            print("green")
        case _ as unreachable:  # pragma: no cover
            assert_never(unreachable)

Other Languages

Other languages with pattern matching, such as Rust, enforce exhaustive pattern matching, and it has been observed to have a positive effect on program correctness.

@ethanhs
Copy link
Collaborator

ethanhs commented Sep 4, 2022

I actually would prefer this to be on by default. The superseding PEP 634 says:

For cases such as sealed classes and enums, where the patterns are all known to be members of a discrete set, static checkers can warn about missing patterns.

In addition, there are already so many flags and I think this is a case where it is reasonable to diverge from runtime.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Sep 4, 2022

Note that mypy will already complain about code like the following, so this shouldn't be a hard check to add:

from enum import Enum, auto

class Color(Enum):
    Red = auto()
    Green = auto()
    Blue = auto()

def print_color(color: Color) -> int:
    match color:
        case Color.Red:
            return 4
        case Color.Green:
            return 5

Take a look at #12267 if you're interested in implementing this.

For what it's worth:

I would certainly accept a disabled-by-default error code for this to mypy. I don't have enough experience with the match statement or sense of how it's used in the ecosystem to have an opinion on enabled-by-default or not.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Sep 4, 2022

(Btw, a little off topic, but if you have open source code that makes interesting use of typing features, pattern matching, or has surfaced mypy usability issues for you previously, let me know, and I can add it to https://github.com/hauntsaninja/mypy_primer )

@johnthagen
Copy link
Contributor Author

I actually would prefer this to be on by default.

@ethanhs I would too. I clarified the initial post to express this. I originally posted off-by-default to try to avoid controversy if the mypy team feels we shouldn't diverge from the runtime behavior. But I agree with everything you said above.

@AlexWaygood AlexWaygood added the topic-match-statement Python 3.10's match statement label Sep 4, 2022
llucax added a commit to llucax/frequenz-channels-python that referenced this issue Jun 9, 2023
This function will replace the current `Select` implementation with the
following improvements:

* Proper type hinting by using the new helper type guard
  `selected_from()`.
* Fixes potential starvation issues.
* Simplifies the interface by providing values one-by-one.
* Simplifies the implementation, so it is easier to maintain.

There are some other improvements we would have liked to be able to make
but was difficult.

For example, typing for `select()` is tricky.  We had the idea of using
a declarative design, something like:

```python
class MySelector(Selector):
    receiver1: x.new_receiver()
    receiver2: y.new_receiver()

async for selected in MySelector:
    if selected.receiver is receiver1:
        # Do something with selected.value
    elif selected.receiver is receiver1:
        # Do something with selected.value
```

This is similar to `Enum`, but `Enum` has special support in `mypy` that
we can't have.

With the current implementation, the typing could be slightly improved
by using `TypeVarTuple`, but we are not because "transformations" are
not supported yet, see: python/typing#1216

Also support for `TypeVarTuple` in general is still experimental (and
very incomplete in `mypy`).

With this we would also probably be able to properly type `select` and
*maybe* even be able to leverage the exhaustiveness checking of `mypy`
to make sure the selected value is narrowed down to the correct type to
make sure all receivers are handled, with the help of `assert_never` as
described in:
https://docs.python.org/3.11/library/typing.html#typing.assert_never

We also explored the possibility of using `match` to perform
exhaustiveness checking, but we couldn't find a way to make it work
with `match`, and `match` is not yet checked for exhaustiveness by
`mypy` anyway, see: python/mypy#13597

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax added a commit to llucax/frequenz-channels-python that referenced this issue Jun 19, 2023
This function will replace the current `Select` implementation with the
following improvements:

* Proper type hinting by using the new helper type guard
  `selected_from()`.
* Fixes potential starvation issues.
* Simplifies the interface by providing values one-by-one.
* Simplifies the implementation, so it is easier to maintain.

There are some other improvements we would have liked to be able to make
but was difficult.

For example, typing for `select()` is tricky.  We had the idea of using
a declarative design, something like:

```python
class MySelector(Selector):
    receiver1: x.new_receiver()
    receiver2: y.new_receiver()

async for selected in MySelector:
    if selected.receiver is receiver1:
        # Do something with selected.value
    elif selected.receiver is receiver1:
        # Do something with selected.value
```

This is similar to `Enum`, but `Enum` has special support in `mypy` that
we can't have.

With the current implementation, the typing could be slightly improved
by using `TypeVarTuple`, but we are not because "transformations" are
not supported yet, see: python/typing#1216

Also support for `TypeVarTuple` in general is still experimental (and
very incomplete in `mypy`).

With this we would also probably be able to properly type `select` and
*maybe* even be able to leverage the exhaustiveness checking of `mypy`
to make sure the selected value is narrowed down to the correct type to
make sure all receivers are handled, with the help of `assert_never` as
described in:
https://docs.python.org/3.11/library/typing.html#typing.assert_never

We also explored the possibility of using `match` to perform
exhaustiveness checking, but we couldn't find a way to make it work
with `match`, and `match` is not yet checked for exhaustiveness by
`mypy` anyway, see: python/mypy#13597

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@xmo-odoo
Copy link

xmo-odoo commented Oct 16, 2023

I was also surprised that even in --strict mode mypy does not warn about this issue, my use case was processing a commands set so along the lines of

class A:
    pass

class B:
    pass

Things = Union[A, B]

def foo(v: Things) -> None:
    match v:
        case A():
            # do thing

and the entire point of the union was the assumption that match would perform exhaustiveness checking, and would tell me if I forgot to process new commands. Sadly it's not to be.

Other languages with pattern matching, such as Rust, enforce exhaustive pattern matching

OCaml and Swift as well, Haskell -- or at least ghc -- stands out for not having exhaustive checking by default, and IIRC it's because the type system is sufficiently complex that it's a hard problem in that specific language, as well as the relevant GHC code being pretty old and crufty and not having a real champion / owner.

@bskinn
Copy link

bskinn commented Dec 23, 2023

TypeScript also is equipped to evaluate its [key in <type>] construction exhaustively, I believe by default.

The following shows a Property '[E_Color.Blue]' is missing in type '{ red: string; green: string; }' but required in type '{ red: string; green: string; blue: string; }' error in one of my projects:

export enum E_Color {
  Red = 'red',
  Green = 'green',
  Blue = 'blue',
}

export const colorsUpper: {[key in E_Color]: string} = {  // <-- Error
  [E_Color.Red]: 'RED',
  [E_Color.Green]: 'GREEN',
}

@9999years
Copy link

9999years commented Mar 26, 2024

I was pretty shocked to discover that this behavior isn't implemented. This error message is also not great:

class Animal(enum.Enum):
    DOG = "Dog"
    CAT = "Cat"

    def __bool__(self) -> bool:
        match self:
            case Animal.DOG:
                return True
error: Missing return statement  [return]
        def __bool__(self) -> bool:
        ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-match-statement Python 3.10's match statement
Projects
None yet
Development

No branches or pull requests

7 participants