Skip to content

mypy thinks raise NotImplemented is okay #5710

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
benjaminp opened this issue Oct 1, 2018 · 7 comments
Closed

mypy thinks raise NotImplemented is okay #5710

benjaminp opened this issue Oct 1, 2018 · 7 comments

Comments

@benjaminp
Copy link

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
Copy link
Member

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
Copy link
Collaborator

JukkaL commented Oct 2, 2018

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

@gvanrossum
Copy link
Member

gvanrossum commented Oct 2, 2018 via email

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 2, 2018

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
Copy link
Member

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

@ilevkivskyi
Copy link
Member

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.

JukkaL pushed a commit that referenced this issue Oct 9, 2024
Refs #5710

Adds special-case handling for raising `NotImplemented`:
```python
raise NotImplemented  # E: Exception must be derived from BaseException; did you mean "NotImplementedError"?
```

Per the linked issue, there's some debate as to how to best handle
`NotImplemented`. This PR special-cases its behavior in `raise`
statements, whereas the leaning in the issue (at the time it was opened)
was towards updating its definition in typeshed and possibly
special-casing its use in the relevant dunder methods.

Going the typeshed/special-dunder route may still happen, but it hasn't
in the six years since the issue was opened. It would be nice to at
least catch errors from raising `NotImplemented` in the interim.

Making this change also uncovered a regression introduced in
python/typeshed#4222. Previously, `NotImplemented` was annotated as
`Any` in typeshed, so returning `NotImplemented` from a non-dunder would
emit an error when `--warn-return-any` was used:
```python
class A:
    def some(self) -> bool: return NotImplemented  # E: Returning Any from function declared to return "bool"
```
However, in python/typeshed#4222, the type of `NotImplemented` was
updated to be a subclass of `Any`. This broke the handling of
`--warn-return-any`, but it wasn't caught since the definition of
`NotImplemented` in `fixtures/notimplemented.pyi` wasn't updated along
with the definition in typeshed. As a result, current mypy doesn't emit
an error here
([playground](https://mypy-play.net/?mypy=1.11.2&python=3.12&flags=strict%2Cwarn-return-any&gist=8a78e3eb68b0b738f73fdd326f0bfca1)),
despite having a test case saying that it should. As a bandaid, this PR
add special handling for `NotImplemented` in return statements to treat
it like an explicit `Any`, restoring the pre- python/typeshed#4222
behavior.
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 9, 2024

Fixed in #17890.

@JukkaL JukkaL closed this as completed Oct 9, 2024
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

4 participants