Skip to content

Preserve type arguments when narrowing with isinstance(x, type)#3604

Open
mikeleppane wants to merge 1 commit into
facebook:mainfrom
mikeleppane:fix/isinstance-type-narrowing
Open

Preserve type arguments when narrowing with isinstance(x, type)#3604
mikeleppane wants to merge 1 commit into
facebook:mainfrom
mikeleppane:fix/isinstance-type-narrowing

Conversation

@mikeleppane
Copy link
Copy Markdown

Summary

Fixes #3585.
isinstance(x, type) dropped the type argument when narrowing a
union, e.g. type[int] | str narrowed to bare type instead of type[int].
Pyright and mypy both keep type[int].

def f(value: type[int] | str):
    if isinstance(value, type):
        reveal_type(value)  # was: type   now: type[int]

Root cause

isinstance narrowing unwraps its right-hand side via the shared
unwrap_class_object_silently, which turned the builtin type into a nominal
ClassType(type). That representation does not intersect cleanly with the
type-form representation of type[int] (Type::Type(ClassType(int))), so the
argument 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 (concrete type[int] stays precise; gradual type[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_issubclass shares unwrap_class_object_silently but
untypes its operand to the instance level, so it still needs the nominal
ClassType(type) - leaving the special case out of the shared helper keeps
issubclass(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 passing
  • cargo test -p pyrefly --lib - full lib suite: 5282 passed, 0 failed
  • python3 test.py --no-test --no-conformance --no-jsonschema - fmt + lint clean

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
@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented May 28, 2026

Hi @mikeleppane!

Thank you for your pull request and welcome to our community.

Action Required

In 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.

Process

In 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 CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@mikeleppane
Copy link
Copy Markdown
Author

The CLA was signed over an hour ago; waiting for a confirmation comment?

@meta-cla meta-cla Bot added the cla signed label May 28, 2026
@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented May 28, 2026

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@github-actions
Copy link
Copy Markdown

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]

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

✅ 4 improvement(s) | 4 project(s) total | +2, -5 errors

4 improvement(s) across bokeh, pytest, beartype, static-frame.

Project Verdict Changes Error Kinds Root Cause
bokeh ✅ Improvement +2, -1 Improved isinstance narrowing exposing real type annotation bug pyrefly/lib/alt/narrow.rs
pytest ✅ Improvement -1 bad-return pyrefly/lib/alt/narrow.rs
beartype ✅ Improvement -2 bad-argument-type pyrefly/lib/alt/narrow.rs
static-frame ✅ Improvement -1 bad-assignment pyrefly/lib/alt/narrow.rs
Detailed analysis

✅ Improvement (4)

bokeh (+2, -1)

Improved isinstance narrowing exposing real type annotation bug: The local variable instance_type: type[Serializable] on line 97 is inconsistent with the narrowed type of self._instance_type after the isinstance check. After isinstance(self._instance_type, type), the narrowed type is type | type[T]type[T] from the first union member and bare type from Callable[[], type[T]] (since type is callable and passes the isinstance check). Previously, the isinstance narrowing bug collapsed type[T] to bare type, partially masking this. Now that narrowing correctly preserves type[T], the full type | type[T] is reported as not assignable to type[Serializable]. Pyright agrees with both errors.
More precise error message on bad-return: The return type error changed from type | type[Serializable] (which included the spurious bare type from broken narrowing leaking into the variable's type) to just type[Serializable]. With the fix, the variable's declared type type[Serializable] is used cleanly, and the error correctly reports that type[Serializable] is not assignable to the declared return type type[T]. The error is still correct but now more precise.

Overall: The PR fixed isinstance narrowing to preserve type arguments (matching pyright/mypy behavior). This exposed a genuine type annotation inconsistency in bokeh's code. The local variable instance_type is annotated as type[Serializable] (line 97) but the class's _instance_type field has type type[T] | Callable[[], type[T]] | str. After the isinstance check isinstance(self._instance_type, type) on line 98, the narrowed type is type | type[T] — the type[T] comes from the first union member, and bare type comes from Callable[[], type[T]] being narrowed by isinstance(..., type) (since type is callable). Previously, the isinstance narrowing bug collapsed type[T] to bare type, masking part of this mismatch. The new bad-assignment error at line 99 correctly reports that type | type[T] cannot be assigned to type[Serializable], since T is bound to object and bare type is not type[Serializable]. The bad-return error at line 110 improved from type | type[Serializable] to just type[Serializable] — the old error included the spurious bare type that leaked through from the broken narrowing into the variable's inferred type, while now the variable's declared type type[Serializable] is used cleanly, and type[Serializable] is correctly reported as not assignable to the declared return type type[T]. Both errors are confirmed by pyright.

Attribution: The change to unwrap_isinstance_target logic in pyrefly/lib/alt/narrow.rs (lines 440-449) now preserves Type::Type(_) when the isinstance target is builtins.type. Previously, isinstance(self._instance_type, type) narrowed type[T] to bare type, losing the type argument. Now it correctly keeps type[T], which exposes the real type mismatch between type[T] and the local annotation type[Serializable].

pytest (-1)

This is a clear improvement. The removed error was a false positive caused by pyrefly incorrectly dropping type arguments during isinstance narrowing. The code return exc on line 425 is correct — after isinstance(exc, type) narrows type[BaseExcT_1] | types.GenericAlias to type[BaseExcT_1], the return type matches the declared type[BaseExcT_1]. The PR fixes this narrowing behavior to match mypy/pyright.
Attribution: The change to unwrap_isinstance_target logic in pyrefly/lib/alt/narrow.rs (lines 440-449) now preserves the type argument when the left operand is already a Type::Type(_). Previously, isinstance(exc, type) would go through unwrap_class_object_silently which turned type into a nominal ClassType(type), losing the BaseExcT_1 argument. Now Type::Type(_) is returned as-is, preserving type[BaseExcT_1].

beartype (-2)

Both removed errors were false positives where pyrefly incorrectly rejected the variable cls (annotated as type) as not assignable to a parameter typed TypeForm[Any]. At line 556, cls is passed to is_hint_pep484585_generic_subbed(cls), and at line 585, cls is passed to is_hint_pep544_protocol(cls). In both cases, the parameter hint expects TypeForm[Any], and cls has type type. Since type represents the class of all classes, and classes are valid type forms, type should be assignable to TypeForm[Any]. The PR fixed pyrefly's internal handling so that type is now correctly recognized as assignable to TypeForm[Any], resolving these spurious errors. This is an improvement in correctness.
Attribution: The change to unwrap_isinstance_target in pyrefly/lib/alt/narrow.rs improved how bare type is represented internally. The new match arm for isinstance(x, type) preserves type arguments and handles bare type as type[Any], which likely improved the internal type representation of type throughout the system, fixing the false bad-argument-type errors when type was passed to TypeForm[Any] parameters.

static-frame (-1)

The removed error was a false positive caused by pyrefly's type narrowing (likely from the isinstance(config_type, type) or issubclass(config_type, StoreConfigBase) checks on lines 340-341) narrowing config_type from type[TVStoreConfig] to type[StoreConfigBase], losing the TypeVar parameterization. As a result, config_type() was inferred as returning StoreConfigBase instead of TVStoreConfig, making the assignment to default: TVStoreConfig | None appear invalid. The PR fixes this by preserving type arguments during narrowing of type[X] expressions. This matches mypy and pyright behavior, which correctly maintain the TypeVar through such narrowing.
Attribution: The change to unwrap_isinstance_target logic in pyrefly/lib/alt/narrow.rs (lines 440-449) adds a special case: when the isinstance right-hand side is builtins.type and the left-hand side is already Type::Type(_), it preserves the original type instead of unwrapping to bare ClassType(type). This directly fixes the false positive where type[TVStoreConfig] was narrowed to bare type.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (4 LLM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

narrowing a type using isinstance doesn't work

1 participant