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

add mypy plugin #825

Closed
wants to merge 2 commits into from
Closed

add mypy plugin #825

wants to merge 2 commits into from

Conversation

rmorshea
Copy link
Collaborator

@rmorshea rmorshea commented Oct 20, 2022

Partially addresses: #794
closes: #616
closes: #615

This works around the problem that the @component decorator hides the signature of the underlying function. Also check for improperly passing children via *args

All you need to do to run this is configure MyPy with:

[mypy]
plugins = idom.mypy

Checklist

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.
  • GitHub Issues which may be closed by this Pull Request have been linked.

@rmorshea rmorshea requested a review from Archmonger October 20, 2022 06:33
"""MyPy plugin for IDOM"""

def get_function_hook(self, fullname: str) -> Callable[[FunctionContext], Type]:
if fullname == "idom.core.component.component":
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we allowed to do an from idom import component and then have this check be fullname == f"{component.__module}.{component.__name__}"?

Would improve refactorability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MyPy automatically resolves the full name of the function in question. Even if component is imported under an alias or from a different location the name is still idom.core.component.component.

Copy link
Contributor

@Archmonger Archmonger Oct 20, 2022

Choose a reason for hiding this comment

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

Yes, however, if we decide to change the module's path from idom.core.component.component in the future, this plugin will break silently.

I would recommend using python imports to generate that path, that way refactoring libraries such as rope can automatically update these paths if they change in the future. Or at the very least, this plugin would create a visible exception to demonstrate that things are broken.

Copy link
Contributor

@Archmonger Archmonger left a comment

Choose a reason for hiding this comment

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

This is not the ideal solution, as it prioritizes MyPy over any other type checkers (ex pylance).

We will need to create an issue to revisit this when Python 3.11 becomes mainstream with better type hinting support

Also, test seem broken for potentially unrelated reasons.

@@ -6,6 +6,7 @@ ignore_missing_imports = True
warn_unused_configs = True
warn_redundant_casts = True
warn_unused_ignores = True
plugins = idom.mypy
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to document this somehow.

Undocumented features essentially do not exist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is more in a draft state. I'm not even sure this belongs in idom. It might be better placed in an idom-typing package.

Copy link
Contributor

Choose a reason for hiding this comment

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

A separate package makes a lot of sense. If/when created we should document that it exists within idom docs.

@rmorshea
Copy link
Collaborator Author

This is not the ideal solution, as it prioritizes MyPy over any other type checkers (ex pylance).

I realize that, but there simply isn't any way to annotate what @component does with the typing tools Python provides right now. PyRight, the type checker in PyLance, also has no support for plugins and it doesn't seem like there will be any time soon.

Other very large projects have already opted to write MyPy plugins. For example, Numpy and Pydantic. So I don't think it's too crazy for us to do the same.

We will need to create an issue to revisit this when Python 3.11 becomes mainstream

Typically, even if typing tools are included in newer versions of Python, they can still be used through typing_extensions. This is how we use newer features like Protocol and ParamSpec even though those were not released in Python 3.7 (the oldest version of Python we support). But again, there are no typing PEPs that have been accepted thus far that would allow us to appropriately annotate what @component does, especially after we make this change.

Also, test seem broken for potentially unrelated reasons.

This is addressed in #823

@rmorshea rmorshea linked an issue Dec 1, 2022 that may be closed by this pull request
1 task
Comment on lines +1 to +9
from idom import component


@component
def temp(x: int, y: int) -> None:
return None


temp(*[1, 2])
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to put temp.py into the gitignore if you frequently use it for testing

@Archmonger
Copy link
Contributor

Archmonger commented Dec 7, 2022

I have been experimenting with the new OpenAI Public Beta to see if it can auto complete this PR.

Input 1

Create a MyPy plugin that only warns if the unpacking operator (* or **) is used within an idom.html.* function call of within a decorated function. This must only apply to functions decorated with @component. The @component decorator path must resolve to idom.core.component.component

Output 1

import ast
from typing import Any, Dict, List, Tuple

