Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygoodAlexWaygood commented Jul 28, 2023

Mypy's --warn-return-any check flags when a function is declared to return a specific type, but mypy can't verify whether it's actually returning that type or not -- it can only infer a vague, unsafe Any type as the returned type. This can often lead to bugs slipping beneath the radar.

In the case of argument clinic, the check only flags a single function. But, turns out that it's a true positive! The function is incorrectly annotated at the moment -- the annotation says that it returns a FunctionType, but that's not true. The function creates a FunctionTypeinstancefn, and then returns whatever calling fn returns returns. Therefore, the proper return annotation for this function is -> Any, not -> FunctionType.

Closes#104050!

@erlend-aasland
Copy link
Contributor

Good sleuthing!

*,
filename: str='-'
) ->FunctionType:
) ->Any:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a comment as to why Any is the correct annotation here?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

You weren't going to make finally closing that issue easy, were you? ;)

Let me know if a5103be is too verbose...!

Copy link
MemberAuthor

@AlexWaygoodAlexWaygoodJul 28, 2023

Choose a reason for hiding this comment

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

Hmm, actually, now that I look at it again, the comment feels like it duplicates the docstring a little bit... not sure if it's worth it? I'll defer to you on whether it's helpful or not!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, you're right. The docstring should be enough!

Copy link
MemberAuthor

@AlexWaygoodAlexWaygoodJul 29, 2023

Choose a reason for hiding this comment

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

Okay -- removed the comment again and tweaked the docstring slightly! How's it look now?

Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

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

Yay!

@AlexWaygoodAlexWaygood merged commit 87de2fb into python:mainJul 29, 2023
@AlexWaygoodAlexWaygood deleted the clinic-return-any branch July 29, 2023 12:39
@bedevere-bot

This comment was marked as off-topic.

Sign up for freeto 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.

Add type annotations to clinic.py

3 participants

@AlexWaygood@erlend-aasland@bedevere-bot