microsoft / pyright

Static Type Checker for Python
Other
13.48k stars 1.48k forks source link

Possible bug with `isinstance` type-narrowing #9441

Open tylerlaprade opened 2 weeks ago

tylerlaprade commented 2 weeks ago

Hi, I'm not sure if this is a bug in the pre-release version or if it's working as intended. Model in this case comes from Django, and its metaclass is ModelBase. This code typechecks just fine on the release version but errors on the commented line when on the pre-release version. The detected type is model_or_iterable: list[type[Model]] | type[Model] | Iterable[type[Model]].

class Model(AltersData, metaclass=ModelBase):
class AbacusAdminSite(admin.AdminSite):
    """Custom admin site class for some UI tweaks."""

    @override
    def register(
        self,
        model_or_iterable: 'type[Model] | Iterable[type[Model]]',
        admin_class: type['ModelAdmin[Any]'] | None = None,
        **options,
    ) -> None:
        super().register(model_or_iterable, admin_class=admin_class, **options)
        if isinstance(model_or_iterable, ModelBase):
            model_or_iterable = [model_or_iterable]
        for model in model_or_iterable:  // "type[Model]" is not iterable
  "__iter__" method not definedPylance[reportGeneralTypeIssues]
            if not _should_make_proxy_model(model):
                continue
            soft_deleted_model = create_proxy_model(model)
            soft_deleted_model_admin = (
                create_softdeleted_model_admin(admin_class) if admin_class else None
            )
            super().register(
                soft_deleted_model, admin_class=soft_deleted_model_admin, **options
            )
erictraut commented 2 weeks ago

Can you provide a minimal self-contained example? The code posted above contains many symbols that are not defined.

tylerlaprade commented 2 weeks ago
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from collections.abc import Iterable

class ModelBase(type):
    pass
class Model(metaclass=ModelBase):
    pass
def register(
        model_or_iterable: 'type[Model] | Iterable[type[Model]]',
    ):
        if isinstance(model_or_iterable, ModelBase):
            model_or_iterable = [model_or_iterable]
        for _model in model_or_iterable:   # "type[Model]" is not iterable "__iter__" method not definedPylance[reportGeneralTypeIssues]
            pass
erictraut commented 2 weeks ago

You mentioned the "pre-release version". Do you suspect that this is a regression? Are you building your own version of pyright from source? I'm not seeing any difference in behavior between the main branch and the latest published version of pyright.

I'm not sure I'd classify this as a bug, but I agree there's an opportunity for further narrowing in the negative case when using a metaclass in an isinstance type guard.

class Meta(type):
    pass

class A(metaclass=Meta):
    pass

def func(m: type[A] | str):
    if not isinstance(m, Meta):
        reveal_type(m)  # type[A] | str, but could be just str
erictraut commented 2 weeks ago

Ah, it just occurred to me that you're talking about the pre-release version of pylance, not of pyright.

Pyright's type narrowing logic changed two versions ago (in 1.1.387) to fix some bugs. It looks like the behavior changed in this case. I'll look at restoring the previous behavior.

tylerlaprade commented 2 weeks ago

Perfect, thanks Eric! Sorry for the confusion.