add exhaustive dep checking

Author: rmorshea

flake8_idom_hooks/__init__.py

@@ -0,0 +1,3 @@
+from .flake8_plugin import Plugin
+
+__all__ = ["Plugin"]

flake8_idom_hooks/exhaustive_deps.py

@@ -0,0 +1,186 @@
+import ast
+from typing import Optional, List, Union, Set
+
+from .utils import is_hook_or_element_def, ErrorVisitor
+
+
+HOOKS_WITH_DEPS = ("use_effect", "use_callback", "use_memo")
+
+
+class ExhaustiveDepsVisitor(ErrorVisitor):
+    def __init__(self) -> None:
+        super().__init__()
+        self._current_hook_or_element: Optional[ast.FunctionDef] = None
+
+    def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
+        if is_hook_or_element_def(node):
+            self._current_hook_or_element = node
+            self.generic_visit(node)
+            self._current_hook_or_element = None
+        elif self._current_hook_or_element is not None:
+            for deco in node.decorator_list:
+                if not isinstance(deco, ast.Call):
+                    continue
+
+                called_func = deco.func
+                if isinstance(called_func, ast.Name):
+                    called_func_name = called_func.id
+                elif isinstance(called_func, ast.Attribute):
+                    called_func_name = called_func.attr
+                else:  # pragma: no cover
+                    continue
+
+                if called_func_name not in HOOKS_WITH_DEPS:
+                    continue
+
+                for kw in deco.keywords:
+                    if kw.arg == "args":
+                        self._check_hook_dependency_list_is_exhaustive(
+                            called_func_name,
+                            node,
+                            kw.value,
+                        )
+                        break
+
+    def visit_Call(self, node: ast.Call) -> None:
+        if self._current_hook_or_element is None:
+            return
+
+        called_func = node.func
+
+        if isinstance(called_func, ast.Name):
+            called_func_name = called_func.id
+        elif isinstance(called_func, ast.Attribute):
+            called_func_name = called_func.attr
+        else:  # pragma: no cover
+            return None
+
+        if called_func_name not in HOOKS_WITH_DEPS:
+            return None
+
+        func: Optional[ast.expr] = None
+        args: Optional[ast.expr] = None
+
+        if len(node.args) == 2:
+            func, args = node.args
+        else:
+            if len(node.args) == 1:
+                func = node.args[0]
+            for kw in node.keywords:
+                if kw.arg == "function":
+                    func = kw.value
+                elif kw.arg == "args":
+                    args = kw.value
+
+        if isinstance(func, ast.Lambda):
+            self._check_hook_dependency_list_is_exhaustive(
+                called_func_name,
+                func,
+                args,
+            )
+
+    def _check_hook_dependency_list_is_exhaustive(
+        self,
+        hook_name: str,
+        func: Union[ast.FunctionDef, ast.Lambda],
+        dependency_expr: Optional[ast.expr],
+    ) -> None:
+        dep_names = self._get_dependency_names_from_expression(
+            hook_name, dependency_expr
+        )
+
+        if dep_names is None:
+            return None
+
+        func_name = "lambda" if isinstance(func, ast.Lambda) else func.name
+
+        visitor = _MissingNameOrAttrVisitor(
+            hook_name,
+            func_name,
+            _param_names_of_function_def(func),
+            dep_names,
+        )
+        if isinstance(func.body, list):
+            for b in func.body:
+                visitor.visit(b)
+        else:
+            visitor.visit(func.body)
+
+        self.errors.extend(visitor.errors)
+
+    def _get_dependency_names_from_expression(
+        self, hook_name: str, dependency_expr: Optional[ast.expr]
+    ) -> Optional[List[str]]:
+        if dependency_expr is None:
+            return []
+        elif isinstance(dependency_expr, (ast.List, ast.Tuple)):
+            dep_names: List[str] = []
+            for elt in dependency_expr.elts:
+                if isinstance(elt, ast.Name):
+                    dep_names.append(elt.id)
+                else:
+                    # ideally we could deal with some common use cases, but since React's
+                    # own linter doesn't do this we'll just take the easy route for now:
+                    # https://github.com/facebook/react/issues/16265
+                    self._save_error(
+                        201,
+                        elt,
+                        (
+                            f"dependency arg of {hook_name!r} is not destructured - "
+                            "dependencies should be refered to directly, not via an "
+                            "attribute or key of an object"
+                        ),
+                    )
+            return dep_names
+        else:
+            self._save_error(
+                202,
+                dependency_expr,
+                (
+                    f"dependency args of {hook_name!r} should be a literal list or "
+                    f"tuple - not expression type {type(dependency_expr).__name__!r}"
+                ),
+            )
+            return None
+
+
+class _MissingNameOrAttrVisitor(ErrorVisitor):
+    def __init__(
+        self,
+        hook_name: str,
+        func_name: str,
+        ignore_names: List[str],
+        dep_names: List[str],
+    ) -> None:
+        super().__init__()
+        self._hook_name = hook_name
+        self._func_name = func_name
+        self._ignore_names = ignore_names
+        self._dep_names = dep_names
+        self.used_deps: Set[str] = set()
+
+    def visit_Name(self, node: ast.Name) -> None:
+        node_id = node.id
+        if node_id not in self._ignore_names:
+            if node_id in self._dep_names:
+                self.used_deps.add(node_id)
+            else:
+                self._save_error(
+                    203,
+                    node,
+                    (
+                        f"dependency {node_id!r} of function {self._func_name!r} "
+                        f"is not specified in declaration of {self._hook_name!r}"
+                    ),
+                )
+
+
+def _param_names_of_function_def(func: Union[ast.FunctionDef, ast.Lambda]) -> List[str]:
+    names: List[str] = []
+    names.extend(a.arg for a in func.args.args)
+    names.extend(kw.arg for kw in func.args.kwonlyargs)
+    if func.args.vararg is not None:
+        names.append(func.args.vararg.arg)
+    if func.args.kwarg is not None:
+        names.append(func.args.kwarg.arg)
+    return names

