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

Disallow instantiating a Protocol class #12261

Closed
adriangb opened this issue Feb 28, 2022 · 4 comments
Closed

Disallow instantiating a Protocol class #12261

adriangb opened this issue Feb 28, 2022 · 4 comments
Labels
bug mypy got something wrong topic-protocols

Comments

@adriangb
Copy link
Contributor

adriangb commented Feb 28, 2022

The following gives an error in Pyright and I think it should give an error in MyPy as well:

from typing import Protocol

class Foo(Protocol):
    ...

Foo()

image

https://mypy-play.net/?mypy=latest&python=3.10&gist=f4ca0ee490d9e8b3a117ee785b7bdaf6

I think that even if the protocol class implements an __init__ method, this should still be disallowed:

from typing import Protocol

class Foo(Protocol):
    def __init__(self) -> None:  pass

Foo()  # disallowed


class ConcreteFoo(Foo):
    ...

ConcreteFoo()  # allowed
@adriangb adriangb added the bug mypy got something wrong label Feb 28, 2022
@A5rocks
Copy link
Contributor

A5rocks commented Mar 1, 2022

My main question here would be whether this fails too:

x = Foo
x()

Because if so, it disallows perfectly valid code:

x: Foo = ConcreteFoo
x()

and if it doesn't, it's a really special case that probably doesn't help much (in my opinion. Perhaps you've run into this and think it would be worth it, which is perfectly valid). Note that mypy has historically gone the all-the-way route with abstract base classes and this prompted #5374 (#5374 (comment) for reference).

@adriangb
Copy link
Contributor Author

adriangb commented Mar 1, 2022

Interesting, that is a good point.

For your first example, Pyright flags it with the same error.
For your second example, Pyright allows it without errors.

But I don't think your second example is valid. It should be:

x: Type[Foo] = Foo

Which Pyright disallows with:

Expression of type "Type[Foo]" cannot be assigned to declared type "Type[Foo]"
"Foo" is not a concrete class type and cannot be assigned to type

For the example presented in #5374 (comment):

from typing import Protocol, Type

class Foo(Protocol):
    ...

class ConcreteFoo(Foo):
    ...


def fun(x: Type[Foo]) -> Foo:
    return x()


fun(Foo)  # error: Argument of type "Type[Foo]" cannot be assigned to parameter "x" of type "Type[Foo]" in function "fun" "Foo" is not a concrete class type and cannot be assigned to type
fun(ConcreteFoo)  # fine

So basically Pyright seems to prohibit using a protocol class as a type (even as an argument to a function). This makes sense to me: I often want to pass in a concrete class as a parameter, but I don't I ever have had a use case for passing in an abstract class as a parameter.

As additional context, the reason I brought this up is https://bugs.python.org/issue44807

from typing import Protocol

class Foo(Protocol):
    x: int
    def __init__(self, x: int) -> None:
        self.x = x


class ConcreteFoo(Foo):
    x: int

ConcreteFoo(x=1)  # ConcreteFoo.__init__() takes exactly one argument (the instance to initialize)

Currently this fails at runtime with quite confusing behavior/errors because of the monkey patching that Protocol does to __init__.
My hope was that this check could be moved from runtime monkey patching to mypy.

@Rahix
Copy link

Rahix commented Mar 17, 2022

but I don't I ever have had a use case for passing in an abstract class as a parameter.

There are usecases, see #4717.

@hauntsaninja
Copy link
Collaborator

mypy complains about the original example. Looks like this got changed a while back, in 0.950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-protocols
Projects
None yet
Development

No branches or pull requests

5 participants