Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Remove reliance on camel case #14

Merged
merged 9 commits into from
Sep 15, 2022
Merged
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
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