-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[red-knot] Support cast
#15413
[red-knot] Support cast
#15413
Conversation
|
I think reusing |
crates/red_knot_python_semantic/resources/mdtest/directives/cast.md
Outdated
Show resolved
Hide resolved
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 good to me now, thanks! Just a couple more small nits
crates/red_knot_python_semantic/resources/mdtest/directives/cast.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/directives/cast.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/directives/cast.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <[email protected]>
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.
Thank you!
## Summary Simplification follow-up to #15413. There's no need to have a dedicated `CallOutcome` variant for every known function, it's only necessary if the special-cased behavior of the known function includes emitting extra diagnostics. For `typing.cast`, there's no such need; we can use the regular `Callable` outcome variant, and update the return type according to the cast. (This is the same way we already handle `len`.) One reason to avoid proliferating unnecessary `CallOutcome` variants is that currently we have to explicitly add emitting call-binding diagnostics, for each outcome variant. So we were previously wrongly silencing any binding diagnostics on calls to `typing.cast`. Fixing this revealed a separate bug, that we were emitting a bogus error anytime more than one keyword argument mapped to a `**kwargs` parameter. So this PR also adds test and fix for that bug. ## Test Plan Existing `cast` tests pass unchanged, added new test for `**kwargs` bug.
Summary
Resolves #15379.
Test Plan
Markdown tests.