Don't narrow a failed class pattern match against type[X] - #11782
Shubham Padkonde (Shubham-Padkonde) wants to merge 2 commits into
Conversation
A class pattern whose class is a type[X] value may match only a subclass of X at runtime, so a failed match says nothing about an instance of X. Negative narrowing treated the pattern as exactly X, eliminated the subject and made the following case unreachable. Skip negative narrowing when the pattern class includes subclasses, as isinstance narrowing already does. Fixes microsoft#11294 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Compared candidate Type checker benchmark🟢 No performance regressions detected. Regression threshold:
Pyright stats
|
|
🔒 Automated review in progress — Bill Schnurr (@bschnurr) is auto-reviewing this PR. |
| case subclass(): | ||
| reveal_type(subject, expected_text="Example") | ||
| case _: | ||
| reveal_type(subject, expected_text="Example") |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
📍 packages/pyright-internal/src/tests/samples/matchClass9.py:15
The PR description promises coverage for cls() inside a classmethod, but this sample contains only free-function type[Example] parameters. Add the classmethod regression case to exercise that distinct producer of subclass-inclusive class types, or correct the PR description.
[verified]
There was a problem hiding this comment.
Corrected the PR description rather than the sample — the cls() claim was false, and it could not be satisfied: pyright rejects case cls(): with "type[Self@Example]" is not a class, on clean main as well. Details and output in this comment.
|
Result: Verification detailsVerification: Isolated verification observed failures whose relationship to this PR could not be determined: Inferred classmethod cls pattern reproduction; this review is not fully verified. Summary: Offline dependencies installed successfully. The targeted MatchClass Jest run passed all nine tests, including the new MatchClass9 test. However, an ad-hoc classmethod `cls()` case described by the PR failed because Pyright reported `type[Self@Example] is not a class`; the committed sample does not contain this claimed case. Verification therefore indicates missing coverage and incomplete support for the described classmethod behavior. Test runs: 2 passed, 1 failed
❌
|
Bill Schnurr (bschnurr)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
|
You're right, and I've corrected the PR description rather than the sample — the On adding it instead: it can't be added, because pyright rejects that pattern outright. Running your reproduction: class Example:
@classmethod
def check(cls, subject: "Example") -> None:
match subject:
case cls():
reveal_type(subject, expected_text="Example")
case _:
reveal_type(subject, expected_text="Example")on this branch: and on clean So The two cases the sample does cover still fail on Disclosure: this investigation was performed by Claude Code (Claude Opus 5) working as my agent, at my direction. The output quoted above comes from real runs in my local environment; I am accountable for this PR. |
🔴 Pyright CLI QA —
|
|
The new CLI QA report flags a potential behavior difference, but does not include a reproducer, expected/actual diagnostics, or the checks that could not complete. Could you share those details so I can reproduce the finding and add a targeted regression? I am investigating the report; I am not treating the incomplete result as a pass. Prepared with Codex assistance. |
|
Fixed final-class negative narrowing in 583f4d4. A type[X] pattern remains conservative when X can have subclasses, but uses normal negative narrowing when X is final (including bool). Added the supplied final-class and bool fallthrough cases: they fail before the fix and all 159 typeEvaluator6 tests pass afterward. TypeScript compilation, ESLint, formatting and git diff --check also pass. Prepared and tested with Codex assistance. |
|
Compared candidate View the full release history charts with the base and PR results. Execution time historyPeak memory historyType checker benchmark🟢 No performance regressions detected. Regression threshold:
Pyright stats
|
Fixes #11294
In a class pattern, the class expression can be a variable of type
type[X](orclsin a classmethod) rather than the classXitself. At runtime that value may be a subclass ofX, socase subclass(...)can fail for an instance ofX. pyright's negative narrowing treated the pattern as exactlyX, eliminated the subject, and reported the followingcaseas unreachable:Change
In
narrowTypeBasedOnClassPattern, the negative (no-match) case no longer narrows when the pattern's class type hasincludeSubclassesset. That's how pyright represents atype[X]value, as opposed to the classXitself. Theisinstancenegative narrowing intypeGuards.tsalready treatsincludeSubclassesfilters as indeterminate for the same reason. Positive narrowing and patterns that name a class directly are unchanged.Tests
New sample
matchClass9.py, run withreportUnreachableenabled, covers:type[X]pattern with no arguments.Correction: an earlier version of this description also claimed coverage for a
cls()pattern in a classmethod. That was wrong — the sample never contained such a case, and it could not, because pyright rejectscase cls():outright with"type[Self@Example]" is not a class. That happens onmainas well, so it is a separate limitation rather than anything this PR affects. See the discussion below.Results:
main(unreachable-code errors andNeverin the fallthrough case) and passes with this change.typeEvaluator1–8suites pass, including everyMatchClass*test.This change was written with help from an AI coding assistant (Claude Code). I reviewed and tested it as described above.
🤖 Generated with Claude Code