From c04108a670576fefe5812c3afa4280b7a227396f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Sep 2022 22:39:02 -0700 Subject: [PATCH] check early returns + match statement --- .github/workflows/test.yaml | 18 +++++++-- README.md | 4 ++ flake8_idom_hooks/rules_of_hooks.py | 57 +++++++++++++++++++---------- noxfile.py | 13 ++++++- tests/cases/hook_usage.py | 8 ++++ tests/cases/match_statement.py | 6 +++ tests/test_cases.py | 17 +++++++-- 7 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 tests/cases/match_statement.py diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9c0e002..0c53b1a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -3,12 +3,24 @@ name: Test on: [push] jobs: - build: + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Use Latest Python + uses: actions/setup-python@v2 + with: + python-version: "3.10" + - name: Install Python Dependencies + run: pip install -r requirements/nox-deps.txt + - name: Run Tests + run: nox -s test + + environments: runs-on: ubuntu-latest strategy: matrix: python-version: ["3.7", "3.8", "3.9", "3.10"] - steps: - uses: actions/checkout@v2 - name: Use Python ${{ matrix.python-version }} @@ -18,4 +30,4 @@ jobs: - name: Install Python Dependencies run: pip install -r requirements/nox-deps.txt - name: Run Tests - run: nox -s test + run: nox -s test -- --no-cov diff --git a/README.md b/README.md index f3230fe..f708531 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,10 @@ tox ROH102 Hook was used inside a conditional or loop statement + + ROH103 + Hook was used after an early return + ROH200 diff --git a/flake8_idom_hooks/rules_of_hooks.py b/flake8_idom_hooks/rules_of_hooks.py index 2b99a53..5356ac5 100644 --- a/flake8_idom_hooks/rules_of_hooks.py +++ b/flake8_idom_hooks/rules_of_hooks.py @@ -1,5 +1,7 @@ +from __future__ import annotations + import ast -from typing import Optional, Union +import sys from .common import CheckContext, set_current @@ -7,12 +9,13 @@ 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 - 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 + self._current_call: ast.Call | None = None + self._current_component: ast.FunctionDef | None = None + self._current_conditional: ast.If | ast.IfExp | ast.Try | None = None + self._current_early_return: ast.Return | None = None + self._current_function: ast.FunctionDef | None = None + self._current_hook: ast.FunctionDef | None = None + self._current_loop: ast.For | ast.While | None = None def visit_FunctionDef(self, node: ast.FunctionDef) -> None: if self._context.is_hook_def(node): @@ -24,6 +27,7 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: # we need to reset these before enter new hook conditional=None, loop=None, + early_return=None, ): self.generic_visit(node) elif self._context.is_component_def(node): @@ -34,13 +38,14 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: # we need to reset these before visiting a new component conditional=None, loop=None, + early_return=None, ): self.generic_visit(node) else: with set_current(self, function=node): self.generic_visit(node) - def _visit_hook_usage(self, node: Union[ast.Name, ast.Attribute]) -> None: + def _visit_hook_usage(self, node: ast.Name | ast.Attribute) -> None: self._check_if_propper_hook_usage(node) visit_Attribute = _visit_hook_usage @@ -53,6 +58,7 @@ def _visit_conditional(self, node: ast.AST) -> None: visit_If = _visit_conditional visit_IfExp = _visit_conditional visit_Try = _visit_conditional + visit_Match = _visit_conditional def _visit_loop(self, node: ast.AST) -> None: with set_current(self, loop=node): @@ -61,14 +67,15 @@ def _visit_loop(self, node: ast.AST) -> None: visit_For = _visit_loop visit_While = _visit_loop + def visit_Return(self, node: ast.Return) -> None: + self._current_early_return = node + 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._context.add_error(100, node, msg) - def _check_if_propper_hook_usage( - self, node: Union[ast.Name, ast.Attribute] - ) -> None: + def _check_if_propper_hook_usage(self, node: ast.Name | ast.Attribute) -> None: if isinstance(node, ast.Name): name = node.id else: @@ -83,14 +90,24 @@ def _check_if_propper_hook_usage( 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] + node_name = _NODE_TYPE_TO_NAME[type(loop_or_conditional)] msg = f"hook {name!r} used inside {node_name}" self._context.add_error(102, node, msg) + + if self._current_early_return: + self._context.add_error( + 103, + node, + f"hook {name!r} used after an early return", + ) + + +_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", +} +if sys.version_info >= (3, 10): + _NODE_TYPE_TO_NAME[ast.Match] = "match statement" diff --git a/noxfile.py b/noxfile.py index 9c83403..676c8dd 100644 --- a/noxfile.py +++ b/noxfile.py @@ -46,7 +46,18 @@ def test_suite(session: Session) -> None: def test_coverage(session: Session) -> None: install_requirements(session, "test-env") session.install("-e", ".") - session.run("pytest", "tests", "--cov=flake8_idom_hooks", "--cov-report=term") + + posargs = session.posargs[:] + + if "--no-cov" in session.posargs: + posargs.remove("--no-cov") + session.log("Coverage won't be checked") + session.install(".") + else: + posargs += ["--cov=flake8_idom_hooks", "--cov-report=term"] + session.install("-e", ".") + + session.run("pytest", "tests", *posargs) def install_requirements(session: Session, name: str) -> None: diff --git a/tests/cases/hook_usage.py b/tests/cases/hook_usage.py index c873618..87724ce 100644 --- a/tests/cases/hook_usage.py +++ b/tests/cases/hook_usage.py @@ -159,3 +159,11 @@ def Component(): @component def use_other(): use_state + + +@component +def example(): + if True: + return None + # error: ROH103 hook 'use_state' used after an early return + use_state() diff --git a/tests/cases/match_statement.py b/tests/cases/match_statement.py new file mode 100644 index 0000000..5bf8b30 --- /dev/null +++ b/tests/cases/match_statement.py @@ -0,0 +1,6 @@ +@component +def example(): + match something: + case int: + # error: ROH102 hook 'use_state' used inside match statement + use_state() diff --git a/tests/test_cases.py b/tests/test_cases.py index 2f1d008..eae278c 100644 --- a/tests/test_cases.py +++ b/tests/test_cases.py @@ -1,4 +1,5 @@ import ast +import sys from pathlib import Path import pytest @@ -32,14 +33,22 @@ def setup_plugin(args): "", "hook_usage.py", ), - ( - "--exhaustive-hook-deps", - "exhaustive_deps.py", - ), ( "", "no_exhaustive_deps.py", ), + pytest.param( + "", + "match_statement.py", + marks=pytest.mark.skipif( + sys.version_info < (3, 10), + reason="Match statement only in Python 3.10 and above", + ), + ), + ( + "--exhaustive-hook-deps", + "exhaustive_deps.py", + ), ( r"--component-decorator-pattern ^(component|custom_component)$", "custom_component_decorator_pattern.py",