Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

brianschubert
Copy link
Collaborator

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)

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

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]

Copy link
Collaborator

@hauntsaninja hauntsaninja left a 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

Alternatively, we could prune further the set of expressions it applies in (e.g. remove it from return, yield, assert statements)

@harahu
Copy link

harahu commented Oct 24, 2024

In general, func-returns-value check is a little lint-y

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 ruff project is building their own type checker these days.

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.

I wonder if we should move it to an optional error code

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency for [func-returns-value]: Error not reported for staticmethods
3 participants