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

mypy not flagging subclasses with incompatible constructors #6967

Open
lbenezriravin opened this issue Jun 11, 2019 · 7 comments
Open

mypy not flagging subclasses with incompatible constructors #6967

lbenezriravin opened this issue Jun 11, 2019 · 7 comments

Comments

@lbenezriravin
Copy link

reproducer

from typing import Type
class Foo:
    def __init__(self, a):
        self.a = a

class Bar(Foo):
    def __init__(self):
        super().__init__("Bar")


def takes_foo(foo: Type[Foo]):
    x = foo("Foo")


takes_foo(Foo)
takes_foo(Bar)

observed behavior
mypy raises no errors, but the code raises a TypeError at runtime.

expected behavior
pep 484 states:

when new_user() calls user_class() this implies that all subclasses of User must support this in their constructor signature...A type checker ought to flag violations of such assumptions, but by default constructor calls that match the constructor signature in the indicated base class (User in the example above) should be allowed.

My understanding of the above is that mypy should be raising an error on the Bar definition because its signature is incompatible with that of its superclass. More generally, I tend to be surprised when code that passes mypy raises a TypeError at runtime.

environment
master as of 2019-06-10

$ pipenv graph
mypy==0.710+dev.e2f31ed71bd1edd60bffc86d3fda9da15ba63b3d
  - mypy-extensions [required: >=0.4.0,<0.5.0, installed: 0.4.1]
  - typed-ast [required: >=1.4.0,<1.5.0, installed: 1.4.0]
$ pipenv run python --version
Python 3.7.3

Thank you all for an amazing tool!

@refi64
Copy link
Contributor

refi64 commented Jun 11, 2019

When a function has no type annotations at all, it's considered untyped, as in this case. If you add a -> None to the end, then it'll be typed and you'll get an error. There are also command line options to show untyped defs (--disallow-untyped-defs?).

@lbenezriravin
Copy link
Author

Thanks for the response! Ah, that was silly of me to leave out the annotations on the __init__. However, adding type annotations hasn't helped in this case. The following still raises no errors:

from typing import Type

class Foo:
    def __init__(self, a: str) -> None:
        self.a = a

class Bar(Foo):
    def __init__(self) -> None:
        super().__init__('Bar')


def takes_foo(foo: Type[Foo]) -> None:
    foo('Foo')

takes_foo(Foo)
takes_foo(Bar)

@ilevkivskyi
Copy link
Member

This is actually deliberate decision, mypy specifically whitelists __init__, __new__, and __init_subclass__ from LSP checks because incompatible constructor overrides is such a widespread idiom in Python (see however #3823).

Maybe we can add the stricter checks beyond flag, but OTOH we already have lots of flags. Maybe we can add a selection of stricter checks under the umbrella of --strict flag (which is currently just a selection of other flags)?

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 11, 2019

If we'd add an option for this, I think it would fit better under optional error codes that can enabled (this applies to some existing strictness related flags). This will be possible once we have support for error codes.

Since object() takes no arguments, we'd need to special case it somehow or no __init__ methods can accept any arguments.

@gin-ahirsch
Copy link

gin-ahirsch commented Nov 25, 2019

Maybe a decorator can be added to typing to allow a kind of "opt-in" or "opt-out" on a case-by-case basis. Something like typing.inherit_signature() and typing.different_signature(). I don't like these names, but I can't think of anything better right now.

typing.inherit_signature() would signal the requirement for a type's __init__() signature to be compatible with super().__init__(), such that def __init__() for Bar in the posted example would fail to type-check.

Using typing.different_signature() would instead make takes_foo(Bar) fail, because the signature for Bar.__init__() is annotated to differ from Foo.__init__(), but it would allow the method to exist without failing the type-check.
In the example the signatures really do not match, but I think if this annotation was added type-checking should also fail if the signatures happened to match, because the annotation signals that this is not intended, meaning it should not be relied upon.

@ilevkivskyi
Copy link
Member

A decorator is also possible, but IMO having an error code to enable this check using a unified mechanism of enabling error codes would be better in this case.

IIRC @JukkaL wanted to add such general mechanism, but it's not clear when we will time to work on this.

@gin-ahirsch
Copy link

I concur the flag is useful, but I think the decorators would still be a useful addition.

@AlexWaygood AlexWaygood added the topic-inheritance Inheritance and incompatible overrides label Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants