Skip to content

Unexpected error where Generic return type is None  #4214

Closed

Description

@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)

Activity

  1. gvanrossum commented on Nov 7, 2017

    @gvanrossum
    Member

    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.

  2. elazarg commented on Nov 7, 2017

    @elazarg
    Contributor

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

  3. neiljp commented on Nov 8, 2017

    @neiljp
    Author

    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]
  4. gvanrossum commented on Nov 8, 2017

    @gvanrossum
    Member

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

  5. ilevkivskyi commented on Nov 13, 2017

    @ilevkivskyi
    Member

    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.

  6. gvanrossum commented on Nov 13, 2017

    @gvanrossum
    Member
  7. elazarg commented on Nov 13, 2017

    @elazarg
    Contributor

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

  8. ilevkivskyi commented on Nov 13, 2017

    @ilevkivskyi
    Member

    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.

  9. neiljp commented on Dec 23, 2017

    @neiljp
    Author

    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)

  10. gvanrossum commented on Dec 23, 2017

    @gvanrossum
    Member

    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.

  11. neiljp commented on Dec 24, 2017

    @neiljp
    Author

    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.

  12. ilevkivskyi commented on Jun 15, 2018

    @ilevkivskyi
    Member

    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.

  13. ilevkivskyi commented on Sep 21, 2018

    @ilevkivskyi
    Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions