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

False positive PLE0302 with special methods as properties. #15572

Open
mikeshardmind opened this issue Jan 18, 2025 · 4 comments · May be fixed by #15582
Open

False positive PLE0302 with special methods as properties. #15572

mikeshardmind opened this issue Jan 18, 2025 · 4 comments · May be fixed by #15582
Labels
bug Something isn't working rule Implementing or modifying a lint rule

Comments

@mikeshardmind
Copy link

mikeshardmind commented Jan 18, 2025

Example code that works at runtime and which type checkers do understand

Code sample in pyright playground

from collections.abc import AsyncGenerator, Generator, Callable, Coroutine
from typing import Any
import concurrent.futures as cf

type AENTER = Callable[[], Coroutine[Any, Any, AsyncGenerator[Any | None]]]
type AEXIT = Callable[[type[BaseException] | None, BaseException | None, object], Coroutine[Any, Any, bool]]

class ACTX[Y]:
    def __init__(self, g: AsyncGenerator[Y], f: cf.Future[None]) -> None:
        self.g = g
        self.f = f

    async def __aenter__(self) -> AsyncGenerator[Y]:
        return self.g

    async def __aexit__(
        self,
        exc_type: type[BaseException] | None,
        exc_value: BaseException | None,
        traceback: object,
    ) -> bool:
        if exc_value is not None:
            self.f.set_exception(exc_value)

        await self.g.aclose()
        return False


async def _a(ctx: ACTX[Any]) -> list[Any]:
    async with ctx as c:
        return [v async for v in c]
    return []


class ExecuteWrapper:
    def __init__(self, _c: ACTX[Any]):
        self._c = _c

    def __await__(self) -> Generator[Any, Any, list[Any]]:
        return (yield from _a(self._c).__await__())

    @property
    def __aenter__(self):
        return self._c.__aenter__
    
    @property
    def __aexit__(self):
        return self._c.__aexit__

    async def fetchone(self) -> Any | None:
        async with self as g:
            async for val in g:
                return val
        return None

Rewriting ExecuteWrapper as:

from operator import attrgetter

class ExecuteWrapper:
    def __init__(self, _c: ACTX[Any]):
        self._c = _c

    def __await__(self) -> Generator[Any, Any, list[Any]]:
        return (yield from _a(self._c).__await__())

    __aenter__: AENTER = property(attrgetter("_c.__aenter__"))  # pyright: ignore[reportAssignmentType]
    __aexit__: AEXIT = property(attrgetter("_c.__aexit__"))  # pyright: ignore[reportAssignmentType]

    async def fetchone(self) -> Any | None:
        async with self as g:
            async for val in g:
                return val
        return None

for contrast has better runtime behavior (slightly, anyhow), but unfortunately, properties are not generic in the type system, so this ends up not type-checked, though ruff has no issue with this definition.

@dylwil3 dylwil3 added bug Something isn't working rule Implementing or modifying a lint rule labels Jan 18, 2025
@dylwil3
Copy link
Collaborator

dylwil3 commented Jan 18, 2025

I think it seems reasonable to skip the check when there's a property decorator (or maybe any decorator).

@charliermarsh
Copy link
Member

I assume I'm lacking understanding. Can ExecuteWrapper not use methods for this?

    async def __aenter__(self):
        return await self._c.__aenter__()

    async def __aexit__(self, exc_type, exc_value, traceback):
        return await self._c.__aexit__(exc_type, exc_value, traceback)

@InSyncWithFoo
Copy link
Contributor

@charliermarsh That would require duplicating type hints for the parameters. The return type might not be necessary (Pyright can infer that flawlessly), but the same cannot be said about parameter types.

@mikeshardmind
Copy link
Author

mikeshardmind commented Jan 19, 2025

Using properties here has multiple benefits, from better automatic type inference to better runtime behavior. With using methods, this would be multiple awaits for a simple wrapper or require self._c.__aexit__(exc_type, exc_value, traceback).__await__() which is harder for human reviewer to reason about. Perhaps that's not the strongest argument given the existing use of similar there to define __await__ in this particular case, but in general I try to avoid writing simple coroutines that only await other coroutines with pass-through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants