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

[WIP] Add get_function_decorator_plugin_hook to plugin interface #9925

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

freundTech
Copy link
Contributor

@freundTech freundTech commented Jan 18, 2021

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 and method 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

  • Make it possible for plugins to disable type checking for a function
  • Expose SematicAnalyzer.check_decorated_function_is_method on the SemanticAnalyzerPluginInterface
  • Instead of having the plugin return true if it handled the decorator and it should be removed, we could assume that, if a callback is returned for a given fullname, it always handles the decorator.

@patrick91
Copy link
Contributor

@freundTech I'd love to see this merged, is there anything I can do to push this forward?

@freundTech
Copy link
Contributor Author

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.

@patrick91
Copy link
Contributor

@freundTech maybe you can send a message on the mailing list as well 😊 https://mail.python.org/archives/list/typing-sig@python.org/

@97littleleaf11
Copy link
Collaborator

Any progress on this PR?

@freundTech
Copy link
Contributor Author

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.

@ljodal
Copy link

ljodal commented May 10, 2022

I have a use case where I need this, so I'd love to see it merged as well 🙏

mypy/plugin.py Outdated Show resolved Hide resolved
test-data/unit/plugins/function_decorator_hook.py Outdated Show resolved Hide resolved
freundTech and others added 2 commits May 11, 2022 14:56
Co-authored-by: Sigurd Ljødal <544451+ljodal@users.noreply.github.com>
@freundTech freundTech marked this pull request as ready for review May 11, 2022 12:57
@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

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

Successfully merging this pull request may close these issues.

Add a plugin hook for handling function/method decorators
4 participants