Skip to content

Commit

Permalink
add options to customize hook/component patterns
Browse files Browse the repository at this point in the history
  • Loading branch information
rmorshea committed Sep 5, 2022
1 parent e816dde commit 64b7b17
Show file tree
Hide file tree
Showing 17 changed files with 451 additions and 252 deletions.
41 changes: 38 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ tox

# Errors

`ROH2**` errors can be enabled with the `--exhaustive-hook-deps` flag or setting
`exhaustive_hook_deps = True` in your `flake8` config.

<table>
<tr>
<th>Code</th>
Expand Down Expand Up @@ -63,3 +60,41 @@ tox
</td>
</tr>
</table>

# Options

All options my be used as CLI flags where `_` characters are replaced with `-`. For
example, `exhaustive_hook_deps` would become `--exhaustive-hook-deps`.

<table>
<tr>
<th>Option</th>
<th>Type</th>
<th>Default</th>
<th>Description</th>
</tr>
<tr>
<td><code>exhaustive_hook_deps</code></td>
<td>Boolean</td>
<td><code>False</code></td>
<td>Enable <code>ROH2**</code> errors (recommended)</td>
</tr>
<tr>
<td><code>component_decorator_pattern</code></td>
<td>Regex</td>
<td><code>^(component|[\w\.]+\.component)$</code></td>
<td>
The pattern which should match the component decorators. Useful if
you import the <code>@component</code> decorator under an alias.
</td>
</tr>
<tr>
<td><code>hook_function_pattern</code></td>
<td>Regex</td>
<td><code>^_*use_\w+$</code></td>
<td>
The pattern which should match the name of hook functions. Best used if you
have existing functions with <code>use_*</code> names that are not hooks.
</td>
</tr>
</table>
4 changes: 3 additions & 1 deletion flake8_idom_hooks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@
from .flake8_plugin import Plugin
from .run import run_checks

__all__ = ["Plugin", "run_checks"]
plugin = Plugin()

__all__ = ["plugin", "run_checks"]
51 changes: 51 additions & 0 deletions flake8_idom_hooks/common.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
from __future__ import annotations

import ast
import re
from contextlib import contextmanager
from typing import Any, Iterator


@contextmanager
def set_current(obj: Any, **attrs: Any) -> Iterator[None]:
old_attrs = {k: getattr(obj, f"_current_{k}") for k in attrs}
for k, v in attrs.items():
setattr(obj, f"_current_{k}", v)
try:
yield
finally:
for k, v in old_attrs.items():
setattr(obj, f"_current_{k}", v)


class CheckContext:
def __init__(
self, component_decorator_pattern: str, hook_function_pattern: str
) -> None:
self.errors: list[tuple[int, int, str]] = []
self._hook_function_pattern = re.compile(hook_function_pattern)
self._component_decorator_pattern = re.compile(component_decorator_pattern)

def add_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_def(self, node: ast.FunctionDef) -> bool:
return self.is_hook_name(node.name)

def is_hook_name(self, name: str) -> bool:
return self._hook_function_pattern.match(name) is not None

def is_component_def(self, node: ast.FunctionDef) -> bool:
return any(map(self.is_component_decorator, node.decorator_list))

def is_component_decorator(self, node: ast.AST) -> bool:
deco_name_parts: list[str] = []
while isinstance(node, ast.Attribute):
deco_name_parts.insert(0, node.attr)
node = node.value
if isinstance(node, ast.Name):
deco_name_parts.insert(0, node.id)
return (
self._component_decorator_pattern.match(".".join(deco_name_parts))
is not None
)
29 changes: 14 additions & 15 deletions flake8_idom_hooks/exhaustive_deps.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import ast
from typing import Optional, Set, Union

from .utils import ErrorVisitor, is_component_def, is_hook_def, set_current
from .common import CheckContext, set_current

HOOKS_WITH_DEPS = ("use_effect", "use_callback", "use_memo")


