fix Pyrefly Fails to Report Non-Exhaustive match Statements and Incorrect Enum Case Patterns #400

[asukaminato0721]

Summary

Fixes #400

now records per-case metadata (literal/class patterns, guard info, coverage range) and emits a Binding::MatchStmt, enabling later passes to reason about matches.

defines the new MatchStmtInfo/MatchCaseInfo structures, adds the MatchStmt binding variant, and wires it into symbol_kind/display logic so the solver can consume the recorded metadata.

adds check_match_stmt plus helpers that a) warn when a match over a single Enum lacks coverage for some members (NonExhaustiveMatch) and b) flag class patterns that are actually Enum members (InvalidEnumPattern). Enum domain analysis handles unions of literals, skips flags, and only triggers when the user is enumerating specific members.

introduces invalid-enum-pattern (error) and non-exhaustive-match (warn) error kinds with default severities.

documents both new diagnostics with examples so the public docs stay in sync.

Test Plan

adds integration tests covering both new diagnostics (Color.RED() misuse and missing Enum cases) to keep behaviour locked down.

[Copilot]

Pull request overview

This PR fixes issue #400 by adding two new diagnostics for pattern matching on enums: invalid-enum-pattern (error) and non-exhaustive-match (warning). The implementation collects metadata about match statements during the binding phase, enabling the solver to perform exhaustiveness analysis on enum matches and detect misuse of enum members in class patterns.

Key changes:

• Adds metadata collection for match statements, recording pattern types, guards, and coverage information
• Implements exhaustiveness checking that warns when enum cases are missing from a match statement
• Detects when enum members are incorrectly used as class patterns (e.g., case Color.RED(): instead of case Color.RED:)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
website/docs/error-kinds.mdx	Documents the two new error kinds with code examples
pyrefly/lib/test/pattern_match.rs	Adds integration tests for both new diagnostics
pyrefly/lib/binding/pattern.rs	Collects match pattern metadata during binding phase
pyrefly/lib/binding/binding.rs	Defines MatchStmtInfo, MatchCaseInfo, and MatchClassPatternInfo structures
pyrefly/lib/alt/solve.rs	Implements enum exhaustiveness checking and invalid pattern detection
crates/pyrefly_config/src/error_kind.rs	Defines InvalidEnumPattern and NonExhaustiveMatch error kinds with appropriate severities

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yangdanny97]

Thanks for the PR, I'll give this a more detailed review when I have time. Just a quick comment on architecture for context:

Match exhaustiveness affects control flow modeling in the bindings layer, but it can't be modeled precisely because it depends on resolved types, which are not available until the solving layer in our exports -> binding -> solving architecture.

So things like knowing when a match w/o _ cases is exhaustive & using it to determine reachability of subsequent statements, for example, would be very hard to do under the current architecture.

Overall, I think this PR is a good start in terms of reporting potential bugs/errors to users, but there will be some remaining gaps around control flow that we can address later, which may require architecture changes.

[grievejia]

I get the impression that the PR only deals with non-exhaustive enum matching, not anything else. And it doesn't quite deal with any type-induced consequences with control flow.

[meta-codesync]

@grievejia has imported this pull request. If you are a Meta employee, you can view this in D89431015.

[yangdanny97]

Hi, thanks for your patience, I just had a look at this in detail and had some concerns about the approach.

My main issue is that this is a lot of code to do a very specific feature, which cannot be generalized to other types very well in the future.

An alternative approach for you to consider: is that when a match statement isn't obviously exhaustive by syntax (I think we have a helper for this somewhere), we generate a new expect binding (similar to what you currently do, but the contents + solving are different) which knows the following things:

1. the subject of the match statement
2. the negated previous narrows of (1) after the last case

When we solve that binding, if (1) turns out to be an enum && (2) is not Never, we give a warning. Compared to the current impl, we don't need to store information about each match case and try to statically evaluate everything.

Some examples:

class MyEnum(Enum):
   A = 1
   B = 2
  
 e: MyEnum = ...
 s: str = ...
 
 # do not generate the new binding because we know the match is exhaustive
 match e: 
    case MyEnum.A:
       pass
    case _:
       pass
       
# generate the new binding: the subject `e` is type `MyEnum`, and the narrowed type of `e` after the last case is `MyEnum.B`, so we error
match e:  
    case MyEnum.A:
       pass
 
# generate the binding: the subject `e` is type `MyEnum`, and the narrowed type of `e` after the last case is `Never` (equivalent to empty union), so we do not error  
 match e:  
    case MyEnum.A:
       pass
    case MyEnum.B:
       pass
       
 # generate the binding: the subject `e` is type `str`, which is not an enum so we skip the check and do not error  
 match s:  
    case "A":
       pass
    case "B":
       pass

Then, the pattern is generalizable in the future to other types. Although our type algebra isn't good enough to handle a general case, I believe we could also apply this to things like unions that only contain literal types today without issues.

Let me know if that makes sense, or if you disagree. Thanks!

[yangdanny97]

Additionally:

flag class patterns that are actually Enum members (InvalidEnumPattern)

I think this can be done in a separate PR. My preferred implementation would be a new binding that gets generated for each class-pattern case which does the checking + solving for the class type, and updating the PatternMatchClassPositional + PatternMatchClassKeyword bindings to use the result of that pattern instead of storing the expression and solving it ad-hoc each time.

Enum domain analysis handles unions of literals, skips flags, and only triggers when the user is enumerating specific members.

These all good properties, I think. The only thing is that I'm not sure the "only triggers when the user is enumerating specific members" is necessary - I think our type narrowing behavior should work pretty well for enums and literal unions, so we may not need to be so strict here.

[yangdanny97]

Lit implements display, you can do lit_enum @ Lit::Enum(_) and then format!("{}", lit_enum) and it will do the same thing

[yangdanny97]

There's a bug somewhere here. if the subject is NarrowingSubject::Facets I don't think it works, since we don't preserve the facet information into the solving step.

For example, this:

from enum import Enum

class Color(Enum):
    RED = "red"
    BLUE = "blue"

class X:
    color: Color

def describe(x: X) -> str:
    match x.color:
        case Color.RED:
            return "danger"
    return "cool"

If we add a case for Color.BLUE the error still gets shown

[yangdanny97]

I'll see if it's easy to patch internally, no action needed from your for now

[rchen152]

Review automatically exported from Phabricator review in Meta.

[meta-codesync]

@yangdanny97 merged this pull request in 2d2a146.

[yangdanny97]

filed #2153 as followup