from mypy.plugin import FunctionContext, MethodContext, Plugin
from mypy.types import CallableType

def is_idom_html_function(func_name: str) -> bool:
    return func_name.startswith('idom.html.')

def is_component_decorated(decorators: List[ast.AST]) -> bool:
    if not decorators:
        return False
    return any(
        isinstance(decorator, ast.Name) and decorator.id == 'component'
        for decorator in decorators
    )

class IdomMypyPlugin(Plugin):
    def get_function_hook(self, fullname: str) -> Optional[Callable[[FunctionContext], Type]]:
        if fullname == 'idom.core.component.component':
            return function_hook

    def function_hook(ctx: FunctionContext) -> Type:
        sig = ctx.default_return_type
        if not isinstance(sig, CallableType):
            return sig
        if not is_component_decorated(ctx.func.decorators):
            return sig
        if not sig.is_var_arg:
            return sig
        ctx.api.warn_unused('W001', ctx.context, ctx.func, 'Unpacking operator used in decorated function')
        return sig

Input 2

Create a MyPy plugin that only warns if the unpacking operator (* or **) is used within an idom.html.* function call of within a decorated function. This must only apply to functions decorated with @component. The @component decorator path must resolve to idom.core.component.component.

Make sure that this only checks idom.html.* function calls that exist within a function decorated with @component.

Output 2a

from mypy import api
from mypy.nodes import StarExpr, DictExpr, TypeInfo, CallExpr, Any
from typing import Callable, Tuple

def idom_html_check(unit: Any) -> None:
    for name, node in unit.names.items():
        if isinstance(node, TypeInfo) and node.fullname == "idom.core.component.component":
            # Check for decorated functions with the @component decorator
            for func in node.functions():
                if func.decorators:
                    # Check for calls to idom.html.* functions within decorated functions
                    for call in func.calls:
                        if call.callee.fullname.startswith("idom.html."):
                            # Check for unpacking operators (* or **) in the function call
                            for arg in call.args:
                                if isinstance(arg, (StarExpr, DictExpr)) and hasattr(arg, "dots"):
                                    warning_message = f"Unpacking operator used in call to {call.callee.fullname}"
                                    print(warning_message)

def plugin(version: str) -> Tuple[str, Callable[[str], None]]:
    return "idom_html_check", idom_html_check

Output 2b

from typing import Callable, Tuple, Type

from mypy.nodes import Any, CallExpr, DictExpr, StarExpr, TypeInfo
from mypy.plugin import Plugin


class IDOMHTMLPlugin(Plugin):
    def __init__(self):
        super().__init__()

    def get_function_hook(self, fullname: str) -> Callable[[CallExpr], None]:
        if fullname.startswith("idom.html."):

            def idom_html_check(call: CallExpr) -> None:
                # Check for unpacking operators (* or **) in the function call
                for arg in call.args:
                    if isinstance(arg, (StarExpr, DictExpr)) and hasattr(arg, "dots"):
                        warning_message = (
                            f"Unpacking operator used in call to {call.callee.fullname}"
                        )
                        print(warning_message)

            return idom_html_check

    def get_type_analyze_hook(self, fullname: str) -> Callable[[TypeInfo], None]:
        if fullname == "idom.core.component.component":

            def idom_html_check(node: TypeInfo) -> None:
                # Check for decorated functions with the @component decorator
                for func in node.funcs.values():
                    if func.decorators:
                        for call in func.calls:
                            # Check for calls to idom.html.* functions within decorated functions
                            if call.callee.fullname.startswith("idom.html."):
                                # Call the hook for idom.html.* function calls
                                self.get_function_hook(call.callee.fullname)(call)

            return idom_html_check

    def get_plugins(self) -> Tuple[str, Tuple[Type[Any], Type[Any]]]:
        return ("idom_html_check", (IDOMHTMLPlugin,))

@rmorshea
Copy link
Collaborator Author

Gonna close this. We can return to this for reference when the time comes.

@rmorshea rmorshea closed this Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants