`assert_never` with `Union` type and `match ... case` triggers `reportUnnecessaryComparison`

[mark-henry01]

Hi, here is a minimal example:

from dataclasses import dataclass
from typing import assert_never

@dataclass
class Foo:
    p1: str

@dataclass
class Bar:
    p2: int

type FooOrBar = Foo | Bar

def do_something(f: FooOrBar):
    match f:
        case Foo():
            print("Foo")
        case Bar():
            print("Bar")
        case _ as unreachable:  # <-- error message here
            assert_never(unreachable)

Error:

Pattern will never be matched for subject type "Never" [reportUnnecessaryComparison]

There already was a discussion in https://github.com/microsoft/pyright/issues/4706. Till now there seems no further incentive to fix this bug in pyright itself. You need to append # pyright: ignore [reportUnnecessaryComparison] at each location as workaround.

Original intention is to provide runtime checks via assert_never in addition to exhaustiveness type checking, even if not strictly needed in this contrived example. Say you have an additional @dataclass union constituent Baz, but have forgotten to declare it within FooOrBar.

Related: https://github.com/DetachHead/basedpyright/pull/450/commits/285d807dbebd4df9a525b245853bb63c87a57472

[mark-henry01]

Thank you much @DetachHead for having taken time to fix this issue. At least with v1.29.1 (pyright 1.1.400), if I use above example, the code location is still (or again, not sure of previous versions) hinted as reportUnnecessaryComparison. https://github.com/DetachHead/basedpyright/issues/1213 might be related. Would appreciate, if you could comment on the status quo.

[DetachHead]

looks like i only fixed it when accessing the match subject variable rather than the variable defined on the individual case statement.

Code sample in basedpyright playground

from dataclasses import dataclass
from typing import assert_never

@dataclass
class Foo:
    p1: str

@dataclass
class Bar:
    p2: int

type FooOrBar = Foo | Bar

def do_something(f: FooOrBar):
    match f:
        case Foo():
            print("Foo")
        case Bar():
            print("Bar")
        case _: # no error
            assert_never(f)
    match f:
        case Foo():
            print("Foo")
        case Bar():
            print("Bar")
        case _ as unreachable: # error
            assert_never(unreachable)
    match f:
        case Foo():
            print("Foo")
        case Bar():
            print("Bar")
        case unreachable: # error
            assert_never(unreachable)

ideally none of these should report an error

[DetachHead]

a workaround is to move the case _ and use assert_type instead:

Code sample in basedpyright playground

from dataclasses import dataclass
from typing import assert_type

@dataclass
class Foo:
    p1: str

@dataclass
class Bar:
    p2: int

type FooOrBar = Foo | Bar

def do_something(f: FooOrBar):
    match f:
        case Foo():
            print("Foo")
        case _:
            assert_type(f, Bar)
            print("Bar")

i feel like this is a cleaner solution anyway, because it avoids adding an unnecessary branch

[flying-sheep]

The “unnecessary” branch is the actual use case for this, i.e. to run defensive code in it like raise ValueError(f"Unknown f is of type {type(f)} and not Foo|Bar as expected")

Not to mention that of course the way to write a distinction between Foo and Bar is of course case Foo() and case Bar(), might as well get rid of match statements again if we’re not supposed to do that.

[mark-henry01]

Sorry for replying late @DetachHead.

I think the solution with assert_type is not quite optimal, as it does no run-time checks.

In my view the main essence for default case _ in combination with assert_never is a last run-time safety belt for non-expected values. For example think about the possibility to later add more sum types to FooOrBar, but still enforce handling all patterns in every case (incl. run-time):

@dataclass
class Baz:
    p2: bool


type FooOrBar = Foo | Bar | Baz

This run-time check is equally important in an environment, where there (currently) is no IDE with a static type checker like Basedpyright available - to fail fast and not cause application state inconsistencies.

Otherwise if expecting a static type checker at all times, we probably would not need case _/assert_never at all, as Pyright/BasedPyright is smart enough to hint an error at f in match f for non-exhaustive matches:

Diagnostics:
1. Cases within match statement do not exhaustively handle all values
     Unhandled type: "Baz"
     If exhaustive handling is not intended, add "case _: pass" [reportMatchNotExhaustive]

So as an idea: Maybe disable reportUnnecessaryComparison for cases, where unreachable in case _ as unreachable: resolves to Never type? This would allow to get rid of # pyright: ignore [reportUnnecessaryComparison] in all exhaustive pattern matching statements.