flake8_idom_hooks/flake8_plugin.py

@@ -0,0 +1,39 @@
+import ast
+from pkg_resources import (
+    get_distribution as _get_distribution,
+    DistributionNotFound as _DistributionNotFound,
+)
+from typing import List, Tuple, Type
+
+from .utils import ErrorVisitor
+from .rules_of_hooks import RulesOfHooksVisitor
+from .exhaustive_deps import ExhaustiveDepsVisitor
+
+try:
+    __version__ = _get_distribution(__name__).version
+except _DistributionNotFound:  # pragma: no cover
+    # package is not installed
+    __version__ = "0.0.0"
+
+
+class Plugin:
+
+    name = __name__
+    version = __version__
+    options = None
+
+    _visitor_types: List[Type[ErrorVisitor]] = [
+        RulesOfHooksVisitor,
+        ExhaustiveDepsVisitor,
+    ]
+
+    def __init__(self, tree: ast.Module) -> None:
+        self._tree = tree
+
+    def run(self) -> List[Tuple[int, int, str, Type["Plugin"]]]:
+        errors = []
+        for vtype in self._visitor_types:
+            visitor = vtype()
+            visitor.visit(self._tree)
+            errors.extend(visitor.errors)
+        return [(line, col, msg, self.__class__) for line, col, msg in errors]

flake8_idom_hooks/rules_of_hooks.py

