mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Add `[disallow-non-exhaustive-match]` check

Open johnthagen opened this issue 3 years ago • 9 comments

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.

johnthagen avatar Sep 03 '22 11:09 johnthagen

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.

emmatyping avatar Sep 04 '22 04:09 emmatyping

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 https://github.com/python/mypy/pull/12267 if you're interested in implementing this.

For what it's worth:

  • pyright chose not to enforce exhaustiveness by default, but has an opt-in strict check for it: https://github.com/microsoft/pyright/issues/2569#issuecomment-969936687
  • Guido was also hesitant about enforcing exhaustiveness by default, but mentioned he could see value in having this per-module (and mypy master now supports per-module error codes): https://github.com/python/mypy/pull/10191#issuecomment-1016608286

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 avatar Sep 04 '22 05:09 hauntsaninja

(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 )

hauntsaninja avatar Sep 04 '22 06:09 hauntsaninja

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.

johnthagen avatar Sep 04 '22 10:09 johnthagen

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.

xmo-odoo avatar Oct 16 '23 09:10 xmo-odoo

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',
}

bskinn avatar Dec 23 '23 18:12 bskinn

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:
        ^

9999years avatar Mar 26 '24 19:03 9999years

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.

Swift doesn't seem to be on this list. In Swift you'll always need a default: block (identical to case _: in Python) if you're not covering all the cases. What's not enabled by default is the enforcement of @unknown default:, i.e. always requiring a fallback for non-frozen enums in case a new case is added afterwards.

stevapple avatar Jun 19 '24 02:06 stevapple

Swift doesn't seem to be on this list. In Swift you'll always need a default: block (identical to case _: in Python) if you're not covering all the cases.

... did you misread my comment? That's what exhaustive matching is, you must cover all cases (whether individually or via a fallback clause -- default: in swift). Which is exactly what's requested of mypy.

xmo-odoo avatar Jun 19 '24 06:06 xmo-odoo