Preserve type arguments when narrowing with isinstance(x, type)#3604
Preserve type arguments when narrowing with isinstance(x, type)#3604mikeleppane wants to merge 1 commit into
isinstance(x, type)#3604Conversation
Narrowing a union such as `type[int] | str` with `isinstance(x, type)` collapsed the class object to bare `type`, dropping the `int` argument (issue facebook#3585). The shared `unwrap_class_object_silently` unwrapped `type` to a nominal `ClassType(type)`, which does not intersect cleanly with the type-form representation of `type[int]`, so the argument was lost. Derive the isinstance target from the value being narrowed instead. When the value is already a type expression (`type[...]` or `TypeForm[...]`), narrow to `type[inner]`: concrete arguments like `type[int]` stay precise and gradual `type[Any]` stays gradual, matching pyright and mypy. Other values fall back to the nominal `ClassType(type)`, which narrows `object` to `type` and non-class instances to `Never`. Scope the special case to the isinstance path. `narrow_issubclass` shares `unwrap_class_object_silently` but untypes its operand to the instance level, so it relies on the nominal `ClassType(type)` to keep correct subclass semantics; routing the type-form target only through `unwrap_isinstance_target` leaves issubclass narrowing unchanged. Fixes facebook#3585
|
Hi @mikeleppane! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
The CLA was signed over an hour ago; waiting for a confirmation comment? |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Diff from mypy_primer, showing the effect of this PR on open source code: bokeh (https://github.com/bokeh/bokeh)
+ ERROR src/bokeh/core/property/instance.py:99:29-48: `type | type[T]` is not assignable to variable `instance_type` with type `type[Serializable]` [bad-assignment]
- ERROR src/bokeh/core/property/instance.py:110:16-29: Returned type `type | type[Serializable]` is not assignable to declared return type `type[T]` [bad-return]
+ ERROR src/bokeh/core/property/instance.py:110:16-29: Returned type `type[Serializable]` is not assignable to declared return type `type[T]` [bad-return]
pytest (https://github.com/pytest-dev/pytest)
- ERROR src/_pytest/raises.py:425:20-23: Returned type `type[BaseException]` is not assignable to declared return type `type[BaseExcT_1]` [bad-return]
beartype (https://github.com/beartype/beartype)
- ERROR beartype/_util/text/utiltextlabel.py:556:41-44: Argument `type` is not assignable to parameter `hint` with type `TypeForm[Any]` in function `beartype._util.hint.pep.proposal.pep484585.generic.pep484585gentest.is_hint_pep484585_generic_subbed` [bad-argument-type]
- ERROR beartype/_util/text/utiltextlabel.py:585:38-41: Argument `type` is not assignable to parameter `hint` with type `TypeForm[Any]` in function `beartype._util.hint.pep.proposal.pep544.is_hint_pep544_protocol` [bad-argument-type]
static-frame (https://github.com/static-frame/static-frame)
- ERROR static_frame/core/store_config.py:349:31-44: `StoreConfigBase` is not assignable to variable `default` with type `TVStoreConfig | None` [bad-assignment]
|
Primer Diff Classification✅ 4 improvement(s) | 4 project(s) total | +2, -5 errors 4 improvement(s) across bokeh, pytest, beartype, static-frame.
Detailed analysis✅ Improvement (4)bokeh (+2, -1)
pytest (-1)
beartype (-2)
static-frame (-1)
Was this helpful? React with 👍 or 👎 Classification by primer-classifier (4 LLM) |
Summary
Fixes #3585.
isinstance(x, type)dropped the type argument when narrowing aunion, e.g.
type[int] | strnarrowed to baretypeinstead oftype[int].Pyright and mypy both keep
type[int].Root cause
isinstancenarrowing unwraps its right-hand side via the sharedunwrap_class_object_silently, which turned the builtin type into a nominalClassType(type). That representation does not intersect cleanly with thetype-form representation of
type[int](Type::Type(ClassType(int))), so theargument was lost and the result collapsed to bare type.
Approach
Derive the isinstance target from the value being narrowed instead of a single
fixed unwrapping of type:
value is
type[...]-> keep it (concretetype[int]stays precise; gradualtype[Any]stays gradual)value is
TypeForm[...]-> narrow to type[inner]anything else → fall back to the nominal ClassType(type), which narrows object to type and non-class instances to Never
The special case is scoped to unwrap_isinstance_target (the isinstance-only
entry point).
narrow_issubclasssharesunwrap_class_object_silentlybutuntypes its operand to the instance level, so it still needs the nominal
ClassType(type)- leaving the special case out of the shared helper keepsissubclass(x, type)subclass semantics intact.Fixes #3585 and #3584
Test Plan
cargo test -p pyrefly test::narrow- narrow suite (204 passing)cargo test -p pyrefly isinstance/issubclass/metaclass- 64 / 22 / 47 passingcargo test -p pyrefly --lib- full lib suite: 5282 passed, 0 failedpython3 test.py --no-test --no-conformance --no-jsonschema- fmt + lint clean