Skip to content

[match-case] unreachable raised multiple times instead of once. #15866

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
randolf-scholz opened this issue Aug 14, 2023 · 7 comments
Open

[match-case] unreachable raised multiple times instead of once. #15866

randolf-scholz opened this issue Aug 14, 2023 · 7 comments
Labels
feature topic-match-statement Python 3.10's match statement topic-reachability Detecting unreachable code topic-type-ignore # type: ignore comments

Comments

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Aug 14, 2023

Bug Report

unreachable is raised multiple times in match-case.

To Reproduce

from typing import Any
import re

def foo(config: dict[str, Any], section: str) -> set[str]:
    """Extract dependencies from pyproject.toml."""
    try:
        for key in section.split("."):
            config = config[key]
    except KeyError:
        return NotImplemented
    
    reveal_type(config)  # dict[str,Any], should be dict[str,Any] | Any
    
    match config:
        # assume format `"package<comparator>version"`
        case list() as lst:  # ✘ unreachable
            regex = re.compile(r"[a-zA-Z0-9_-]*")   # ✘ unreachable
            return {re.search(regex, dep).group() for dep in lst}
        # assume format `package = "<comparator>version"`
        case dict() as dct:
            return set(dct.keys())       
        case _:
            raise TypeError(f"Unexpected type: {type(config)}")

https://mypy-play.net/?mypy=latest&python=3.11&flags=strict%2Cwarn-unreachable&gist=d747fcb30a23bd50025c2ca47ed8c5cf

Expected Behavior

A single type: ignore[unreachable] on line case list() as lst: should suffice to silence both messages / there should only be a singular unreachable error to begin with.

Actual Behavior

main.py:16: error: Subclass of "dict[str, Any]" and "list[Any]" cannot exist: would have incompatible method signatures  [unreachable]
main.py:17: error: Statement is unreachable  [unreachable]
Found 2 errors in 1 file (checked 1 source file)

with the added type: ignore on line 16 it's still

main.py:17: error: Statement is unreachable  [unreachable]
Found 1 error in 1 file (checked 1 source file)

EDIT: moved comment in code.

@randolf-scholz randolf-scholz added the bug mypy got something wrong label Aug 14, 2023
@AlexWaygood AlexWaygood added topic-match-statement Python 3.10's match statement topic-reachability Detecting unreachable code feature topic-type-ignore # type: ignore comments and removed bug mypy got something wrong labels Aug 14, 2023
@sobolevn
Copy link
Member

sobolevn commented Aug 21, 2023

This doesn't look like a bug to me. The first error message is correct: you cannot have dict that is a list. The second message is also correct: regex = re.compile is unreachable.

@randolf-scholz
Copy link
Contributor Author

@sobolevn I don't think the first error message is correct either to be honest. From the try-except block, mypy should infer that config is now of type dict[str, Any] | Any.

The point about the second error message is that it is superfluous. Having subsequent [unreachable]-errors in a linear piece of code is redundant.

@randolf-scholz
Copy link
Contributor Author

randolf-scholz commented Aug 21, 2023

The issue seems to be that [unreachble] is used with two different meanings. In the first message

main.py:16: error: Subclass of "dict[str, Any]" and "list[Any]" cannot exist: would have incompatible method signatures [unreachable]

We basically get told that the program can never progress from line 16 to line 17, because the case cannot occur.
So this is about unreachability of the end of a statement. On the other hand, line 17.

main.py:17: error: Statement is unreachable [unreachable]

We get told that this line can never be reached, in the sense that the program can never proceed from the previous line to this line.

So there are two [unreachable] -statements are semantically different. The first is [unreachable-end-of-line] whereas the latter is [unreachable-beginning-of-line]. They both talk about the same line transition 16 -> 17 and are hence redundant.

[unreachable-beginning-of-line] should therefore only ever be triggered if the previous line doesn't trigger [unreachable-end-of-line].

Alternatively, Subclass of "dict[str, Any]" and "list[Any]" cannot exist could trigger a different error code than [unreachable].

@randolf-scholz
Copy link
Contributor Author

randolf-scholz commented Aug 21, 2023

@sobolevn In principle, it would make sense for [unreachable] to be triggered for and only for the immediate statement following unreachable code. Ideally, this would be within the same line.

from typing import NoReturn

def foo() -> NoReturn:
    while True:
        pass

def bar(x: int) -> int:
    return x
    
y = bar(foo())

This code does not produce any errors currently (#15821). However, imo, it should produce a message like

main.py:10: error: Statement is unreachable  [unreachable]

    y = bar(foo())
        ^^^
        bar can never be reached since foo is non-returning.

@erictraut
Copy link

@randolf-scholz, you said:

From the try-except block, mypy should infer that config is now of type dict[str, Any] | Any.

That's an incorrect assumption. You've declared the type of config to be dict[str, Any]. The assignment within the try block assigns a value of type dict[str, Any] | Any to the variable config. This assignment is allowed (because dict[str, Any] | Any is compatible with the declared type), but that doesn't mean the type of config changes to dict[str, Any] | Any after the assignment. It remains dict[str, Any]. The type of an expression is never narrowed to Any upon assignment unless its declaration contains an Any.

You can see this with a simpler example.

def func(y: Any):
    x: list[int]
    x = y
    reveal_type(x) # list[int]

@randolf-scholz
Copy link
Contributor Author

randolf-scholz commented Aug 21, 2023

@erictraut I was describing an 'ought' rather than an 'is'. If a human looks at this code:

def foo(config: dict[str, Any], section: str) -> set[str]:
    """Extract dependencies from pyproject.toml."""
    try:
        for key in section.split("."):
            config = config[key]
    except KeyError:
        return NotImplemented

Then the logical conclusion is that if the try-block succeeded, the result could be an object of any type. At least with --allow-redefinition, I would expect that this change is considered (what is the difference to this example?). Besides, I wonder why you call it a "narrowing" to Any; it seems more like a "widening" of the type to me.

How would you type hint the code above? One could put config = cast(Any, config) at the first line in the function, but honestly that is just ugly.

@sobolevn
Copy link
Member

@randolf-scholz use two different variables with different types: config and parsed_config (or whatever)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-match-statement Python 3.10's match statement topic-reachability Detecting unreachable code topic-type-ignore # type: ignore comments
Projects
None yet
Development

No branches or pull requests

4 participants