-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
gh-104050: Argument clinic: enable mypy's --warn-return-any
setting
#107405
Conversation
Good sleuthing! |
@@ -4288,7 +4288,7 @@ def eval_ast_expr( | |||
globals: dict[str, Any], | |||
*, | |||
filename: str = '-' | |||
) -> FunctionType: | |||
) -> Any: |
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.
Should we add a comment as to why Any
is the correct annotation here?
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.
You weren't going to make finally closing that issue easy, were you? ;)
Let me know if a5103be is too verbose...!
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.
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!
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.
Sorry, you're right. The docstring should be enough!
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.
Okay -- removed the comment again and tweaked the docstring slightly! How's it look now?
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.
Yay!
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, unsafeAny
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 aFunctionType
instancefn
, and then returns whatever callingfn
returns returns. Therefore, the proper return annotation for this function is-> Any
, not-> FunctionType
.Closes #104050!