From 8320bd3e4f984732161662e6c28134132c63228b Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Thu, 28 Oct 2021 18:14:58 -0700 Subject: [PATCH 1/9] Remove reliance on CamelCase component names --- flake8_idom_hooks/utils.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index 14e501e..a4adfcc 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -28,12 +28,15 @@ def is_hook_def(node: ast.FunctionDef) -> bool: def is_component_def(node: ast.FunctionDef) -> bool: - return is_component_function_name(node.name) - - -def is_component_function_name(name: str) -> bool: - return name[0].upper() == name[0] and "_" not in name + return any(decorator.value.id == "idom" for decorator in node.decorator_list) def is_hook_function_name(name: str) -> bool: - return name.lstrip("_").startswith("use_") + return name.lstrip("_") in { + "use_state", + "use_effect", + "use_memo", + "use_reducer", + "use_callback", + "use_ref", + } From 0d19b5bf8c196c890f9102c942f24ad612659ccf Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Thu, 28 Oct 2021 18:26:30 -0700 Subject: [PATCH 2/9] check for idom.component instead of just idom --- flake8_idom_hooks/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index a4adfcc..4714e86 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -28,7 +28,10 @@ def is_hook_def(node: ast.FunctionDef) -> bool: def is_component_def(node: ast.FunctionDef) -> bool: - return any(decorator.value.id == "idom" for decorator in node.decorator_list) + return any( + decorator.value.id == "idom" and decorator.attr == "component" + for decorator in node.decorator_list + ) def is_hook_function_name(name: str) -> bool: From 41b895c9de8ad9b1bbe9934d3ac6022f55ea54bc Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Wed, 3 Nov 2021 02:00:40 -0700 Subject: [PATCH 3/9] revert is_hook_function_name --- flake8_idom_hooks/utils.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index 4714e86..60448be 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -35,11 +35,4 @@ def is_component_def(node: ast.FunctionDef) -> bool: def is_hook_function_name(name: str) -> bool: - return name.lstrip("_") in { - "use_state", - "use_effect", - "use_memo", - "use_reducer", - "use_callback", - "use_ref", - } + return name.lstrip("_").startswith("use_") From 4f1ffd7f043543b8ea1b1e47b55f6449c40400ba Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Fri, 19 Aug 2022 02:18:18 -0700 Subject: [PATCH 4/9] add component decorator --- requirements/test-env.txt | 1 + tests/hook_usage_test_cases.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/requirements/test-env.txt b/requirements/test-env.txt index 9955dec..a95ee81 100644 --- a/requirements/test-env.txt +++ b/requirements/test-env.txt @@ -1,2 +1,3 @@ pytest pytest-cov +idom \ No newline at end of file diff --git a/tests/hook_usage_test_cases.py b/tests/hook_usage_test_cases.py index 29df1fd..bedf343 100644 --- a/tests/hook_usage_test_cases.py +++ b/tests/hook_usage_test_cases.py @@ -1,9 +1,14 @@ +from idom import component + + +@component def HookInIf(): if True: # error: ROH102 hook 'use_state' used inside if statement use_state +@component def HookInElif(): if False: pass @@ -12,6 +17,7 @@ def HookInElif(): use_state +@component def HookInElse(): if False: pass @@ -20,6 +26,7 @@ def HookInElse(): use_state +@component def HookInIfExp(): ( # error: ROH102 hook 'use_state' used inside inline if expression @@ -29,6 +36,7 @@ def HookInIfExp(): ) +@component def HookInElseOfIfExp(): ( None @@ -39,6 +47,7 @@ def HookInElseOfIfExp(): ) +@component def HookInTry(): try: # error: ROH102 hook 'use_state' used inside try statement @@ -47,6 +56,7 @@ def HookInTry(): pass +@component def HookInExcept(): try: raise ValueError() @@ -55,6 +65,7 @@ def HookInExcept(): use_state +@component def HookInFinally(): try: pass @@ -63,37 +74,45 @@ def HookInFinally(): use_state +@component def HookInForLoop(): for i in range(3): # error: ROH102 hook 'use_state' used inside for loop use_state +@component def HookInWhileLoop(): while True: # error: ROH102 hook 'use_state' used inside while loop use_state +@component def outer_function(): # error: ROH100 hook 'use_state' defined as closure in function 'outer_function' + @component def use_state(): ... +@component def generic_function(): # error: ROH101 hook 'use_state' used outside component or hook definition use_state +@component def use_state(): use_other +@component def Component(): use_state +@component def use_custom_hook(): use_state @@ -105,11 +124,13 @@ def use_custom_hook(): module.use_effect() +@component def not_hook_or_component(): # error: ROH101 hook 'use_state' used outside component or hook definition use_state +@component def CheckEffects(): x = 1 y = 2 @@ -166,34 +187,41 @@ def CheckEffects(): ) @use_effect(args=[x]) + @component def my_effect(): x @use_effect(args=[]) + @component def my_effect(): # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' x @use_effect(args=[]) @some_other_deco_that_adds_args_to_func_somehow + @component def my_effect(*args, **kwargs): args kwargs @module.use_effect(args=[]) + @component def my_effect(): # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' x @not_a_decorator_we_care_about + @component def some_func(): ... @not_a_decorator_we_care_about() + @component def some_func(): ... @use_effect + @component def impropper_usage_of_effect_as_decorator(): # ignored because bad useage x @@ -210,8 +238,10 @@ def impropper_usage_of_effect_as_decorator(): ) +@component def make_component(): # nested component definitions are ok. + @component def NestedComponent(): use_state @@ -219,6 +249,7 @@ def NestedComponent(): some_global_variable +@component def Component(): # referencing a global variable is OK use_effect(lambda: some_global_variable, []) @@ -226,9 +257,11 @@ def Component(): if True: + @component def Component(): # this is ok since the conditional is outside the component use_state + @component def use_other(): use_state From b92d10668cc6c23549716d5627dd2935bf20162e Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Fri, 19 Aug 2022 02:53:22 -0700 Subject: [PATCH 5/9] fix tests --- flake8_idom_hooks/utils.py | 10 ++++++++-- tests/hook_usage_test_cases.py | 18 +++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index 60448be..175d599 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -29,8 +29,14 @@ def is_hook_def(node: ast.FunctionDef) -> bool: def is_component_def(node: ast.FunctionDef) -> bool: return any( - decorator.value.id == "idom" and decorator.attr == "component" - for decorator in node.decorator_list + is_idom_component_decorator(decorator) for decorator in node.decorator_list + ) + + +def is_idom_component_decorator(node) -> bool: + return getattr(node, "id", None) == "component" or ( + getattr(getattr(node, "value", None), "id", None) == "idom" + and getattr(node, "attr", None) == "component" ) diff --git a/tests/hook_usage_test_cases.py b/tests/hook_usage_test_cases.py index bedf343..dc6334e 100644 --- a/tests/hook_usage_test_cases.py +++ b/tests/hook_usage_test_cases.py @@ -1,3 +1,4 @@ +import idom from idom import component @@ -88,16 +89,14 @@ def HookInWhileLoop(): use_state -@component def outer_function(): # error: ROH100 hook 'use_state' defined as closure in function 'outer_function' - @component def use_state(): ... -@component def generic_function(): + # error: ROH101 hook 'use_state' used outside component or hook definition use_state @@ -112,6 +111,11 @@ def Component(): use_state +@idom.component +def IdomLongImportComponent(): + use_state + + @component def use_custom_hook(): use_state @@ -124,7 +128,6 @@ def use_custom_hook(): module.use_effect() -@component def not_hook_or_component(): # error: ROH101 hook 'use_state' used outside component or hook definition use_state @@ -187,41 +190,34 @@ def CheckEffects(): ) @use_effect(args=[x]) - @component def my_effect(): x @use_effect(args=[]) - @component def my_effect(): # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' x @use_effect(args=[]) @some_other_deco_that_adds_args_to_func_somehow - @component def my_effect(*args, **kwargs): args kwargs @module.use_effect(args=[]) - @component def my_effect(): # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' x @not_a_decorator_we_care_about - @component def some_func(): ... @not_a_decorator_we_care_about() - @component def some_func(): ... @use_effect - @component def impropper_usage_of_effect_as_decorator(): # ignored because bad useage x From d7ebfa89007a86073e4c96cfdd3bb331f692f71a Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Fri, 19 Aug 2022 21:08:02 -0700 Subject: [PATCH 6/9] node: Any --- flake8_idom_hooks/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index 175d599..450c00e 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -33,7 +33,7 @@ def is_component_def(node: ast.FunctionDef) -> bool: ) -def is_idom_component_decorator(node) -> bool: +def is_idom_component_decorator(node: Any) -> bool: return getattr(node, "id", None) == "component" or ( getattr(getattr(node, "value", None), "id", None) == "idom" and getattr(node, "attr", None) == "component" From f3340f6b977d309604339a2d43f2425bcbcf14b0 Mon Sep 17 00:00:00 2001 From: Mark <16909269+Archmonger@users.noreply.github.com> Date: Mon, 22 Aug 2022 22:46:50 -0700 Subject: [PATCH 7/9] remove idom from test.txt --- requirements/test-env.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements/test-env.txt b/requirements/test-env.txt index a95ee81..9955dec 100644 --- a/requirements/test-env.txt +++ b/requirements/test-env.txt @@ -1,3 +1,2 @@ pytest pytest-cov -idom \ No newline at end of file From e816dde6b5eb3439053e24a66bfcacee7df78974 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 23 Aug 2022 22:45:12 -0700 Subject: [PATCH 8/9] improve component decorator check --- flake8_idom_hooks/utils.py | 29 ++++++++++++++++++++--------- noxfile.py | 8 ++++++++ tox.ini | 32 -------------------------------- 3 files changed, 28 insertions(+), 41 deletions(-) delete mode 100644 tox.ini diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py index 450c00e..4e81242 100644 --- a/flake8_idom_hooks/utils.py +++ b/flake8_idom_hooks/utils.py @@ -1,7 +1,10 @@ import ast +import re from contextlib import contextmanager from typing import Any, Iterator, List, Tuple +_COMPONENT_DECORATOR_NAME_PATTERN = re.compile(r"^(idom.(\w+\.)*)?component$") + @contextmanager def set_current(obj: Any, **attrs: Any) -> Iterator[None]: @@ -28,17 +31,25 @@ def is_hook_def(node: ast.FunctionDef) -> bool: def is_component_def(node: ast.FunctionDef) -> bool: - return any( - is_idom_component_decorator(decorator) for decorator in node.decorator_list - ) + return any(map(_is_component_decorator, node.decorator_list)) + + +def is_hook_function_name(name: str) -> bool: + return name.lstrip("_").startswith("use_") -def is_idom_component_decorator(node: Any) -> bool: - return getattr(node, "id", None) == "component" or ( - getattr(getattr(node, "value", None), "id", None) == "idom" - and getattr(node, "attr", None) == "component" +def _is_component_decorator(node: ast.AST) -> bool: + return ( + _COMPONENT_DECORATOR_NAME_PATTERN.match( + ".".join(reversed(list(_get_dotted_name(node)))) + ) + is not None ) -def is_hook_function_name(name: str) -> bool: - return name.lstrip("_").startswith("use_") +def _get_dotted_name(node: ast.AST) -> Iterator[str]: + while isinstance(node, ast.Attribute): + yield node.attr + node = node.value + if isinstance(node, ast.Name): + yield node.id diff --git a/noxfile.py b/noxfile.py index 77dfee3..9c83403 100644 --- a/noxfile.py +++ b/noxfile.py @@ -6,6 +6,13 @@ REQUIREMENTS_DIR = ROOT / "requirements" +@session +def format(session: Session) -> None: + install_requirements(session, "style") + session.run("black", ".") + session.run("isort", ".") + + @session def test(session: Session) -> None: session.notify("test_style") @@ -17,6 +24,7 @@ def test(session: Session) -> None: @session def test_style(session: Session) -> None: install_requirements(session, "style") + session.run("black", "--check", ".") session.run("isort", "--check", ".") session.run("flake8", ".") diff --git a/tox.ini b/tox.ini deleted file mode 100644 index 3f3cae2..0000000 --- a/tox.ini +++ /dev/null @@ -1,32 +0,0 @@ - -[tox] -skip_missing_interpreters = True -envlist = - {py36,py37}-nocov, - py38-{cov,lint} - -[gh-actions] -python = - 3.6: py36-nocov - 3.7: py37-nocov - 3.8: py38-{cov,lint} - -[testenv] -wheel = true -deps = - nocov: -r requirements/test.txt - cov: -r requirements/test.txt -usedevelop = - nocov: false - cov: true -commands = - nocov: pytest tests --no-cov {posargs} -vv - cov: pytest tests {posargs} -vv - -[testenv:py38-lint] -skip_install = true -deps = -r requirements/lint.txt -commands = - black . --check - flake8 . - mypy --strict flake8_idom_hooks From 64b7b1784021dad43083464dcff2fbd54605816d Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 5 Sep 2022 12:26:47 -0700 Subject: [PATCH 9/9] add options to customize hook/component patterns --- README.md | 41 ++++++- flake8_idom_hooks/__init__.py | 4 +- flake8_idom_hooks/common.py | 51 ++++++++ flake8_idom_hooks/exhaustive_deps.py | 29 +++-- flake8_idom_hooks/flake8_plugin.py | 60 +++++++-- flake8_idom_hooks/rules_of_hooks.py | 26 ++-- flake8_idom_hooks/run.py | 22 ++-- flake8_idom_hooks/utils.py | 55 --------- setup.cfg | 2 +- setup.py | 2 +- .../custom_component_decorator_pattern.py | 12 ++ tests/cases/custom_hook_function_pattern.py | 7 ++ tests/cases/exhaustive_deps.py | 114 ++++++++++++++++++ .../hook_usage.py} | 102 ---------------- tests/cases/no_exhaustive_deps.py | 68 +++++++++++ tests/test_cases.py | 70 +++++++++++ tests/test_flake8_idom_hooks.py | 38 ------ 17 files changed, 451 insertions(+), 252 deletions(-) create mode 100644 flake8_idom_hooks/common.py delete mode 100644 flake8_idom_hooks/utils.py create mode 100644 tests/cases/custom_component_decorator_pattern.py create mode 100644 tests/cases/custom_hook_function_pattern.py create mode 100644 tests/cases/exhaustive_deps.py rename tests/{hook_usage_test_cases.py => cases/hook_usage.py} (52%) create mode 100644 tests/cases/no_exhaustive_deps.py create mode 100644 tests/test_cases.py delete mode 100644 tests/test_flake8_idom_hooks.py diff --git a/README.md b/README.md index 4e68087..f3230fe 100644 --- a/README.md +++ b/README.md @@ -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. - @@ -63,3 +60,41 @@ tox
Code
+ +# Options + +All options my be used as CLI flags where `_` characters are replaced with `-`. For +example, `exhaustive_hook_deps` would become `--exhaustive-hook-deps`. + + + + + + + + + + + + + + + + + + + + + + + + + + +
OptionTypeDefaultDescription
exhaustive_hook_depsBooleanFalseEnable ROH2** errors (recommended)
component_decorator_patternRegex^(component|[\w\.]+\.component)$ + The pattern which should match the component decorators. Useful if + you import the @component decorator under an alias. +
hook_function_patternRegex^_*use_\w+$ + The pattern which should match the name of hook functions. Best used if you + have existing functions with use_* names that are not hooks. +
diff --git a/flake8_idom_hooks/__init__.py b/flake8_idom_hooks/__init__.py index 689fb5d..7f0500b 100644 --- a/flake8_idom_hooks/__init__.py +++ b/flake8_idom_hooks/__init__.py @@ -10,4 +10,6 @@ from .flake8_plugin import Plugin from .run import run_checks -__all__ = ["Plugin", "run_checks"] +plugin = Plugin() + +__all__ = ["plugin", "run_checks"] diff --git a/flake8_idom_hooks/common.py b/flake8_idom_hooks/common.py new file mode 100644 index 0000000..41c0cdc --- /dev/null +++ b/flake8_idom_hooks/common.py @@ -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 + ) diff --git a/flake8_idom_hooks/exhaustive_deps.py b/flake8_idom_hooks/exhaustive_deps.py index 7ce6620..b745e8a 100644 --- a/flake8_idom_hooks/exhaustive_deps.py +++ b/flake8_idom_hooks/exhaustive_deps.py @@ -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: @@ -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 @@ -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, @@ -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]]: @@ -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, ( @@ -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, ( @@ -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 @@ -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, ( diff --git a/flake8_idom_hooks/flake8_plugin.py b/flake8_idom_hooks/flake8_plugin.py index 8b5bc53..8b44170 100644 --- a/flake8_idom_hooks/flake8_plugin.py +++ b/flake8_idom_hooks/flake8_plugin.py @@ -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: @@ -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 diff --git a/flake8_idom_hooks/rules_of_hooks.py b/flake8_idom_hooks/rules_of_hooks.py index ead80ba..2b99a53 100644 --- a/flake8_idom_hooks/rules_of_hooks.py +++ b/flake8_idom_hooks/rules_of_hooks.py @@ -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 @@ -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, @@ -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, @@ -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] @@ -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: @@ -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) diff --git a/flake8_idom_hooks/run.py b/flake8_idom_hooks/run.py index f734bac..b06c65b 100644 --- a/flake8_idom_hooks/run.py +++ b/flake8_idom_hooks/run.py @@ -2,23 +2,27 @@ import ast +from .common import CheckContext from .exhaustive_deps import ExhaustiveDepsVisitor from .rules_of_hooks import RulesOfHooksVisitor -from .utils import ErrorVisitor + +DEFAULT_COMPONENT_DECORATOR_PATTERN = r"^(component|[\w\.]+\.component)$" +DEFAULT_HOOK_FUNCTION_PATTERN = r"^_*use_\w+$" def run_checks( tree: ast.Module, exhaustive_hook_deps: bool, + component_decorator_pattern: str = DEFAULT_COMPONENT_DECORATOR_PATTERN, + hook_function_pattern: str = DEFAULT_HOOK_FUNCTION_PATTERN, ) -> list[tuple[int, int, str]]: - visitor_types: list[type[ErrorVisitor]] = [RulesOfHooksVisitor] + context = CheckContext(component_decorator_pattern, hook_function_pattern) + + visitors: list[ast.NodeVisitor] = [RulesOfHooksVisitor(context)] if exhaustive_hook_deps: - visitor_types.append(ExhaustiveDepsVisitor) + visitors.append(ExhaustiveDepsVisitor(context)) - errors: list[tuple[int, int, str]] = [] - for vtype in visitor_types: - visitor = vtype() - visitor.visit(tree) - errors.extend(visitor.errors) + for v in visitors: + v.visit(tree) - return errors + return context.errors diff --git a/flake8_idom_hooks/utils.py b/flake8_idom_hooks/utils.py deleted file mode 100644 index 4e81242..0000000 --- a/flake8_idom_hooks/utils.py +++ /dev/null @@ -1,55 +0,0 @@ -import ast -import re -from contextlib import contextmanager -from typing import Any, Iterator, List, Tuple - -_COMPONENT_DECORATOR_NAME_PATTERN = re.compile(r"^(idom.(\w+\.)*)?component$") - - -@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 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_def(node: ast.FunctionDef) -> bool: - return is_hook_function_name(node.name) - - -def is_component_def(node: ast.FunctionDef) -> bool: - return any(map(_is_component_decorator, node.decorator_list)) - - -def is_hook_function_name(name: str) -> bool: - return name.lstrip("_").startswith("use_") - - -def _is_component_decorator(node: ast.AST) -> bool: - return ( - _COMPONENT_DECORATOR_NAME_PATTERN.match( - ".".join(reversed(list(_get_dotted_name(node)))) - ) - is not None - ) - - -def _get_dotted_name(node: ast.AST) -> Iterator[str]: - while isinstance(node, ast.Attribute): - yield node.attr - node = node.value - if isinstance(node, ast.Name): - yield node.id diff --git a/setup.cfg b/setup.cfg index fc385a0..bbfaae1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -11,7 +11,7 @@ extend-exclude = .nox venv .venv - tests/hook_usage_test_cases.py + tests/cases/* [coverage:report] fail_under = 100 diff --git a/setup.py b/setup.py index 24d7e78..b3a2e46 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ package = { "name": name, "packages": setuptools.find_packages(exclude=["tests*"]), - "entry_points": {"flake8.extension": ["ROH=flake8_idom_hooks:Plugin"]}, + "entry_points": {"flake8.extension": ["ROH=flake8_idom_hooks:plugin"]}, "python_requires": ">=3.6", "description": "Flake8 plugin to enforce the rules of hooks for IDOM", "author": "Ryan Morshead", diff --git a/tests/cases/custom_component_decorator_pattern.py b/tests/cases/custom_component_decorator_pattern.py new file mode 100644 index 0000000..54e546f --- /dev/null +++ b/tests/cases/custom_component_decorator_pattern.py @@ -0,0 +1,12 @@ +@component +def check_normal_pattern(): + if True: + # error: ROH102 hook 'use_state' used inside if statement + use_state + + +@custom_component +def check_custom_pattern(): + if True: + # error: ROH102 hook 'use_state' used inside if statement + use_state diff --git a/tests/cases/custom_hook_function_pattern.py b/tests/cases/custom_hook_function_pattern.py new file mode 100644 index 0000000..c1b5b35 --- /dev/null +++ b/tests/cases/custom_hook_function_pattern.py @@ -0,0 +1,7 @@ +@component +def check(): + if True: + # this get's ignored because of custom pattern + use_ignore_this + # error: ROH102 hook 'use_state' used inside if statement + use_state diff --git a/tests/cases/exhaustive_deps.py b/tests/cases/exhaustive_deps.py new file mode 100644 index 0000000..ee9abfe --- /dev/null +++ b/tests/cases/exhaustive_deps.py @@ -0,0 +1,114 @@ +# error: ROH101 hook 'use_effect' used outside component or hook definition +use_effect(lambda: x) # no need to check deps outside component/hook + + +@component +def check_effects(): + x = 1 + y = 2 + + # check that use_state is not treated as having dependencies. + use_state(lambda: x) + + use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + + y + ), + [y], + ) + + use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + ) + ) + + use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x.y + ), + [ + # error: ROH200 dependency arg of 'use_effect' is not destructured - dependencies should be refered to directly, not via an attribute or key of an object + x.y + ], + ) + + module.use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + ), + [], + ) + + module.submodule.use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + ), + [], + ) + + use_effect( + lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + ), + args=[], + ) + + use_effect( + function=lambda: ( + # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' + x + ), + args=[], + ) + + @use_effect(args=[x]) + def my_effect(): + x + + @use_effect(args=[]) + def my_effect(): + # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' + x + + @use_effect(args=[]) + @some_other_deco_that_adds_args_to_func_somehow + def my_effect(*args, **kwargs): + args + kwargs + + @module.use_effect(args=[]) + def my_effect(): + # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' + x + + @not_a_decorator_we_care_about + def some_func(): + ... + + @not_a_decorator_we_care_about() + def some_func(): + ... + + @use_effect + def impropper_usage_of_effect_as_decorator(): + # ignored because bad useage + x + + use_effect( + lambda: None, + # error: ROH201 dependency args of 'use_effect' should be a literal list, tuple, or None - not expression type 'Name' + not_a_list_or_tuple, + ) + + use_effect( + lambda: None, + args=None, # Ok, to explicitely set None + ) diff --git a/tests/hook_usage_test_cases.py b/tests/cases/hook_usage.py similarity index 52% rename from tests/hook_usage_test_cases.py rename to tests/cases/hook_usage.py index dc6334e..c873618 100644 --- a/tests/hook_usage_test_cases.py +++ b/tests/cases/hook_usage.py @@ -96,7 +96,6 @@ def use_state(): def generic_function(): - # error: ROH101 hook 'use_state' used outside component or hook definition use_state @@ -133,107 +132,6 @@ def not_hook_or_component(): use_state -@component -def CheckEffects(): - x = 1 - y = 2 - - use_effect( - lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x - + y - ), - [y], - ) - - use_effect( - lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x - ) - ) - - use_effect( - lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x.y - ), - [ - # error: ROH200 dependency arg of 'use_effect' is not destructured - dependencies should be refered to directly, not via an attribute or key of an object - x.y - ], - ) - - module.use_effect( - lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x - ), - [], - ) - - use_effect( - lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x - ), - args=[], - ) - - use_effect( - function=lambda: ( - # error: ROH202 dependency 'x' of function 'lambda' is not specified in declaration of 'use_effect' - x - ), - args=[], - ) - - @use_effect(args=[x]) - def my_effect(): - x - - @use_effect(args=[]) - def my_effect(): - # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' - x - - @use_effect(args=[]) - @some_other_deco_that_adds_args_to_func_somehow - def my_effect(*args, **kwargs): - args - kwargs - - @module.use_effect(args=[]) - def my_effect(): - # error: ROH202 dependency 'x' of function 'my_effect' is not specified in declaration of 'use_effect' - x - - @not_a_decorator_we_care_about - def some_func(): - ... - - @not_a_decorator_we_care_about() - def some_func(): - ... - - @use_effect - def impropper_usage_of_effect_as_decorator(): - # ignored because bad useage - x - - use_effect( - lambda: None, - # error: ROH201 dependency args of 'use_effect' should be a literal list, tuple, or None - not expression type 'Name' - not_a_list_or_tuple, - ) - - use_effect( - lambda: None, - args=None, # Ok, to explicitely set None - ) - - @component def make_component(): # nested component definitions are ok. diff --git a/tests/cases/no_exhaustive_deps.py b/tests/cases/no_exhaustive_deps.py new file mode 100644 index 0000000..fb1d6fa --- /dev/null +++ b/tests/cases/no_exhaustive_deps.py @@ -0,0 +1,68 @@ +# confirm that we're still checking for other errors +def generic_function(): + # error: ROH101 hook 'use_state' used outside component or hook definition + use_state + + +@component +def check_dependency_checks_are_ignored(): + x = 1 + y = 2 + + use_effect( + lambda: x + y, + [y], + ) + + use_effect(lambda: x) + + use_effect( + lambda: x.y, + [x.y], + ) + + module.use_effect( + lambda: x, + [], + ) + + use_effect( + lambda: x, + args=[], + ) + + use_effect( + function=lambda: x, + args=[], + ) + + @use_effect(args=[x]) + def my_effect(): + x + + @use_effect(args=[]) + def my_effect(): + x + + @use_effect(args=[]) + @some_other_deco_that_adds_args_to_func_somehow + def my_effect(*args, **kwargs): + args + kwargs + + @module.use_effect(args=[]) + def my_effect(): + x + + @not_a_decorator_we_care_about + def some_func(): + ... + + @not_a_decorator_we_care_about() + def some_func(): + ... + + use_effect( + lambda: None, + not_a_list_or_tuple, + ) diff --git a/tests/test_cases.py b/tests/test_cases.py new file mode 100644 index 0000000..2f1d008 --- /dev/null +++ b/tests/test_cases.py @@ -0,0 +1,70 @@ +import ast +from pathlib import Path + +import pytest +from flake8.options.manager import OptionManager + +import flake8_idom_hooks +from flake8_idom_hooks.flake8_plugin import Plugin + +HERE = Path(__file__).parent + + +def setup_plugin(args): + options_manager = OptionManager( + version="0.0.0", + plugin_versions=flake8_idom_hooks.__version__, + parents=[], + ) + + plugin = Plugin() + plugin.add_options(options_manager) + options = options_manager.parse_args(args) + plugin.parse_options(options) + + return plugin + + +@pytest.mark.parametrize( + "options_args, case_file_name", + [ + ( + "", + "hook_usage.py", + ), + ( + "--exhaustive-hook-deps", + "exhaustive_deps.py", + ), + ( + "", + "no_exhaustive_deps.py", + ), + ( + r"--component-decorator-pattern ^(component|custom_component)$", + "custom_component_decorator_pattern.py", + ), + ( + r"--hook-function-pattern ^_*use_(?!ignore_this)\w+$", + "custom_hook_function_pattern.py", + ), + ], +) +def test_flake8_idom_hooks(options_args, case_file_name): + case_file = Path(__file__).parent / "cases" / case_file_name + # save the file's AST + file_content = case_file.read_text() + + # find 'error' comments to construct expectations + expected_errors = set() + for index, line in enumerate(file_content.split("\n")): + lstrip_line = line.lstrip() + if lstrip_line.startswith("# error:"): + lineno = index + 2 # use 2 since error should be on next line + col_offset = len(line) - len(lstrip_line) + message = line.replace("# error:", "", 1).strip() + expected_errors.add((lineno, col_offset, message, flake8_idom_hooks.Plugin)) + + plugin = setup_plugin(options_args.split()) + actual_errors = plugin(ast.parse(file_content, case_file_name)) + assert set(actual_errors) == expected_errors diff --git a/tests/test_flake8_idom_hooks.py b/tests/test_flake8_idom_hooks.py deleted file mode 100644 index 524b0ac..0000000 --- a/tests/test_flake8_idom_hooks.py +++ /dev/null @@ -1,38 +0,0 @@ -import ast -from pathlib import Path - -from flake8.options.manager import OptionManager - -import flake8_idom_hooks - -options_manager = OptionManager( - version="0.0.0", - plugin_versions=flake8_idom_hooks.__version__, - parents=[], -) -flake8_idom_hooks.Plugin.add_options(options_manager) - - -def test_flake8_idom_hooks(): - path_to_case_file = Path(__file__).parent / "hook_usage_test_cases.py" - with path_to_case_file.open() as file: - # save the file's AST - file_content = file.read() - tree = ast.parse(file_content, path_to_case_file.name) - - # find 'error' comments to construct expectations - expected_errors = set() - for index, line in enumerate(file_content.split("\n")): - lstrip_line = line.lstrip() - if lstrip_line.startswith("# error:"): - lineno = index + 2 # use 2 since error should be on next line - col_offset = len(line) - len(lstrip_line) - message = line.replace("# error:", "", 1).strip() - expected_errors.add( - (lineno, col_offset, message, flake8_idom_hooks.Plugin) - ) - - options = options_manager.parse_args(["--exhaustive-hook-deps"]) - flake8_idom_hooks.Plugin.parse_options(options) - - assert set(flake8_idom_hooks.Plugin(tree).run()) == expected_errors