-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix [func-returns-value]
not being trigged when calling decorated functions
#18015
base: master
Are you sure you want to change the base?
Fix [func-returns-value]
not being trigged when calling decorated functions
#18015
Conversation
Co-authored-by: Harald Husum <harald.husum@intelecy.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Diff from mypy_primer, showing the effect of this PR on open source code: prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/filesystems.py:491: error: Incompatible types in "await" (actual type "None", expected type "Awaitable[Any]") [misc]
+ src/prefect/filesystems.py:491: error: "get_directory" of "RemoteFileSystem" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/task_worker.py:446: error: Incompatible types in "await" (actual type "None", expected type "Awaitable[Any]") [misc]
+ src/prefect/task_worker.py:446: error: "start" of "TaskWorker" does not return a value (it only ever returns None) [func-returns-value]
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/series.py:3561: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None) [func-returns-value]
+ pandas/core/series.py:5629: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None) [func-returns-value]
+ pandas/core/frame.py:7067: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None) [func-returns-value]
+ pandas/core/frame.py:7077: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None) [func-returns-value]
+ pandas/core/frame.py:7092: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None) [func-returns-value]
spark (https://github.com/apache/spark)
+ python/pyspark/sql/profiler.py:60: error: "zero" of "PStatsParam" does not return a value (it only ever returns None) [func-returns-value]
+ python/pyspark/sql/profiler.py:60: error: "zero" of "MemUsageParam" does not return a value (it only ever returns None) [func-returns-value]
pandera (https://github.com/pandera-dev/pandera)
+ tests/core/test_decorators.py:717: error: "optional_in" does not return a value (it only ever returns None) [func-returns-value]
jax (https://github.com/google/jax)
+ jax/_src/environment_info.py:59: error: "print" does not return a value (it only ever returns None) [func-returns-value]
core (https://github.com/home-assistant/core)
+ homeassistant/components/trace/websocket_api.py:155: error: "breakpoint_set" does not return a value (it only ever returns None) [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:180: error: "breakpoint_clear" does not return a value (it only ever returns None) [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:269: error: "debug_continue" does not return a value (it only ever returns None) [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:293: error: "debug_step" does not return a value (it only ever returns None) [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:317: error: "debug_stop" does not return a value (it only ever returns None) [func-returns-value]
spack (https://github.com/spack/spack)
+ lib/spack/spack/test/util/compression.py:26: error: "update" of "MutableMapping" does not return a value (it only ever returns None) [func-returns-value]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
Hmm, that's a fair number of new primer hits.
In general, func-returns-value check is a little lint-y and probably of limited value. It looks like it dates to the pre-strict-optional days when mypy would just let None be assigned to anything, like NULL, so this would have been valuable then.
You can find a few issues complaining about this. I wonder if we should move it to an optional error code
- error "does not return a value" where method really should not return a value #6549
- error: "x" does not return a value [func-returns-value] #15785
- Cannot assert a callable returns
None
when it must returnNone
#17568 - Mypy doesn't recognize None is checked when using or operator #13725
- Error caused by print inside a list comprehension #10455
- Allow yielding result of function that returns None #1933 (comment)
- Mypy error "__ does not return a value" when using
lambda
with multiple expression #17364 - Docs: confirm that use of None-typed function return value is disallowed #3226
Alternatively, we could prune further the set of expressions it applies in (e.g. remove it from return, yield, assert statements)
There's no denying that. However, I don't think it is a bad thing for mypy to offer lint-y checks. High quality linting requires type checking, so a linter and and a type checker run separately will never be as useful as the two working in unison. I believe this is why the That said, providing a clear separation between rules that are purely about type checking and rules, like this one, that are more lint-y seems like a good idea. This applies both to when configuring mypy, as well as how mypy reports errors.
That sounds like a fair solution. I haven't looked though all the examples of people complaining, but some of them leaves me asking "why would you even want to be doing what you are trying to do". Anecdotally, I, at least, have yet to experience anything I would consider a "false positive" case of this error. So I'm not sure pruning the set of expressions is the right approach. |
Fixes #14179
@harahu previously wrote a unit test for this issue in #17834 (open). I wasn't sure how to best integrate that work, so this PR includes its own tests with @harahu added as co-author. (Happy to tweak if there's a better way to handle this)