DetachHead / basedpyright

pyright fork with various type checking improvements, improved vscode support and pylance features built into the language server
https://docs.basedpyright.com
Other
1.19k stars 21 forks source link

`reportUninitializedInstanceVariable` false positive in some cases when extending multiple protocols #870

Open vikigenius opened 4 days ago

vikigenius commented 4 days ago

edit: minimal repro

Code sample in basedpyright playground

from typing import Protocol, final

class EProto(Protocol):
    a: int

class E(EProto):
    a: int = 1

class AProto(EProto, Protocol):
    at: int

class A(AProto, E):
    at: int = 1

@final
class AB(A, AProto): ...  # error: reportUninitializedInstanceVariable on `at`

print(AB().at)  # works at runtime, prints 1

original issue

from __future__ import annotations
from typing import final
from advanced_alchemy.repository import SQLAlchemyAsyncSlugRepository

from chatpyre.db.models import Role

@final
class RoleRepository(SQLAlchemyAsyncSlugRepository[Role]):
    """User SQLAlchemy Repository."""

    model_type = Role

I get type checker errors (basedpyright) about

/home/void/Projects/Python/chatpyre/chatpyre/domain/accounts/test_repositories.py
  /home/void/Projects/Python/chatpyre/chatpyre/domain/accounts/test_repositories.py:9:7 - error: Variables defined in abstract base class are not initialized in final class "RoleRepository"
    Instance variable "id_attribute" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized
    Instance variable "statement" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized
    Instance variable "session" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized
    Instance variable "auto_expunge" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized
    Instance variable "auto_refresh" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized
    Instance variable "auto_commit" is defined in abstract base class "SQLAlchemyAsyncRepositoryProtocol" but not initialized (reportUninitializedInstanceVariable)
1 error, 0 warnings, 0 notes

Here is where these classes are defined: https://github.com/litestar-org/advanced-alchemy/blob/main/advanced_alchemy/repository/_async.py

As you can see even though id_attribute is not initialized in the SQLAlchemyAsyncRepositoryProtocol it is being initialized in SQLAlchemyAsyncRepository which SQLAlchemyAsyncSlugRepository also inherits from.

So I don't understand why this is an error.

vikigenius commented 4 days ago

There is a weirdness here. When I remove the @final decorator it no longer complains about reportUninitializedInstanceVariable why is this?

vikigenius commented 4 days ago

Even more weirdly, this minimal example does not cause any errors

from typing import Protocol, TypeVar, final, override, runtime_checkable

T = TypeVar("T")

@runtime_checkable
class AProto(Protocol[T]):
    a: int
    t: T

class A(AProto[T]):
    a: int = 4

    def __init__(self, t: T):
        self.t: T = t

@runtime_checkable
class ABProto(AProto[T], Protocol[T]):
    """BA Protocol."""
    def get_by_t(self, t: T) -> T: ...

class AB(A[T], ABProto[T]):
    """Testing."""
    @override
    def get_by_t(self, t: T):
        return self.t

@final
class CBA(AB[int]):
    c = 4

Even though it seems equivalent. Pinging @cofin to take a look.

DetachHead commented 4 days ago

There is a weirdness here. When I remove the @final decorator it no longer complains about reportUninitializedInstanceVariable why is this?

(based)pyright treats classes decorated with @final as though they need to implement all members, because it's impossible for a subclass to do so. if you attempt to instantiate the RoleRepository without the @final decorator you will see an error there instead stating that the class is still abstract and therefore cannot be instantiated

not sure why exactly it's occurring here if those attributes are actually implemented in the base class, so i'd need a minimal repro to investigate further

vikigenius commented 3 days ago

@DetachHead I have a minimal example now

from typing import Protocol, TypeVar, final, override, runtime_checkable

T = TypeVar("T")

@runtime_checkable
class EProto(Protocol[T]):
    et: T

    def get_e(self) -> int: ...

class E(EProto[T]):
    et: T

    @override
    def get_e(self) -> int:
        return 0

@runtime_checkable
class AProto(EProto[T], Protocol[T]):
    a: int
    at: T

class A(AProto[T], E[T]):
    a: int = 4
    at: T

    def __init__(self, at: T):
        self.at = at

@runtime_checkable
class ABProto(AProto[T], Protocol[T]):
    """BA Protocol."""
    def get_by_t(self, t: T) -> T: ...

class AB(A[T], ABProto[T]):
    """Testing."""
    @override
    def get_by_t(self, t: T):
        return self.at

@final
class CBA(AB[int]):
    et = 4
    c = 4

CBA complains about

/home/void/Projects/Python/chatpyre/chatpyre/artest.py
  /home/void/Projects/Python/chatpyre/chatpyre/artest.py:45:7 - error: Variables defined in abstract base class are not initialized in final class "CBA"
    Instance variable "a" is defined in abstract base class "AProto" but not initialized
    Instance variable "at" is defined in abstract base class "AProto" but not initialized (reportUninitializedInstanceVariable)
1 error, 0 warnings, 0 notes

a is definitely initialized in A which CBA does inherit from indirectly

This seems very close to the previous minimal example I had that produced no errors, except for one thing. I am inheriting multiple protocols.

How is it that an unrelated protocol is causing a change in behavior here?

DetachHead commented 3 days ago

thanks. i was able to minify it even further:

from typing import Protocol, final

class EProto(Protocol):
    a: int

class E(EProto):
    a: int = 1

class AProto(EProto, Protocol):
    at: int

class A(AProto, E):
    at: int = 1

@final
class AB(A, AProto): ...