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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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.


[flake8]
ignore = E203, E266, E501, W503, F811, N802, N806
Expand Down Expand Up @@ -41,6 +42,7 @@ exclude_lines =
raise NotImplementedError
omit =
src/idom/__main__.py
src/idom/mypy.py
src/idom/core/_fixed_jsonpatch.py

[build_sphinx]
Expand Down
86 changes: 86 additions & 0 deletions src/idom/mypy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
from __future__ import annotations

from typing import Any, Callable, Final

from mypy.errorcodes import ErrorCode
from mypy.nodes import ArgKind
from mypy.plugin import FunctionContext, Plugin
from mypy.types import CallableType, Instance, Type


KEY_IN_RENDER_FUNC_ERROR: Final = ErrorCode(
"idom-key-in-render-func",
"Component render function has reserved 'key' parameter",
"IDOM",
)

NO_STAR_ARGS_ERROR: Final = ErrorCode(
"idom-no-star-args",
"Children were passed using *args instead of as a list or tuple",
"IDOM",
)


class MypyPlugin(Plugin):
"""MyPy plugin for IDOM"""

def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
self._idom_component_function_names: set[str] = set()

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.

return self._idom_component_decorator_hook
elif fullname in self._idom_component_function_names:
return self._idom_component_function_hook
return super().get_function_hook(fullname)

def _idom_component_function_hook(self, ctx: FunctionContext) -> CallableType:
if any(ArgKind.ARG_STAR in arg_kinds for arg_kinds in ctx.arg_kinds):
ctx.api.msg.fail(
"Cannot pass variable number of children to component using *args",
ctx.context,
code=NO_STAR_ARGS_ERROR,
)
ctx.api.msg.note(
"You can pass a sequence of children directly to a component. That is, "
"`example(*children)` will be treated the same as `example(children)`. "
'Bear in mind that all children in such a sequence must have a "key" '
"that uniquely identifies each child amongst its siblings.",
ctx.context,
code=NO_STAR_ARGS_ERROR,
)
return ctx.default_return_type

def _idom_component_decorator_hook(self, ctx: FunctionContext) -> CallableType:
if not ctx.arg_types or not ctx.arg_types[0]:
return ctx.default_return_type

render_func: CallableType = ctx.arg_types[0][0]

if render_func.definition:
self._idom_component_function_names.add(render_func.definition.fullname)

if render_func.argument_by_name("key") is not None:
ctx.api.msg.fail(
"Component render function has reserved 'key' parameter",
ctx.context,
code=KEY_IN_RENDER_FUNC_ERROR,
)
return ctx.default_return_type

component_symbol = self.lookup_fully_qualified("idom.core.component.Component")
assert component_symbol is not None
assert component_symbol.node is not None

return render_func.copy_modified(
arg_kinds=render_func.arg_kinds + [ArgKind.ARG_NAMED_OPT],
arg_names=render_func.arg_names + ["key"],
arg_types=render_func.arg_types + [ctx.api.named_generic_type("str", [])],
ret_type=Instance(component_symbol.node, []),
)


def plugin(version: str):
# ignore version argument if the plugin works with all mypy versions.
return MypyPlugin
9 changes: 9 additions & 0 deletions temp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from idom import component


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


temp(*[1, 2])
Comment on lines +1 to +9
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