Skip to content
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

Cases within match statement do not exhaustively handle all values Unhandled type: "." when using unions #4692

Closed
patrick91 opened this issue Aug 7, 2023 · 3 comments
Assignees
Labels
needs repro Issue has not been reproduced yet

Comments

@patrick91
Copy link

Environment data

  • Language Server version: 2023.8.10
  • OS and version: darwin arm64
  • Python version (and distribution if applicable, e.g. Anaconda):
  • python.analysis.indexing: true
  • python.analysis.typeCheckingMode: strict

Code Snippet

import json
from typing import Literal, TypedDict, Any, cast


class CodegenMessage(TypedDict):
    type: Literal["message"]
    message: str


class CodegenResult(TypedDict):
    type: Literal["result"]
    sandbox_url: str


CodegenChunk = CodegenMessage | CodegenResult


def something(data: Any) -> None:
    result = cast(CodegenChunk, json.loads(data["message"]))

    match result:
        case {"type": "message", "message": text}:
            print("message", text)
        case {"type": "result", "sandbox_url": sandbox_url}:
            print("result", sandbox_url)

Repro Steps

Run VS code on that file with pylance

  1. XXX

Expected behavior

No Error

Actual behavior

Cases within match statement do not exhaustively handle all values
  Unhandled type: "CodegenChunk"
  If exhaustive handling is not intended, add "case _: pass"Pylance[reportMatchNotExhaustive](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportMatchNotExhaustive)
CleanShot 2023-08-07 at 12 40 57@2x

Notes

This issue not present on pyright

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Aug 7, 2023
@erictraut
Copy link
Contributor

It looks like you have the reportMatchNotExhaustive check enabled. This check isn't enabled by default. It's considered a "strict" type checking option, and it's a bit opinionated in how it works.

Exhaustion detection is based on type narrowing. You're using a pattern here that pyright (the type checker that underlies pylance) does not narrow. Mypy (another Python static type checker) also doesn't support this pattern. What you're doing here is pretty atypical. The more typical way of handling this problem is to match on the discriminating key only. Both pyright and mypy support this.

If you change your code as follows, it will work as expected.

def something(data: Any) -> None:
    result = cast(CodegenChunk, json.loads(data["message"]))

    match result:
        case {"type": "message"}:
            print("message", result["message"])
        case {"type": "result"}:
            print("result", result["sandbox_url"])

I don't know how this function is intended to be used, but if the JSON data is coming from some external source (i.e. its contents haven't been generated from within your program or already validated by some other code), then it's not safe to assume that its contents will match one of these two patterns. In that case, I recommend that you stick with the original code but include a fallback pattern:

    match result:
        case {"type": "message", "message": text}:
            print("message", text)
        case {"type": "result", "sandbox_url": sandbox_url}:
            print("result", sandbox_url)
        case _:
            print("None of the above")

In summary, I don't consider this a bug in pyright. It could be a feature request, but we're unlikely to add support for this particular type narrowing pattern unless/until we get signal from enough other pyright users that it's a commonly-used pattern.

@patrick91
Copy link
Author

Thanks for the explanation! I think it makes sense! I'll change the code to just check the discriminating key 😊

Shall I keep this open or leave it for a bit?

@rchiodo
Copy link
Contributor

rchiodo commented Aug 7, 2023

I think we'd rather close it. If it's not actually a bug, we don't keep issues open.

@rchiodo rchiodo closed this as completed Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs repro Issue has not been reproduced yet
Projects
None yet
Development

No branches or pull requests

3 participants