Skip to content

Inheriting from Any destroys subtype relation #2212

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

Closed
ddfisher opened this issue Oct 3, 2016 · 8 comments
Closed

Inheriting from Any destroys subtype relation #2212

ddfisher opened this issue Oct 3, 2016 · 8 comments

Comments

@ddfisher
Copy link
Collaborator

ddfisher commented Oct 3, 2016

I've been thinking a bit more about a formalism for mypy's type system and realized that allowing classes to inherit from Any is pretty problematic. When classes inherit from Any, any proper notion of subtyping goes completely out the window and you're left with only the non-transitive is-compatible-with. (Otherwise, your subtype graph has cycles.) Therefore, I think we should disallow inheriting from Any. (It's possible that there's some other patch that can fix this, but it wasn't obvious to me.)

To my knowledge, there are two ways that classes can inherit from Any: 1) explicit inheritance, and 2) due to a silent import. 1) is easy to fix -- we can just make explicitly inheriting from Any an error. 2) doesn't have an obvious of a fix, but is something I think we want to address regardless, as we're already seeing problems from silent imports being too permissive.

Here's my proposal for silent imports: instead of classes from a silently imported module being Any, they're unique, named classes that inherit only from object, but support any operation. This makes them still somewhat useful for typechecking (in that e.g. you can't pass an int to something that expects a silently imported class), but hopefully leaves them permissive enough that silent imports is still effective. We'd have to test this empirically to be sure.

@gvanrossum thoughts?

@gvanrossum
Copy link
Member

gvanrossum commented Oct 3, 2016 via email

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 3, 2016

If we import a class from an silently imported module, we lose any knowledge of the MRO of the class -- other than the fact it's of form [X, ...something..., object], where X is the imported class. Depending on the target module, it's possible that there are lots of relevant base classes in the MRO.

For example, assume that we silently import from the mypy.nodes module (this clearly isn't realistic, but similar class hierarchies are not uncommon):

from mypy.nodes import FuncDef, Node

def f(n: Node) -> None:
    ...

def g(fdef: FuncDef) -> None:
    f(fdef)  # Should be okay, since FuncDef is a subclass of Node

How would we handle with code like f and g in the above example, if we don't have the MROs of the classes? There seems to be no way of distinguishing between FuncDef being a subclass of Node, and it not being a subclass of ClassDef, for example. This is the motivation for treating something with an Any base class as compatible with anything -- there could be almost anything in the MRO (and often there is something), and it would be pretty arbitrary to assume that there is no non-trivial inheritance at all.

Okay, maybe we could treat built-in types specially, and assume that no silently imported class has one of them as a base class. But then we still have the original problem, where any silently imported class X is compatible with any other silently imported class Y, and vice versa -- there still isn't a meaningful subtyping relation between those classes.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Oct 3, 2016

Guido: we could potentially do that contextually. E.g.:

from silent_module import Foo
class C(Foo):  # Foo is understood to be a special silent imports class here
    pass
def f(x: Foo) -> None:  # Foo is understood to be the silent imports class here too
    pass

x = 1 + Foo  # Foo is treated like Any here
Foo(5)  # Foo is treated like Any here too

@ddfisher
Copy link
Collaborator Author

ddfisher commented Oct 3, 2016

Jukka: the original problem isn't just that silently imported class X is compatible with silently imported class Y. For one, silently imported class X is compatible with any class C. But the bigger problem is you can no longer talk about subtype relations given the existence of a class that inherits from Any. This change would definitely make silent import code more restrictive (and potentially too restrictive in some cases), but I think that would be better than the current situation. In the specific example you gave, you would be forced to non-silently import the mypy.nodes module, but I think that's acceptable.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 4, 2016

Yeah, the way silently imported classes work now is problematic. A simpler though partial fix would be to disallow partially imported classes in annotations. It wouldn't fix the issue with a subclass of a partially imported class being compatible with every type, though.

In order to make this idea work smoothly, it seems that we should give useful diagnostics when a compatibility check fails potentially because of silent imports. If we'd just use the existing error messages, there would be no indication what can be done to work around problems such as the example I gave above, and a user would likely be quite lost. Here's a strawman proposal for an error message:

program.py:5: error: Argument 1 to "f" has incompatible type "FuncDef"; expected "Node"
program.py:5: note: Module "mypy.nodes" which defines "FuncDef" is silently imported.
program.py:5: note: The base classes of silently imported classes are not available
program.py:5: note: for type checking. Add "mypy.nodes" to files to be type checked to get
program.py:5: note: the full MRO for "FuncDef".

This is very verbose, but it's not obvious how to make a very concise error message that explains enough for a user to understand what is going on. Maybe we can add a link to documentation?

Another issue is third-party libraries without stubs. Users currently have to silently import these. The proposed change would make silent imports less useful for library modules that use a lot of on subclassing. The mypy.nodes issue is one example of the potential issues. Maybe we can run some experiments to determine if this is likely to be a major problem.

The way silent imports work now, subtyping isn't meaningful for classes that have a silently imported base class, similar to how subtyping isn't meaningful for Any. Each such class would generally not be a subtype of anything other than other classes in their MRO, similarly how Any would likely only be a subtype of Any (and maybe also object? -- I don't remember how this should work). Still, I think that this behavior would be okay from a type system correctness point of view.

gvanrossum pushed a commit that referenced this issue Oct 4, 2016
Fixes #2212.

All that's left is some calls to in_checked_function(), which replaces
the old typing_mode_full() and typing_mode_none() (its negation after
the elimination of typing_mode_weak()).

The meaning of in_checked_function() is:

- True if --check-untyped-defs is set.
- True outside functions.
- True in annotated functions.
- False otherwise.
gvanrossum added a commit that referenced this issue Oct 4, 2016
Fixes #2212.

All that's left is some calls to in_checked_function(), which replaces
the old typing_mode_full() and typing_mode_none() (its negation after
the elimination of typing_mode_weak()).
@gvanrossum
Copy link
Member

Sorry, wrong issue.

@gvanrossum gvanrossum reopened this Oct 4, 2016
@ddfisher
Copy link
Collaborator Author

FWIW there was another instance of user confusion in one of our internal codebases around mypy missing what looked (on inspection) to be clearly a bug (passing an A where you needed a B) because of Any inheritance.

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 29, 2020

It's probably too late to change this, since there is a lot of code that implicitly depends on the current semantics.

@JukkaL JukkaL closed this as completed Jan 29, 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

3 participants