[ty] Handle enum flag in if/else and match#23808
[ty] Handle enum flag in if/else and match#23808silamon wants to merge 2 commits intoastral-sh:mainfrom
Conversation
silamon
requested review from
AlexWaygood,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 87.10% to 87.20%. The percentage of expected errors that received a diagnostic increased from 77.81% to 77.91%. The number of fully passing files improved from 63/131 to 64/131. SummaryHow are test cases classified?Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (
Test file breakdown1 file altered
True positives added (1)1 diagnostic
False positives removed (1)1 diagnostic
|
|
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
| STOPPED = 4 | ||
|
|
||
| reveal_type(Status.READY) # revealed: Literal[Status.READY] | ||
| reveal_type(Status.READY | Status.RUNNING) # revealed: Literal[Status.READY] |
There was a problem hiding this comment.
hmm, this seems incorrect. At runtime, this produces a value that is not equal to either Status.READY or to Status.RUNNING:
>>> from enum import Flag
... from typing import Literal
...
... class Permissions(Flag):
... READ = 1
... WRITE = 2
... EXECUTE = 4
...
>>> Permissions.READ | Permissions.WRITE
<Permissions.READ|WRITE: 3>
>>> Permissions.READ | Permissions.WRITE == Permissions.READ
False
>>> Permissions.READ | Permissions.WRITE == Permissions.WRITE
FalseIt honestly seems quite strange to me to consider Flag enum members to inhabit Literal types at all -- but every other type checker does, so I suppose we have to follow suit on that one.
Pyright, zuban and pyrefly all have the same "bug" here -- which is really just us following typeshed's annotations correctly, so perhaps typeshed just needs to change here. So maybe we should just add a comment noting that this isn't accurate with respect to the runtime?
| EXECUTE = 4 | ||
|
|
||
| reveal_type(Permissions.READ) # revealed: Literal[Permissions.READ] | ||
| reveal_type(Permissions.READ | Permissions.WRITE) # revealed: Literal[Permissions.READ, Permissions.WRITE] |
There was a problem hiding this comment.
same here as https://github.com/astral-sh/ruff/pull/23808/changes#r2902187320 -- this seems wrong; it's not what happens at runtime. We should probably just add a TODO comment for now.
| def test(f: Flags) -> None: | ||
| match f: | ||
| case Flags.A | Flags.B: | ||
| # Pattern matching on flags does not narrow to specific literals |
| assert_type(f, Flags) | ||
| ``` | ||
|
|
||
| ### Difference from regular enums |
There was a problem hiding this comment.
isn't this section testing the same thing as the ### Flag narrowing with if/else section above?
3 participants
Summary
Add support for handling enum flags better in if/else and match statements.
Test Plan
Mdtests