Error in subclass of generic base at None

[gnprice]

Under --strict-optional, when inheriting from a generic base class applied to None, method implementations on the subclass can produce spurious errors like so:

$ cat /tmp/foo.py
from typing import Generic, TypeVar

T = TypeVar('T')

class Base(Generic[T]):
  def f(self) -> T:
    pass

class SubNone(Base[None]):
  def f(self) -> None:
    pass

class SubInt(Base[int]):
  def f(self) -> int:
    return 1

$ mypy /tmp/foo.py --strict-optional
/tmp/foo.py: note: In class "SubNone":
/tmp/foo.py:10: error: Return type of "f" incompatible with supertype "Base"

Note there's no error when the same thing is done with int instead of None. There's also no error without --strict-optional.

This issue causes about 109 error messages when type-checking mypy itself under --strict-optional, in various subclasses of NodeVisitor[None] and TypeVisitor[None].

I haven't tried hard to reduce this repro -- it's possible that some of the elements of it are unnecessary.

[gnprice]

Initial results from debugging: this is a mismatch between certain code using Void and other code using NoneTyp.

When we report this error, the stack looks like this:

  /home/gregprice/w/mypy/mypy/checker.py(201)accept()
-> typ = node.accept(self)
  /home/gregprice/w/mypy/mypy/nodes.py(476)accept()
-> return visitor.visit_func_def(self)
  /home/gregprice/w/mypy/mypy/checker.py(402)visit_func_def()
-> self.check_method_override(defn)
  /home/gregprice/w/mypy/mypy/checker.py(758)check_method_override()
-> self.check_method_or_accessor_override_for_base(defn, base)
  /home/gregprice/w/mypy/mypy/checker.py(767)check_method_or_accessor_override_for_base()
-> self.check_method_override_for_base_with_name(defn, name, base)
  /home/gregprice/w/mypy/mypy/checker.py(803)check_method_override_for_base_with_name()
-> defn)
> /home/gregprice/w/mypy/mypy/checker.py(863)check_override()
-> name, name_in_super, supertype, node)
  /home/gregprice/w/mypy/mypy/messages.py(664)return_type_incompatible_with_supertype()
-> .format(name, target), context)

and the issue is that the arguments override and original to check_override differ in that original.ret_type is a NoneTyp while override.ret_type is a Void.

That, in turn, comes because in the caller:

(Pdb) ll
778         def check_method_override_for_base_with_name(
779                 self, defn: FuncBase, name: str, base: TypeInfo) -> None:
780             base_attr = base.names.get(name)
781             if base_attr:
782                 # The name of the method is defined in the base class.
783     
784                 # Construct the type of the overriding method.
785                 typ = self.method_type(defn)
786                 # Map the overridden method type to subtype context so that
787                 # it can be checked for compatibility.
788                 original_type = base_attr.type
789                 if original_type is None and isinstance(base_attr.node,
790                                                         FuncDef):
791                     original_type = self.function_type(base_attr.node)
792                 if isinstance(original_type, FunctionLike):
793                     original = map_type_from_supertype(
794                         method_type(original_type),
795                         defn.info, base)
796                     # Check that the types are compatible.
797                     # TODO overloaded signatures
798                     self.check_override(typ,
799                                         cast(FunctionLike, original),
800                                         defn.name(),
801                                         name,
802                                         base.name(),
803  ->                                     defn)
804                 else:
805                     assert original_type is not None
806                     self.msg.signature_incompatible_with_supertype(
807                         defn.name(), name, base.name(), defn)

when we applied map_type_from_supertype, it translated the type variable into a NoneTyp, while defn.type.ret_type was a Void all along and that carries through to typ.ret_type.

I'm not sure whether the immediate right fix is more Void on one side or more NoneTyp on the other.

In the commit message of #1562 "Pr/strict optional", I see

    - "None" in type annotations refers to NoneTyp, except as a return
      value, where it still refers to Void

So it looks like what's happening is that the return type of the new definition is duly a Void, but the return type from the base is a type variable and when that type variable is instantiated as NoneTyp it goes along in becoming NoneTyp. Seems like one possible quick-fix hack could be to have map_type_from_supertype translate NoneTyp to Void when it sees it in a return type; but I don't think I fully understand the context here.

@ddfisher, can you fill in more of the story around that remark in the commit message? Do you have thoughts on how best to fix this, either for a quick fix to unblock further progress or for the longer term?

[gnprice]

Side conclusion from that bit of debugging: the reason the error message is confusing is that it's rendering both a Void and a NoneTyp as "None". They're unequal and don't match each other, but are printed identically. That's pretty much inevitably a cause for puzzlement and pain.

Quite possibly the ultimate solution will be to eliminate Void in favor of NoneTyp, or even NoneTyp as well in favor of just another Instance, per #1278 and #1847. In the interim, we should do something to prevent these "foo does not match foo" error messages. I'll file a separate issue for that.

[gnprice]

This will probably be resolved by doing #1975.