Unexpected error where Generic return type is None

[neiljp]

I introduced this problem on gitter, and am posting this as an issue after confirmation there.

The code below is a much-simplified piece of code to explore if strict-optional could be valid for some recent improved typing of request.pyi in current zulip.
(zulip/zulip@42f5eea)

The simplified code below produces the error:
error: "X" does not return a value

I thought this was just with strict-optional, but the reduced code gives this issue without too, so I think I must have lost the extra code which caused that difference.

from typing import TypeVar, Optional
T = TypeVar('T')
def X(val: T) -> T: ...
x_in = None
def Y(x: Optional[str] = X(x_in)): ...

The last line is not required specifically, for example the following also triggers the error in the same way:

xx: Optional[int] = X(x_in)

[gvanrossum]

So what you're really saying here is that, while we complain about using the return value of a function explicitly declared as -> None, we should not complain if a None return type was the result of expanding a type variable that happens to be None.

I'm not sure I really like that though. Why should generics be special here? Maybe we should just stop complaining about functions with -> None too? At least in strict-optional mode -- after all we'll get other complaints if the None is unacceptable for the target.

[elazarg]

Dropping this check sounds good, except that Any lvalues will lose an important error.

[neiljp]

The idea that generics aren't special seems more consistent. In a more general sense, my surprise perhaps come down to the asymmetry of:

def none() -> None:
    return None
def string() -> str:
    return "5"
works=string() # type: Optional[str]
fails=none()   # type: Optional[str]

[gvanrossum]

I'm willing to live without the error if the target/context has type Any.

[ilevkivskyi]

I don't think we should remove this error completely even with --strict-optional. If a user writes something like this:

def f() -> None:
    # some complex function
    ...
# 1000 lines later
x: Optional[str] = f()

Then it is probably a mistake. Why would one write this instead of just x: Optional[int] = None? I think a warning like f() always returns None makes sense for such cases. But I agree that we don't need this for inferred None i.e. in cases where f not always returns None.

[gvanrossum]

I still fee like mypy is slightly overreaching here. The code is type-safe. There may be reasons to write code this way (perhaps some kind of repeated pattern where an exception would be distracting, or a plan to return something else in the future).

[elazarg]

We can add another flag for that, or provide a Void type.

[ilevkivskyi]

There is another possibility: for situations that are not errors, but are suspicious, maybe we can have a concept of warning (as opposed to error) that will print some message, but will exit with 0? IIRC there is such level in errors.py but it is not used currently.

[neiljp]

I've been looking at this again while continuing to work on strict-optional support in Zulip; for some reason these errors currently don't surface running with the extra parameters used in the Zulip mypy script, but do when strict-optional is enabled.

I'm open to another solution, and perhaps there is another issue masking this, but there are now multiple errors (38) arising from this issue in Zulip, where mypy sees various code like the simplified snippet:

  1 from typing import Optional, TypeVar
  2 
  3 T = TypeVar('T')
  4 
  5 def X(default: T) -> T: ...
  6 
  7 def func(param: Optional[str]=X(default=None)) -> None: ...
  8 def func_(param: Optional[str]=X(default="")) -> None: ...
  9 def func2(param: Optional[int]=X(default=5)) -> None: ...
 10 def func3(param: str=X(default="")) -> None: ...

mypy only errors on line 7; this feels like reasonable code, if a little strange out of context.
(in Zulip, X is REQ, and accepts various parameters)

[gvanrossum]

I'm beginning to think this is important -- for now labeling it as "normal" priority (which better than no priority label for sure). I don't really like warnings (that's just an invitation to ignore them). So I think there are approaches:

1. Stop treating -> None special
2. Special-case -> None as the expansion of -> T for a type variable that's None in context

I still like (2) best.

[neiljp]

After a bit more work today, I wanted to clarify that while I do think this should be retained as an issue, with appropriate type-hinting I have managed to bypass this - assuming my approach is considered reasonable within Zulip.

Essentially, by telling X that there is another piece of type information for T in addition to None, then mypy expands the type (presumably to Optional[T]), which it then considers reasonable. This also caught some typing issues, so this issue definitely needs to be handled carefully if it is changed, as it may not have found them otherwise.

[ilevkivskyi]

Note that #3295 gives another example where this behaviour is unwanted -- overloads. Currently, I would propose to just special case this error to only be shown if the return type is None as originally annotated. If it is a result of any kind of type inference (generic or overload variants), we suppress it.

[ilevkivskyi]

I am raising priority to high because it keeps coming and this error is quite annoying because mypy is complaining about perfectly valid code.