-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
[WIP] Add get_function_decorator_plugin_hook to plugin interface #9925
base: master
Are you sure you want to change the base?
[WIP] Add get_function_decorator_plugin_hook to plugin interface #9925
Conversation
@freundTech I'd love to see this merged, is there anything I can do to push this forward? |
I think the code is mostly finished and just needs some cleanup. I posted this as WIP, because I wanted some opinions on the "additional considerations" section first. Then a maintainer has to approve this. I'll link this in the typing gitter to see if I can get some people to have a look at it. |
@freundTech maybe you can send a message on the mailing list as well 😊 https://mail.python.org/archives/list/typing-sig@python.org/ |
Any progress on this PR? |
I think my last comment still stands. I haven't received any feedback on if this feature is needed/wanted/the best way to solve #9911 yet. If there is consensus that this should be added I can rebase it and clean up the code. |
I have a use case where I need this, so I'd love to see it merged as well 🙏 |
Co-authored-by: Sigurd Ljødal <544451+ljodal@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
1 similar comment
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Description
Closes #9915
This PR adds the plugin hook I proposes in #9915.
Test Strategy
Two new unit tests have been added to test the new plugin hook
Q&A
Why is this plugin hook needed?:
This hook can be used to solve problems like for example #9911, which can't be solved by existing plugin hooks.
There is already a plugin hook for handling class decorators, but none for method and function decorators.
Why is there no
get_method_decorator_plugin_hook
?:From what I can tell the semantic analysis phase doesn't distinguish between functions and methods. Therefor only one hook has been added. The other hooks, which have
function
andmethod
variants run during the checking phase.What can this hook do?
Mark decorated functions as properties, abstract, final and coroutines
What can't this hook do?
This hook currently can't disable type checking for decorated functions, despite the builtin '@typing.no_type_check' decorator being able to do so. Enabling this would require more code changes, as a local variable is used for this and would have to be exposed in some way.
When does this hook run?
This hook runs after the builtin code has handled the decorators, but before handled decorators are removed and the decorated function is semantically analyzed.
Additional considerations
SematicAnalyzer.check_decorated_function_is_method
on theSemanticAnalyzerPluginInterface