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

Provide more context about why incompatible with supertype is an error #5705

Closed
JukkaL opened this issue Oct 1, 2018 · 7 comments
Closed

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 1, 2018

Users often don't realize that overriding a method with an incompatible signature is unsafe from a type checking perspective; see #1237 for an example. We could generate some more descriptive notes with the error message, perhaps with a link to mypy documentation what contains a detailed discussion of Liskov Substitution Principle. We could also discuss how to work around the issue.

#5704 and #5025 are related issues.

@ckarnell
Copy link
Contributor

@JukkaL I would be happy to take a stab at this if I get a couple extra cycles soon, if were a little more clear exactly what we want here. What should the notes be exactly? Or should I just wing it?

@adhibhuta
Copy link

Hey @ckarnell are you still working on it?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Mar 20, 2020

Here's a proposal for the notes that could be added:

prog.py:7: error: Argument 1 of "f" is incompatible with supertype "C"; supertype defines the argument type as "Optional[int]"
prog.py:7: note: This violates the Liskov substitution principle
prog.py:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides

@ckarnell
Copy link
Contributor

Hi, just want to drop a note that I haven't gotten a chance to work on this.

@ChetanKhanna
Copy link
Contributor

Hi, if no one's working on it, maybe I can?
And jump-starting to it, can someone let me know which file should define these new error/note messages? Is it one of the functions in mypy/messages.py that needs to be edited?

@msullivan
Copy link
Collaborator

Yeah, feel free to take a shot. Probably the messages are in mypy/messages.py, yeah. It is possible that you'll need to gather more information from the call sites (probably in checker.py) to produce a good message.

ChetanKhanna added a commit to ChetanKhanna/mypy that referenced this issue May 20, 2020
Added notes as suggested in the issue description after error from
incompatible arguments in subtype is raised.
@ChetanKhanna
Copy link
Contributor

Here's a proposal for the notes that could be added:

prog.py:7: error: Argument 1 of "f" is incompatible with supertype "C"; supertype defines the argument type as "Optional[int]"
prog.py:7: note: This violates the Liskov substitution principle
prog.py:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides

I made a PR with the notes suggested here

@JukkaL JukkaL closed this as completed in 05571b1 May 22, 2020
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

5 participants