-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Reword the error message related to void functions #15876
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
Conversation
Aims to provide better assistance to users who may be confused when their void functions technically return None.
This comment has been minimized.
This comment has been minimized.
mypy/messages.py
Outdated
| message = "{} does not return a value (or returns None)".format( | ||
| "Function" if name is None else capitalize(name) | ||
| ) |
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.
| message = "{} does not return a value (or returns None)".format( | |
| "Function" if name is None else capitalize(name) | |
| ) | |
| name = capitalize(name or "Function") | |
| message = f"{name} does not return a value (or returns None)" |
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.
| message = "{} does not return a value (or returns None)".format( | |
| "Function" if name is None else capitalize(name) | |
| ) | |
| name = "Function" if name is None else capitalize(name) | |
| message = f"{name} does not return a value (or returns None)" |
Thanks for the suggestion! By using an f-string along with the name variable the code definitely looks better. One note, perhaps we could keep an explicit check for None as in the original code?
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.
Of course. :)
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.
(I "simplified" it because an empty string wouldn't make for a good interpolation either, but it also suggests it's a possibility where currently it's not.)
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.
Considering the implementation of callable_name() it seems to me that we could omit the capitalization:
Lines 2963 to 2967 in b49be10
| def callable_name(type: FunctionLike) -> str | None: | |
| name = type.get_name() | |
| if name is not None and name[0] != "<": | |
| return f'"{name}"'.replace(" of ", '" of "') | |
| return name |
It always returns either None, "Foo", or <Foo>, where capitalization does not seem to be applicable. What do you think?
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.
Addressed in dceaac1
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.
Looks much better!
Co-authored-by: Ilya Priven <ilya.konstantinov@gmail.com>
This comment has been minimized.
This comment has been minimized.
hauntsaninja
left a comment
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 this PR! I'd vote for a slightly more specific phrase, something like Error: "f" does not return a value (it only ever returns None)
I think this is less likely to confuse users who either don't understand return semantics or from the message think mypy doesn't understand return semantics.
|
Diff from mypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)
- steam/protobufs/__init__.py:77: error: "setattr" does not return a value [func-returns-value]
+ steam/protobufs/__init__.py:77: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/protobufs/__init__.py:78: error: "setattr" does not return a value [func-returns-value]
+ steam/protobufs/__init__.py:78: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/gateway.py:165: error: "info" of "Logger" does not return a value [func-returns-value]
+ steam/gateway.py:165: error: "info" of "Logger" does not return a value (it only ever returns None) [func-returns-value]
- steam/gateway.py:185: error: "info" of "Logger" does not return a value [func-returns-value]
+ steam/gateway.py:185: error: "info" of "Logger" does not return a value (it only ever returns None) [func-returns-value]
- steam/ext/csgo/protobufs/__init__.py:18: error: "setattr" does not return a value [func-returns-value]
+ steam/ext/csgo/protobufs/__init__.py:18: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/ext/tf2/protobufs/__init__.py:17: error: "setattr" does not return a value [func-returns-value]
+ steam/ext/tf2/protobufs/__init__.py:17: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/filesystems.py:494: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:494: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:587: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:587: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:723: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:723: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:821: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:821: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:393: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:393: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:395: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:395: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:397: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:397: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:399: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:399: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/engine.py:1451: error: Function does not return a value [func-returns-value]
+ src/prefect/engine.py:1451: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/flow_run_notification_policies.py:47: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/flow_run_notification_policies.py:47: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/configuration.py:29: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/configuration.py:29: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/concurrency_limits_v2.py:94: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/concurrency_limits_v2.py:94: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/concurrency_limits_v2.py:217: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/concurrency_limits_v2.py:217: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/agents.py:39: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/agents.py:39: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/services/scheduler.py:210: error: "rollback" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/services/scheduler.py:210: error: "rollback" of "Session" does not return a value (it only ever returns None) [func-returns-value]
streamlit (https://github.com/streamlit/streamlit)
- lib/tests/streamlit/elements/slider_test.py:200:9: error: "assertEqual" of "TestCase" does not return a value [func-returns-value]
+ lib/tests/streamlit/elements/slider_test.py:200:9: error: "assertEqual" of "TestCase" does not return a value (it only ever returns None) [func-returns-value]
discord.py (https://github.com/Rapptz/discord.py)
- discord/channel.py:660: error: Function does not return a value [func-returns-value]
+ discord/channel.py:660: error: Function does not return a value (it only ever returns None) [func-returns-value]
ibis (https://github.com/ibis-project/ibis)
- ibis/examples/gen_registry.py:75: error: "make_descriptions" does not return a value [func-returns-value]
+ ibis/examples/gen_registry.py:75: error: "make_descriptions" does not return a value (it only ever returns None) [func-returns-value]
|
|
I committed the wording tweak, thanks again for this PR! :-) |
|
Thanks @hauntsaninja! |
Fixes #3226.
Aims to provide better assistance to users who may be confused when their void functions technically return None.