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

Reword the error message related to void functions #15876

Merged
merged 3 commits into from
Sep 2, 2023

Conversation

atugushev
Copy link
Contributor

Fixes #3226.

Aims to provide better assistance to users who may be confused when their void functions technically return None.

Aims to provide better assistance to users who may be confused
when their void functions technically return None.
@github-actions

This comment has been minimized.

mypy/messages.py Outdated
Comment on lines 1028 to 1030
message = "{} does not return a value (or returns None)".format(
"Function" if name is None else capitalize(name)
)
Copy link
Contributor

@ikonst ikonst Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)"

Copy link
Contributor Author

@atugushev atugushev Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. :)

Copy link
Contributor

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.)

Copy link
Contributor Author

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:

mypy/mypy/messages.py

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dceaac1

Copy link
Contributor

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>
@github-actions

This comment has been minimized.

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 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.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2023

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]

@hauntsaninja
Copy link
Collaborator

I committed the wording tweak, thanks again for this PR! :-)

@hauntsaninja hauntsaninja merged commit 1655b0c into python:master Sep 2, 2023
19 checks passed
@atugushev atugushev deleted the void-return branch September 3, 2023 13:31
@atugushev
Copy link
Contributor Author

Thanks @hauntsaninja!

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

Successfully merging this pull request may close these issues.

Docs: confirm that use of None-typed function return value is disallowed
3 participants