python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.26k stars 2.79k forks source link

mypy thinks `raise NotImplemented` is okay #5710

Open benjaminp opened 6 years ago

benjaminp commented 6 years ago

A mistake I've seen several times is to write raise NotImplemented rather than raise NotImplementedError. mypy is silent about this bug:

$ mypy --version
mypy 0.630
$ cat example.py
def f():
    # type: () -> None
    raise NotImplemented
$ mypy example.py
$
gvanrossum commented 6 years ago

It looks like this is because in typeshed, NotImplemented is given type Any -- if it had pretty much any other type that doesn't derive from BaseException it would be flagged.

Unfortunately if we change it to what it really is (a magic singleton) then everything that returns NotImplemented will be flagged as an error unless we special-case that value in mypy (preferably only on binary operators). So this isn't a "please go fix it in typeshed" response -- we have to do two things in concert.

JukkaL commented 5 years ago

One option would be to special case only raise NotImplemented and raise NotImplemented(...) to generate an error.

gvanrossum commented 5 years ago

Honestly if we're going to special-case NotImplemented I'd prefer to special-case return NotImplemented, so we can give NotImplemented a better type than Any.

JukkaL commented 5 years ago

That's a very good point. Maybe return NotImplemented should be only valid in relavant dunder methods (or with a sufficiently permissive return type)?

gvanrossum commented 5 years ago

Right, that's what I was trying to say earlier. :-)

ilevkivskyi commented 5 years ago

A very easy but hacky way to fix this is to annotate NotImplemented as NoReturn. The latter is a subtype of al types, so it will be accepted as a return value, but it is not Any so it will be invalid in some other situations such as raising it.