Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol with property returning ContextManager fails validation #4034

Closed
mjpieters opened this issue Oct 11, 2022 · 2 comments
Closed

Protocol with property returning ContextManager fails validation #4034

mjpieters opened this issue Oct 11, 2022 · 2 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@mjpieters
Copy link

mjpieters commented Oct 11, 2022

Describe the bug

When defining a Protocol with a @property that returns a ContextManager referencing the protocol itself, a concrete implementation fails to validate against this. If no @property is involved, the issue goes away.

To Reproduce

from __future__ import annotations
from typing import Any, ContextManager, Generic, Protocol, TypeVar


_FooProtocol = TypeVar("_FooProtocol", bound="FooProtocol")
_FooPropProtocol = TypeVar("_FooPropProtocol", bound="FooPropProtocol")
_Foo = TypeVar("_Foo", bound="Foo")
_FooProp = TypeVar("_FooProp", bound="FooProp")


class FooProtocol(Protocol):
    def some_cm(self: _FooProtocol) -> ContextManager[_FooProtocol]: ...
class FooPropProtocol(Protocol):
    @property
    def some_cm(self: _FooPropProtocol) -> ContextManager[_FooPropProtocol]: ...

class Foo():
    def some_cm(self: _Foo) -> ContextManager[_Foo]: ...
class FooProp():
    @property
    def some_cm(self: _FooProp) -> ContextManager[_FooProp]: ...


def accepts_foo_protocol(foo: FooProtocol) -> None: ...
def accepts_fooprop_protocol(foo: FooPropProtocol) -> None: ...


accepts_foo_protocol(Foo())   # accepted
accepts_fooprop_protocol(FooProp())   # fails

The error produced is:

Argument of type "FooProp" cannot be assigned to parameter "foo" of type "FooPropProtocol" in function "accepts_fooprop_protocol"
  "FooProp" is incompatible with protocol "FooPropProtocol"
    "some_cm" is an incompatible type
      Type "(self: _FooProp@some_cm) -> ContextManager" cannot be assigned to type "(self: _FooPropProtocol@some_cm) -> ContextManager"
        Parameter 1: type "_FooPropProtocol@some_cm" cannot be assigned to type "_FooProp@some_cm"
          Type "_FooPropProtocol@some_cm" cannot be assigned to type "FooProp"
        Function return type "ContextManager" is incompatible with type "ContextManager"
          "contextlib.AbstractContextManager" is incompatible with "contextlib.AbstractContextManager"

Expected behavior

FooProp() should be accepted as a valid argument for accepts_fooprop_protocol()

VS Code extension or command-line

Tested with VS Code extension v1.1.274

Additional context

The real-world project is the SQLAlchemy typing stubs, where the following code fails to validate:

from typing import TYPE_CHECKING

from sqlalchemy.ext.asyncio import async_scoped_session
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.ext.asyncio import create_async_engine
from sqlalchemy.orm import sessionmaker

if TYPE_CHECKING:
    from sqlalchemy.ext.asyncio.session import TypingAsyncSessionProtocol

engine = create_async_engine(...)
SessionType: type[TypingAsyncSessionProtocol] = AsyncSession
SM = sessionmaker(engine, class_=SessionType)

TypingAsyncSessionProtocol.no_autoflush is a property returning a context manager.

The work-around is to use Self from PEP 673 instead of TypeVar()s with bounds. However, PEP 673 support is not yet available in mypy.

Another work-around is to drop the bounds from the typevars for _FooPropProtocol and _FooProp.

@erictraut erictraut added the bug Something isn't working label Oct 12, 2022
@mjpieters mjpieters changed the title Protocol with property returning ContextManager fails validate Protocol with property returning ContextManager fails validation Oct 12, 2022
erictraut pushed a commit that referenced this issue Nov 19, 2022
…g for a protocol that includes a property with a getter whose `self` parameter is annotated with a TypeVar. This addresses #4034.
@erictraut
Copy link
Collaborator

This will be addressed in the next release.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Nov 19, 2022
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.281, which I just published. It will also be included in a future release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants