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

no error when contextmanager decorator is above staticmethod decorator #15911

Closed
DetachHead opened this issue Aug 19, 2023 · 6 comments
Closed
Labels

Comments

@DetachHead
Copy link
Contributor

from collections.abc import Iterator
from contextlib import contextmanager


class Foo:
    @contextmanager
    @staticmethod
    def foo() -> Iterator[None]:
        yield None

    def bar(self):
        with self.foo():
            ...


Foo().bar()

playground

at runtime the following error occurs:

TypeError: Foo.foo() takes 0 positional arguments but 1 was given
@DetachHead DetachHead added the bug mypy got something wrong label Aug 19, 2023
@AlexWaygood
Copy link
Member

I'm not sure how mypy could catch this error without special-casing @contextmanager, and I don't think this comes up often enough for there to be a compelling reason to do so. In general, type checkers don't promise to catch every possible error that occurs at runtime.

@AlexWaygood AlexWaygood added feature and removed bug mypy got something wrong labels Aug 19, 2023
@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2023
@DetachHead
Copy link
Contributor Author

@AlexWaygood i still think these kind of errors are mypy's responsibility. if you look at any other language, things like this are usually implemented using keywords rather than decorators. and their type checkers will complain if you put them in the wrong order. take typescript for example:

declare class Foo {
    static public foo(): void // error: 'public' modifier must precede 'static' modifier.
}

@erictraut
Copy link

Mypy is doing the right thing here based on the type information it's given. I don't think this is a bug in mypy. At best, it's a bug in typeshed or a missing feature in the type system.

I agree with Alex that this isn't an issue that deserves special-casing in the type checker logic because it's not a common source of bugs.

This isn't at all similar to the TypeScript example because static and public are keywords, and it's considered a syntax error to use them in the wrong order.

@DetachHead
Copy link
Contributor Author

This isn't at all similar to the TypeScript example because static and public are keywords, and it's considered a syntax error to use them in the wrong order.

I don't think its considered a syntax error because it can be suppressed, still compiles and runs with no runtime error. So in a way typescript is even less obligated to complain than mypy is.

I don't understand the inconsistent approach around special casing. For example isinstance is special cased to work around missing functionality in TypeGuard. I don't get how that's any different?

@DetachHead
Copy link
Contributor Author

also, why does mypy complain about decorators in the wrong order in some cases but not others?

from abc import abstractmethod
from typing import Callable, TypeVar

Fn = TypeVar("Fn", bound=Callable[..., object])

def decorator(fn: Fn) -> Fn:
    return fn

class Foo:
    @abstractmethod # no error
    @property
    def foo(self) -> int:
        return 1
    
    @decorator # error: Decorators on top of @property are not supported  [misc]
    @property
    def bar(self) -> int:
        return 1

either it should be consistent and have errors for all decorators, or none at all. the current behavior is extremely confusing

@KotlinIsland
Copy link
Contributor

@AlexWaygood @erictraut

Interestingly, if we re-implement staticmethod by hand, mypy correctly reports the error here.

So it seems that mypys dedicated special-casing of staticmethod is causing this issue to be incorrectly not reported.

from collections.abc import Callable
from typing import Generic, ParamSpec, TypeVar, overload

P = ParamSpec("P")
T = TypeVar("T")
R_co = TypeVar("R_co", covariant=True)


def contextmanager(fn: Callable[P, R_co]) -> Callable[P, R_co]:
    def func(*args: P.args, **kwargs: P.kwargs) -> R_co:
        return fn(*args, **kwargs)
    return func


class static(Generic[P, R_co]):
    @property
    def __func__(self) -> Callable[P, R_co]: ...
    @property
    def __isabstractmethod__(self) -> bool: ...
    def __init__(self, __f: Callable[P, R_co]) -> None: ...
    @overload
    def __get__(self, __instance: None, __owner: type) -> Callable[P, R_co]: ...
    @overload
    def __get__(self, __instance: T, __owner: type[T] | None = None) -> Callable[P, R_co]: ...
    __name__: str
    __qualname__: str
    @property
    def __wrapped__(self) -> Callable[P, R_co]: ...
    def __call__(self, *args: P.args, **kwargs: P.kwargs) -> R_co: ...


class Foo:
    @contextmanager
    @static
    def foo() -> None: ...

    @contextmanager
    @staticmethod
    def bar() -> None: ...


Foo().foo()  # error: Attribute function "foo" with type "Callable[[], None]" does not accept self argument  [misc]
Foo().bar()  # no error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants