Skip to content

Add flag to raise error if match statement does not match exaustively #19144

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Don-Burns
Copy link

Fixes #19136

Change is to add a mode to catch when a match statement is not handling all cases exhaustively, similar to what pyright does by default.
After discussion on #19136 I put it behind a new flag that is not enabled by default.
I updated docs to include information on the new flag also.

Please let me know if anything is not following standards, in particular I wasn't sure what to name this new flag to be descriptive while following existing flag naming style.

@A5rocks
Copy link
Collaborator

A5rocks commented May 23, 2025

  1. could you enable it by default for mypy primer's output
  2. IMO it would be neater to put this behind a new error code instead. Though those are even less discoverable than new modes so...

This comment has been minimized.

Add flag to primer as asked in PR
@Don-Burns
Copy link
Author

  1. could you enable it by default for mypy primer's output
  2. IMO it would be neater to put this behind a new error code instead. Though those are even less discoverable than new modes so...

I added where it looked like made sense to pass the arg to mypy_primer (workflow files?), let me know if I misunderstood where to pass this.

I'll have a look at the error code option when I have a chance and see how it feels. Should still be possible to enable discovery, but right now at least I think CLI mention in --help would be ideal so users don't need the docs unless they are really deep diving. Not sure if there is some middle ground in this regard.

@A5rocks
Copy link
Collaborator

A5rocks commented May 23, 2025

Sorry I meant specifically just default to enabled for a commit and let mypy primer run.

Anyways, this should already work for an error code, I just don't know how to disable them by default. (The fact it would be a smaller change is why I said it would be neater)

This comment has been minimized.

Don-Burns added 4 commits May 24, 2025 08:39
There appears to be a parsing issue in sphyx? For the lines with `match val: # error: ...` the `: # error:` part is being interpreted as a directive I think? Means the # is stripped from the python code and the docs can't build

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Don-Burns
Copy link
Author

Sorry I meant specifically just default to enabled for a commit and let mypy primer run.

CI run with default set to True
commit: 0f9ed6a
mypy_primer comment: #19144 (comment)

This comment has been minimized.

@Don-Burns Don-Burns marked this pull request as ready for review May 24, 2025 15:16
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@Don-Burns
Copy link
Author

Test passing now.

My preference is for the CLI arg for discoverability in cli help etc.
But if using an error code only to enable via config is preferred, this is what that would look like from what I can tell: https://github.com/python/mypy/compare/master...Don-Burns:mypy:feat/exhaustive-match-error-code-approach?expand=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mypy not raising error on inexhaustive case/match statements in strict mode
2 participants