class ExhaustiveDepsVisitor(ErrorVisitor):
def __init__(self) -> None:
super().__init__()
self._current_function: Optional[ast.FunctionDef] = None
class ExhaustiveDepsVisitor(ast.NodeVisitor):
def __init__(self, context: CheckContext) -> None:
self._context = context
self._current_hook_or_component: Optional[ast.FunctionDef] = None

def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
if is_hook_def(node) or is_component_def(node):
if self._context.is_hook_def(node) or self._context.is_component_def(node):
with set_current(self, hook_or_component=node):
self.generic_visit(node)
elif self._current_hook_or_component is not None:
Expand Down Expand Up @@ -53,10 +52,10 @@ def visit_Call(self, node: ast.Call) -> None:
elif isinstance(called_func, ast.Attribute):
called_func_name = called_func.attr
else: # pragma: no cover
return None
return

if called_func_name not in HOOKS_WITH_DEPS:
return None
return

func: Optional[ast.expr] = None
args: Optional[ast.expr] = None
Expand Down Expand Up @@ -101,6 +100,7 @@ def _check_hook_dependency_list_is_exhaustive(
variables_defined_in_scope = top_level_variable_finder.variable_names

missing_name_finder = _MissingNameFinder(
self._context,
hook_name=hook_name,
func_name=func_name,
dep_names=dep_names,
Expand All @@ -113,8 +113,6 @@ def _check_hook_dependency_list_is_exhaustive(
else:
missing_name_finder.visit(func.body)

self.errors.extend(missing_name_finder.errors)

def _get_dependency_names_from_expression(
self, hook_name: str, dependency_expr: Optional[ast.expr]
) -> Optional[Set[str]]:
Expand All @@ -129,7 +127,7 @@ def _get_dependency_names_from_expression(
# 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(
self._context.add_error(
200,
elt,
(
Expand All @@ -143,7 +141,7 @@ def _get_dependency_names_from_expression(
isinstance(dependency_expr, (ast.Constant, ast.NameConstant))
and dependency_expr.value is None
):
self._save_error(
self._context.add_error(
201,
dependency_expr,
(
Expand All @@ -156,16 +154,17 @@ def _get_dependency_names_from_expression(
return set()


class _MissingNameFinder(ErrorVisitor):
class _MissingNameFinder(ast.NodeVisitor):
def __init__(
self,
context: CheckContext,
hook_name: str,
func_name: str,
dep_names: Set[str],
ignore_names: Set[str],
names_in_scope: Set[str],
) -> None:
super().__init__()
self._context = context
self._hook_name = hook_name
self._func_name = func_name
self._ignore_names = ignore_names
Expand All @@ -179,7 +178,7 @@ def visit_Name(self, node: ast.Name) -> None:
if node_id in self._dep_names:
self.used_deps.add(node_id)
else:
self._save_error(
self._context.add_error(
202,
node,
(
Expand Down
60 changes: 49 additions & 11 deletions flake8_idom_hooks/flake8_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
from flake8.options.manager import OptionManager

from flake8_idom_hooks import __version__
from flake8_idom_hooks.run import run_checks
from flake8_idom_hooks.run import (
DEFAULT_COMPONENT_DECORATOR_PATTERN,
DEFAULT_HOOK_FUNCTION_PATTERN,
run_checks,
)

from .exhaustive_deps import HOOKS_WITH_DEPS


class Plugin:
Expand All @@ -15,26 +21,58 @@ class Plugin:
version = __version__

exhaustive_hook_deps: bool
component_decorator_pattern: str
hook_function_pattern: str

@classmethod
def add_options(cls, option_manager: OptionManager) -> None:
def add_options(self, option_manager: OptionManager) -> None:
option_manager.add_option(
"--exhaustive-hook-deps",
action="store_true",
default=False,
help=f"Whether to check hook dependencies for {', '.join(HOOKS_WITH_DEPS)}",
dest="exhaustive_hook_deps",
parse_from_config=True,
)
option_manager.add_option(
"--component-decorator-pattern",
nargs="?",
default=DEFAULT_COMPONENT_DECORATOR_PATTERN,
help=(
"The pattern which should match the component decorators. "
"Useful if you import the component decorator under an alias."
),
dest="component_decorator_pattern",
parse_from_config=True,
)
option_manager.add_option(
"--hook-function-pattern",
nargs="?",
default=DEFAULT_HOOK_FUNCTION_PATTERN,
help=(
"The pattern which should match the name of hook functions. Best used "
"if you have existing functions with 'use_*' names that are not hooks."
),
dest="hook_function_pattern",
parse_from_config=True,
)

@classmethod
def parse_options(cls, options: Namespace) -> None:
cls.exhaustive_hook_deps = getattr(options, "exhaustive_hook_deps", False)

def __init__(self, tree: ast.Module) -> None:
self._tree = tree
def parse_options(self, options: Namespace) -> None:
self.exhaustive_hook_deps = options.exhaustive_hook_deps
self.component_decorator_pattern = options.component_decorator_pattern
self.hook_function_pattern = options.hook_function_pattern

def run(self) -> list[tuple[int, int, str, type[Plugin]]]:
def __call__(self, tree: ast.Module) -> list[tuple[int, int, str, type[Plugin]]]:
return [
error + (self.__class__,)
for error in run_checks(self._tree, self.exhaustive_hook_deps)
for error in run_checks(
tree,
self.exhaustive_hook_deps,
self.component_decorator_pattern,
self.hook_function_pattern,
)
]

def __init__(self) -> None:
# Hack to convince flake8 to accept plugins that are instances
# see: https://github.com/PyCQA/flake8/pull/1674
self.__init__ = self.__call__ # type: ignore
26 changes: 10 additions & 16 deletions flake8_idom_hooks/rules_of_hooks.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
import ast
from typing import Optional, Union

from .utils import (
ErrorVisitor,
is_component_def,
is_hook_def,
is_hook_function_name,
set_current,
)
from .common import CheckContext, set_current


class RulesOfHooksVisitor(ErrorVisitor):
def __init__(self) -> None:
super().__init__()
class RulesOfHooksVisitor(ast.NodeVisitor):
def __init__(self, context: CheckContext) -> None:
self._context = context
self._current_hook: Optional[ast.FunctionDef] = None
self._current_component: Optional[ast.FunctionDef] = None
self._current_function: Optional[ast.FunctionDef] = None
Expand All @@ -21,7 +15,7 @@ def __init__(self) -> None:
self._current_loop: Union[None, ast.For, ast.While] = None

def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
if is_hook_def(node):
if self._context.is_hook_def(node):
self._check_if_hook_defined_in_function(node)
with set_current(
self,
Expand All @@ -32,7 +26,7 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
loop=None,
):
self.generic_visit(node)
elif is_component_def(node):
elif self._context.is_component_def(node):
with set_current(
self,
component=node,
Expand Down Expand Up @@ -70,7 +64,7 @@ def _visit_loop(self, node: ast.AST) -> None:
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)
self._context.add_error(100, node, msg)

def _check_if_propper_hook_usage(
self, node: Union[ast.Name, ast.Attribute]
Expand All @@ -80,12 +74,12 @@ def _check_if_propper_hook_usage(
else:
name = node.attr

if not is_hook_function_name(name):
if not self._context.is_hook_name(name):
return None

if self._current_hook is None and self._current_component is None:
msg = f"hook {name!r} used outside component or hook definition"
self._save_error(101, node, msg)
self._context.add_error(101, node, msg)

loop_or_conditional = self._current_conditional or self._current_loop
if loop_or_conditional is not None:
Expand All @@ -99,4 +93,4 @@ def _check_if_propper_hook_usage(
}
node_name = node_type_to_name[node_type]
msg = f"hook {name!r} used inside {node_name}"
self._save_error(102, node, msg)
self._context.add_error(102, node, msg)
Loading

0 comments on commit 64b7b17

Please sign in to comment.