Skip to content

Improve error message for overload overrides w/ swapped variants #5369

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

Conversation

Michael0x2a
Copy link
Collaborator

This commit is intended to resolve #1270.

Currently, running mypy on the following program:

from typing import overload

class Parent:
    @overload
    def foo(self, x: A) -> A: ...
    @overload
    def foo(self, x: B) -> B: ...

class Child(Parent):
    @overload
    def foo(self, x: B) -> B: ...
    @overload
    def foo(self, x: A) -> A: ...

...will make mypy report the following error -- it considers the fact that we've swapped the order of the two variants to be unsafe:

test.pyi:10: error: Signature of "foo" incompatible with supertype "Parent"

This error message can be confusing for some users who may not be aware that the order in which your overloads are defined can sometimes matter/is something mypy relies on.

This commit modifies the error message to the following to try
and make this more clear:

test.pyi:10: error: Signature of "foo" incompatible with supertype "Parent"
test.pyi:10: note: Overload variants must be defined in the same order as they are in "Parent"

This commit is intended to resolve python#1270.

Currently, running mypy on the following program:

    from typing import overload

    class Parent:
        @overload
        def foo(self, x: A) -> A: ...
        @overload
        def foo(self, x: B) -> B: ...

    class Child(Parent):
        @overload
        def foo(self, x: B) -> B: ...
        @overload
        def foo(self, x: A) -> A: ...

...will make mypy report the following error -- it considers the fact
that we've swapped the order of the two variants to be unsafe:

    test.pyi:10: error: Signature of "foo" incompatible with supertype "Parent"

This error message can be confusing for some users who may not be
aware that the order in which your overloads are defined can sometimes
matter/is something mypy relies on.

This commit modifies the error message to the following to try
and make this more clear:

    test.pyi:10: error: Signature of "foo" incompatible with supertype "Parent"
    test.pyi:10: note: Overload variants must be defined in the same order as they are in "Parent"
@Michael0x2a
Copy link
Collaborator Author

As a misc note, one idea the issue I linked to above mentioned was that we could maybe print out the signature of the parent method. I decided not to do this for now because it felt inconsistent: we don't do this in other cases where the user incorrectly overrides a method.

If printing out the parent signature seems like a helpful thing to do in general, I'm happy to make a new PR w/ that change.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This additional note makes sense. Also I think it is right to only show in in obvious cases.

@ilevkivskyi ilevkivskyi merged commit bfb47e1 into python:master Aug 2, 2018
@Michael0x2a Michael0x2a deleted the refine-overload-overloading-error-message branch August 2, 2018 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ordering matters for @overload
2 participants