@@ -0,0 +1,90 @@
+import ast
+from contextlib import contextmanager
+from typing import Iterator, Union, Optional, Any
+
+from .utils import is_hook_or_element_def, ErrorVisitor, is_hook_function_name
+
+
+class RulesOfHooksVisitor(ErrorVisitor):
+    def __init__(self) -> None:
+        super().__init__()
+        self._current_hook_or_element: Optional[ast.FunctionDef] = None
+        self._current_function: Optional[ast.FunctionDef] = None
+        self._current_call: Optional[ast.Call] = None
+        self._current_conditional: Union[None, ast.If, ast.IfExp, ast.Try] = None
+        self._current_loop: Union[None, ast.For, ast.While] = None
+
+    def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
+        if is_hook_or_element_def(node):
+            self._check_if_hook_defined_in_function(node)
+            with self._set_current(hook_or_element=node, function=node):
+                self.generic_visit(node)
+        else:
+            with self._set_current(function=node):
+                self.generic_visit(node)
+
+    def _visit_hook_usage(self, node: Union[ast.Name, ast.Attribute]) -> None:
+        self._check_if_propper_hook_usage(node)
+
+    visit_Attribute = _visit_hook_usage
+    visit_Name = _visit_hook_usage
+
+    def _visit_conditional(self, node: ast.AST) -> None:
+        with self._set_current(conditional=node):
+            self.generic_visit(node)
+
+    visit_If = _visit_conditional
+    visit_IfExp = _visit_conditional
+    visit_Try = _visit_conditional
+
+    def _visit_loop(self, node: ast.AST) -> None:
+        with self._set_current(loop=node):
+            self.generic_visit(node)
+
+    visit_For = _visit_loop
+    visit_While = _visit_loop
+
+    def _check_if_hook_defined_in_function(self, node: ast.FunctionDef) -> None:
+        if self._current_function is not None:
+            msg = f"hook {node.name!r} defined as closure in function {self._current_function.name!r}"
+            self._save_error(100, node, msg)
+
+    def _check_if_propper_hook_usage(
+        self, node: Union[ast.Name, ast.Attribute]
+    ) -> None:
+        if isinstance(node, ast.Name):
+            name = node.id
+        else:
+            name = node.attr
+
+        if not is_hook_function_name(name):
+            return None
+
+        if self._current_hook_or_element is None:
+            msg = f"hook {name!r} used outside element or hook definition"
+            self._save_error(101, node, msg)
+
+        _loop_or_conditional = self._current_conditional or self._current_loop
+        if _loop_or_conditional is not None:
+            node_type = type(_loop_or_conditional)
+            node_type_to_name = {
+                ast.If: "if statement",
+                ast.IfExp: "inline if expression",
+                ast.Try: "try statement",
+                ast.For: "for loop",
+                ast.While: "while loop",
+            }
+            node_name = node_type_to_name[node_type]
+            msg = f"hook {name!r} used inside {node_name}"
+            self._save_error(102, node, msg)
+
+    @contextmanager
+    def _set_current(self, **attrs: Any) -> Iterator[None]:
+        old_attrs = {k: getattr(self, f"_current_{k}") for k in attrs}
+        for k, v in attrs.items():
+            setattr(self, f"_current_{k}", v)
+        try:
+            yield
+        finally:
+            for k, v in old_attrs.items():
+                setattr(self, f"_current_{k}", v)

flake8_idom_hooks/utils.py

@@ -0,0 +1,22 @@
+import ast
+from typing import List, Tuple
+
+
+class ErrorVisitor(ast.NodeVisitor):
+    def __init__(self) -> None:
+        self.errors: List[Tuple[int, int, str]] = []
+
+    def _save_error(self, error_code: int, node: ast.AST, message: str) -> None:
+        self.errors.append((node.lineno, node.col_offset, f"ROH{error_code} {message}"))
+
+
+def is_hook_or_element_def(node: ast.FunctionDef) -> bool:
+    return is_element_function_name(node.name) or is_hook_function_name(node.name)
+
+
+def is_element_function_name(name: str) -> bool:
+    return name[0].upper() == name[0] and "_" not in name
+
+
+def is_hook_function_name(name: str) -> bool:
+    return name.lstrip("_").startswith("use_")

requirements/lint.txt

@@ -1,2 +1,3 @@
 flake8 >=3.7
 black
+mypy

setup.py

@@ -16,7 +16,7 @@
 
 package = {
     "name": name,
-    "py_modules": ["flake8_idom_hooks"],
+    "py_modules": setuptools.find_packages(exclude=["tests"]),
     "entry_points": {"flake8.extension": ["ROH=flake8_idom_hooks:Plugin"]},
     "python_requires": ">=3.6",
     "description": "Flake8 plugin to enforce the rules of hooks for IDOM",