Add flag to raise error if match statement does not match exaustively #19144
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This comment has been minimized.
This comment has been minimized.
Add flag to primer as asked in PR
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 |
|
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.
This comment has been minimized.
This reverts commit 3341b4b.
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.
Requested in PR
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
CI run with default set to |
This reverts commit 0f9ed6a.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Test passing now. My preference is for the CLI arg for discoverability in cli help etc. |
A5rocks
approved these changes
May 25, 2025
| @@ -612,3 +612,51 @@ Example: | |||
| # mypy: disallow-any-explicit | |||
| from typing import Any | |||
| x: Any = 1 # Error: Explicit "Any" type annotation [explicit-any] | |||
|
|
|||
|
|
|||
| .. _code-exhaustive-match: | |||
There was a problem hiding this comment.
I'm not sure if the others have duplicated docs but if not could you just link from one of these sections to the other?
There was a problem hiding this comment.
Based on feedback below, I moved to only be error code based.
Means the duplication in the CLI command doc page is removed.
| @@ -469,6 +469,11 @@ If we forget to handle one of the cases, mypy will generate an error: | |||
|
|
|||
| Exhaustiveness checking is also supported for match statements (Python 3.10 and later). | |||
There was a problem hiding this comment.
I think this could be expanded instead of adding a new paragraph:
Suggested change
| Exhaustiveness checking is also supported for match statements (Python 3.10 and later). | |
| Exhaustiveness checking is also supported for match statements (Python 3.10 and later). With | |
| :option:`--disallow-inexhaustive-match-statements <mypy --disallow-inexhaustive-match-statements>`, | |
| mypy will even warn if match statements are inexhaustive. |
(not sure that's better than your current approach now that I write it out, actually)
There was a problem hiding this comment.
I think you are right here 👍
Changed to 1 paragraph
|
Copying mypy-primer comment for ease of viewing: cki-lib (https://gitlab.com/cki-project/cki-lib)
+ cki_lib/inttests/remote_responses.py:187: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/computation/parsing.py:170: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
pylox (https://github.com/sco1/pylox)
+ pylox/parser.py:106: error: Cases within match statement do not exhaustively handle all values: "Literal[TokenType.LEFT_PAREN, TokenType.RIGHT_PAREN, TokenType.LEFT_BRACE, TokenType.RIGHT_BRACE, TokenType.COMMA, TokenType.DOT, TokenType.MINUS, TokenType.PLUS, TokenType.SEMICOLON, TokenType.SLASH, TokenType.STAR, TokenType.CARAT, TokenType.PERCENT, TokenType.BACK_SLASH, TokenType.BANG, TokenType.BANG_EQUAL, TokenType.EQUAL, TokenType.EQUAL_EQUAL, TokenType.GREATER, TokenType.GREATER_EQUAL, TokenType.LESS, TokenType.LESS_EQUAL, TokenType.IDENTIFIER, TokenType.STRING, TokenType.NUMBER, TokenType.AND, TokenType.BREAK, TokenType.CONTINUE, TokenType.ELSE, TokenType.FALSE, TokenType.INCLUDE, TokenType.NIL, TokenType.OR, TokenType.SUPER, TokenType.THIS, TokenType.TRUE, TokenType.EOF]". If not intended to handle all cases, use `case _: pass` [misc]
steam.py (https://github.com/Gobot1234/steam.py)
+ steam/id.py:326: error: Cases within match statement do not exhaustively handle all values: "int | Never". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/id.py:431: error: Cases within match statement do not exhaustively handle all values: "int | Never". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/id.py:484: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/manifest.py:380: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:397: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:397: error: Cases within match statement do not exhaustively handle all values: "int | Never". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:1196: error: Cases within match statement do not exhaustively handle all values: "int | EChatRoomMemberStateChange". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:1224: error: Cases within match statement do not exhaustively handle all values: "int | EChatRoomMemberStateChange". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:1432: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:1432: error: Cases within match statement do not exhaustively handle all values: "int | EChatRoomServerMessage". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/state.py:2358: error: Cases within match statement do not exhaustively handle all values: "int". If not intended to handle all cases, use `case _: pass` [misc]
+ steam/ext/commands/cooldown.py:60: error: Cases within match statement do not exhaustively handle all values: "int | Never". If not intended to handle all cases, use `case _: pass` [misc]
core (https://github.com/home-assistant/core)
+ homeassistant/components/recorder/statistics.py:1052: error: Cases within match statement do not exhaustively handle all values: "Literal[StatisticMeanType.NONE, StatisticMeanType.ARITHMETIC]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/recorder/statistics.py:1301: error: Cases within match statement do not exhaustively handle all values: "Literal[StatisticMeanType.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/recorder/statistics.py:1321: error: Cases within match statement do not exhaustively handle all values: "Literal[StatisticMeanType.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/recorder/statistics.py:1397: error: Cases within match statement do not exhaustively handle all values: "Literal[StatisticMeanType.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/onkyo/media_player.py:112: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/homekit_controller/utils.py:26: error: Cases within match statement do not exhaustively handle all values: "list[str]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/home_connect/coordinator.py:191: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/go2rtc/__init__.py:265: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/knx/project.py:142: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/knx/expose.py:211: error: Cases within match statement do not exhaustively handle all values: "str | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/bryant_evolution/climate.py:168: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/sensor/recorder.py:620: error: Cases within match statement do not exhaustively handle all values: "Literal[StatisticMeanType.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/miele/switch.py:136: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/home_connect/__init__.py:115: error: Cases within match statement do not exhaustively handle all values: "int". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/eheimdigital/climate.py:107: error: Cases within match statement do not exhaustively handle all values: "Literal[HVACMode.HEAT, HVACMode.COOL, HVACMode.HEAT_COOL, HVACMode.DRY, HVACMode.FAN_ONLY]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/eheimdigital/climate.py:138: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/qbus/config_flow.py:49: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/home_connect/light.py:138: error: Cases within match statement do not exhaustively handle all values: "tuple[Never, Never]". If not intended to handle all cases, use `case _: pass` [misc]
+ homeassistant/components/eq3btsmart/climate.py:240: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
jax (https://github.com/google/jax)
+ jax/_src/lax/lax.py:5819: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/lax/lax.py:6034: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/lax/lax.py:6057: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/pallas/core.py:562: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/pallas/mosaic/verification.py:382: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/pallas/mosaic/pallas_call_registration.py:80: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
+ jax/_src/pallas/mosaic_gpu/core.py:213: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
bokeh (https://github.com/bokeh/bokeh)
+ src/bokeh/core/serialization.py: note: In function "_encode_typed_array":
+ src/bokeh/core/serialization.py:395:13: error: Cases within match statement do not exhaustively handle all values: "Literal['u']". If not intended to handle all cases, use `case _: pass` [misc]
+ src/bokeh/core/serialization.py:401:21: error: Cases within match statement do not exhaustively handle all values: "int". If not intended to handle all cases, use `case _: pass` [misc]
+ src/bokeh/core/serialization.py:407:21: error: Cases within match statement do not exhaustively handle all values: "int". If not intended to handle all cases, use `case _: pass` [misc]
+ src/bokeh/core/serialization.py: note: At top level:
archinstall (https://github.com/archlinux/archinstall)
+ archinstall/tui/curses_menu.py:79: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Skip, ResultType.Reset]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/tui/curses_menu.py:912: error: Cases within match statement do not exhaustively handle all values: "Literal[PreviewStyle.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/tui/curses_menu.py:925: error: Cases within match statement do not exhaustively handle all values: "Literal[PreviewStyle.NONE]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/menu/abstract_menu.py:108: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Skip]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/models/device_model.py:246: error: Cases within match statement do not exhaustively handle all values: "Literal[Unit.B, Unit.kB, Unit.MB, Unit.GB, Unit.TB, Unit.PB, Unit.EB, Unit.ZB, Unit.YB, Unit.KiB, Unit.MiB, Unit.GiB, Unit.TiB, Unit.PiB, Unit.EiB, Unit.ZiB, Unit.YiB]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/models/audio_configuration.py:41: error: Cases within match statement do not exhaustively handle all values: "Literal[Audio.NO_AUDIO]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/mirrors.py:200: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Reset]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/mirrors.py:329: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/installer.py:400: error: Cases within match statement do not exhaustively handle all values: "Literal[EncryptionType.NoEncryption]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/installer.py:1002: error: Cases within match statement do not exhaustively handle all values: "Literal[EncryptionType.Luks]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/configuration.py:128: error: Cases within match statement do not exhaustively handle all values: "Any | Any". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/configuration.py:128: error: Cases within match statement do not exhaustively handle all values: "Any | None". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/configuration.py:206: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Skip, ResultType.Reset]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/configuration.py:228: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Skip, ResultType.Reset]". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/configuration.py:239: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/profile/profile_menu.py:229: error: Cases within match statement do not exhaustively handle all values: "<subclass of "archinstall.default_profiles.profile.SelectResult" and "enum.auto"> | SelectResult". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/disk/partitioning_menu.py:278: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/disk/partitioning_menu.py:292: error: Cases within match statement do not exhaustively handle all values: "str". If not intended to handle all cases, use `case _: pass` [misc]
+ archinstall/lib/disk/partitioning_menu.py:500: error: Cases within match statement do not exhaustively handle all values: "Literal[ResultType.Reset]". If not intended to handle all cases, use `case _: pass` [misc]
Expression (https://github.com/cognitedata/Expression)
+ expression/core/tagged_union.py:67: error: Cases within match statement do not exhaustively handle all values: "tuple[Never, Never]". If not intended to handle all cases, use `case _: pass` [misc]
+ expression/extra/parser.py:130: error: Cases within match statement do not exhaustively handle all values: "Result[tuple[_A, tuple[str, int]], str]". If not intended to handle all cases, use `case _: pass` [misc]
+ expression/extra/parser.py:136: error: Cases within match statement do not exhaustively handle all values: "Result[tuple[_B, tuple[str, int]], str]". If not intended to handle all cases, use `case _: pass` [misc]
+ tests/test_tagged_union.py:264: error: Cases within match statement do not exhaustively handle all values: "Any". If not intended to handle all cases, use `case _: pass` [misc]
mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/net/http/validate.py:79: error: Cases within match statement do not exhaustively handle all values: "bytes". If not intended to handle all cases, use `case _: pass` [misc]
openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/utils/isbn.py: note: In function "get_isbn_10s_and_13s":
+ openlibrary/utils/isbn.py:141: error: Cases within match statement do not exhaustively handle all values: "int". If not intended to handle all cases, use `case _: pass` [misc]
|
There was a problem hiding this comment.
Thanks! This is a pretty popular feature request: #13597
+1 on the error code suggestion. Mypy has been moving towards using error codes for changes like this. We can improve discoverability separately
Thanks for the feedback. I have merged the change to only be error code based into this PR |
|
2 CI tasks failed on the github side, is it possible to retrigger them? |
|
You could push an empty commit |
Fair I forgot whole branch will be squashed so no need to worry about history here 😅 |
This comment has been minimized.
This comment has been minimized.
sterliakov
reviewed
May 28, 2025
| @@ -5508,6 +5509,11 @@ def visit_match_stmt(self, s: MatchStmt) -> None: | |||
| else: | |||
| self.accept(b) | |||
| self.push_type_map(else_map, from_assignment=False) | |||
| unmatched_types = else_map | |||
There was a problem hiding this comment.
Does it work as intended when typemaps contain more than immediate targets?
For example, what happens in the following fork of an existing test testMatchNarrowingUnionTypedDictViaIndex with this flag enabled? If I remember the surrounding logic correctly, there will be two typemap entries - for d['tag'] and for d, so two diagnostics?
from typing import Literal, TypedDict
class A(TypedDict):
tag: Literal["a"]
name: str
class B(TypedDict):
tag: Literal["b"]
num: int
d: A | B
match d["tag"]:
case "a":
reveal_type(d) # N: Revealed type is "TypedDict('__main__.A', {'tag': Literal['a'], 'name': builtins.str})"
reveal_type(d["name"]) # N: Revealed type is "builtins.str"There was a problem hiding this comment.
I would say that yes, it is behaving as I originally intended with 2 diagnostics. Added a test testExhaustiveMatchNarrowingUnionTypedDictViaIndex for the use case you have above.
d219871
My thinking of having multiple diagnostics is it might be a bit easier to read/understand in the case of longer unions, but I don't have a very strong opinion here either way
Co-authored-by: Stanislav Terliakov <50529348+sterliakov@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Fix for more deterministic behaviour
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.