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

Type narrowing broken with dict items #18440

Open
xceral opened this issue Jan 10, 2025 · 4 comments · May be fixed by #18446
Open

Type narrowing broken with dict items #18440

xceral opened this issue Jan 10, 2025 · 4 comments · May be fixed by #18446
Labels
bug mypy got something wrong topic-match-statement Python 3.10's match statement topic-type-narrowing Conditional type narrowing / binder

Comments

@xceral
Copy link

xceral commented Jan 10, 2025

Bug Report

When using a dict item, the type does not get narrowed.

To Reproduce

https://mypy-play.net/?mypy=latest&python=3.12&gist=77cccd913f6c38a3536c4306c1439cf3

Expected Behavior

Both functions should typecheck.

Actual Behavior

main.py:6: error: Item "int" of "int | str" has no attribute "upper"  [union-attr]
main.py:16: error: Argument 1 to "assert_never" has incompatible type "int | str"; expected "Never"  [arg-type]
Found 2 errors in 1 file (checked 1 source file)

A workaround with an intermediate variable typechecks without issues.

Your Environment

  • Mypy version used: 1.13.0 (initially; also tested with master, 1.14.1, 1.12.0, 1.0.0)
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.12
@xceral xceral added the bug mypy got something wrong label Jan 10, 2025
@tyralla
Copy link
Collaborator

tyralla commented Jan 10, 2025

Thanks for the report!

The first example is not a bug. Consider this example:

def a(k: str, d: dict[str, int | str]):
    if isinstance(d[k], str):
        d[k] = 1
        d[k].upper()  # clearly a crash

However, the second example is in fact a limitation:

def b(k: str, d: dict[str, int|str]):
    match d[k]:
        case str(s):
            print(s.upper())
        case int(x):
            print(x)
        case unreachable:
            assert_never(unreachable)  # really not reachable

@tyralla tyralla added topic-match-statement Python 3.10's match statement topic-type-narrowing Conditional type narrowing / binder labels Jan 10, 2025
@sterliakov
Copy link
Contributor

And the second part happens because only calls are currently considered "special enough", and an IndexExpr is not a CallExpr:

mypy/mypy/checker.py

Lines 5272 to 5284 in 106f714

def visit_match_stmt(self, s: MatchStmt) -> None:
named_subject: Expression
if isinstance(s.subject, CallExpr):
# Create a dummy subject expression to handle cases where a match statement's subject
# is not a literal value. This lets us correctly narrow types and check exhaustivity
# This is hack!
id = s.subject.callee.fullname if isinstance(s.subject.callee, RefExpr) else ""
name = "dummy-match-" + id
v = Var(name)
named_subject = NameExpr(name)
named_subject.node = v
else:
named_subject = s.subject

That was introduced in #16503. I guess it should be possible to rewrite that name generation logic to make it work with any expression, not only a simple call.

@tyralla
Copy link
Collaborator

tyralla commented Jan 10, 2025

@sterliakov: Cool, do you plan to extend "this hack"? I was starting to do it, but only have the test case so far:

# flags: --warn-unreachable

from typing import Dict, Union

m: Dict[str, Union[int, str]]
k: str

match m[k]:
    case str(s):
        reveal_type(s)  # N: Revealed type is "builtins.str"
    case int(i):
        reveal_type(i)  # N: Revealed type is "builtins.int"
    case u:
        u

[builtins fixtures/dict.pyi]

Feel free to take it, if this small snippet seems useful.

@sterliakov
Copy link
Contributor

@tyralla yep, I'll try to get this done soon. I think I can (at a reasonably low cost of just keeping a last_used_id per TypeChecker) handle that for arbitrary expressions in match without trying to specialize for any specific one. That would also allow narrowing of immediately-constructed tuples/lists/dicts/sets (with tuple being probably the most popular pattern), and maybe some other expressions I didn't think of yet.

We can also try to extend this spaghetti one-by-one for call, index, tuple, list, ... expressions, but I don't think it's the most ergonomic solution. This id shouldn't ever leak to error messages, so IMO just f"dummy-match-{self.last_used_id}" is a perfect candidate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-match-statement Python 3.10's match statement topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants