diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 461ad67..fefdb95 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,19 +51,15 @@ repos: hooks: - id: pylint args: ["--rcfile=.pylintrc"] -# TODO re-enable the static analyzer, when the rest of the dev tools have -# stabilized. -#- repo: https://github.com/pre-commit/mirrors-mypy -# rev: v1.6.1 -# hooks: -# - id: mypy -# # `mypy` is run as a system script to ensure that it is checking against -# # the libraries we have installed to the environment. -# language: system -# args: [--config-file=.mypy.ini, "--cache-dir=/dev/null"] # `pre-commit` no longer provides a hook for `pytest`, by design - repo: local hooks: + # TODO Future: enable `mypy` on the full project. `make analyze` only checks some modules. + - id: mypy + language: system + name: mypy-parser + pass_filenames: false + entry: make analyze - id: pytest language: system name: pytest diff --git a/Makefile b/Makefile index 0509f63..3842a85 100644 --- a/Makefile +++ b/Makefile @@ -35,8 +35,10 @@ export PRINT_HELP_PYSCRIPT BROWSER := python -c "$$BROWSER_PYSCRIPT" -# Include everything EXCEPT the `examples` and `commands` directories. -MYPY_FILES := percy/parser/*.py percy/render/*.py percy/repodata/*.py scripts/*.py +# TODO fully enable on the project. For now, only enforce the `parser/` module, which has been built with the static +# analyzer in mind. +# Long-term: Include everything EXCEPT the `examples` and `commands` directories. +MYPY_FILES := percy/parser/*.py # percy/render/*.py percy/repodata/*.py scripts/*.py clean: clean-cov clean-build clean-env clean-pyc clean-test clean-other ## remove all build, test, coverage and Python artifacts diff --git a/percy/parser/_traverse.py b/percy/parser/_traverse.py index fb3928a..5855167 100644 --- a/percy/parser/_traverse.py +++ b/percy/parser/_traverse.py @@ -4,11 +4,14 @@ """ from __future__ import annotations -from typing import Callable, Optional +from typing import Callable, Final, Optional from percy.parser._node import Node from percy.parser._types import ROOT_NODE_VALUE, StrStack, StrStackImmutable +# Indicates an array index that is not valid +INVALID_IDX: Final[int] = -1 + def remap_child_indices_virt_to_phys(children: list[Node]) -> list[int]: """ @@ -128,11 +131,11 @@ def traverse_with_index(root: Node, path: StrStack) -> tuple[Optional[Node], int - If the node is a member of a list, the PHYSICAL index returned will be >= 0 """ if len(path) == 0: - return None, -1 + return None, INVALID_IDX, INVALID_IDX node: Optional[Node] - virt_idx: int = -1 - phys_idx: int = -1 + virt_idx: int = INVALID_IDX + phys_idx: int = INVALID_IDX # Pre-determine if the path is targeting a list position. Patching only applies on the last index provided. if path[0].isdigit(): # Find the index position of the target on the parent's list diff --git a/percy/parser/recipe_parser.py b/percy/parser/recipe_parser.py index 75e3b6c..2f4eadd 100644 --- a/percy/parser/recipe_parser.py +++ b/percy/parser/recipe_parser.py @@ -25,7 +25,13 @@ from percy.parser._node import Node from percy.parser._selector_info import SelectorInfo -from percy.parser._traverse import remap_child_indices_virt_to_phys, traverse, traverse_all, traverse_with_index +from percy.parser._traverse import ( + INVALID_IDX, + remap_child_indices_virt_to_phys, + traverse, + traverse_all, + traverse_with_index, +) from percy.parser._types import PERCY_SUB_MARKER, ROOT_NODE_VALUE, ForceIndentDumper, Regex, StrStack from percy.parser._utils import ( dedupe_and_preserve_order, @@ -666,9 +672,9 @@ def _patch_and_log(patch: JsonPatchType) -> None: continue # Strip the []'s around the selector - bool_expression: Final[str] = selector[1:-1] + bool_expression = selector[1:-1] # Convert to a public-facing path representation - selector_path: Final[str] = stack_path_to_str(info.path) + selector_path = stack_path_to_str(info.path) # For now, if a selector lands on a boolean value, use a ternary statement. Otherwise use the # conditional logic. @@ -682,14 +688,20 @@ def _patch_and_log(patch: JsonPatchType) -> None: # CEP-13 states that ONLY list members may use the `if/then/else` blocks # TODO figure out how best to handle this edge case. if not info.node.list_member_flag: + msg_tbl.add_message( + MessageCategory.WARNING, f"A non-list item had a selector at: {selector_path}" + ) continue - bool_object: Final[dict[str, Primitives]] = {"if": bool_expression, "then": info.node.value} + bool_object = { + "if": bool_expression, + "then": None if isinstance(info.node.value, SentinelType) else info.node.value, + } patch = { "op": "replace", "path": selector_path, # Hack: Surround the patched value in a list to render as a list member. # TODO: Figure out if this is a bug in the patch code. - "value": [bool_object], + "value": cast(JsonType, [bool_object]), } # Apply the patch _patch_and_log(patch) @@ -826,7 +838,7 @@ def get_package_paths(self) -> list[str]: """ paths: list[str] = ["/"] - outputs: Final[list[str]] = cast(list[JsonType], self.get_value("/outputs", [])) + outputs: Final[list[str]] = cast(list[str], self.get_value("/outputs", [])) for i in range(len(outputs)): paths.append(f"/outputs/{i}") @@ -1135,7 +1147,7 @@ def _patch_add_find_target(self, path_stack: StrStack) -> tuple[Optional[Node], applicable - A flag indicating if the new data will be appended to a list """ if len(path_stack) == 0: - return None, -1, "", False + return None, INVALID_IDX, INVALID_IDX, "", False # Special case that only applies to `add`. The `-` character indicates the new element can be added to the end # of the list. diff --git a/percy/parser/types.py b/percy/parser/types.py index b3d2ec2..ff3f54a 100644 --- a/percy/parser/types.py +++ b/percy/parser/types.py @@ -110,7 +110,7 @@ class MessageTable: handle the logging of these messages. """ - def __init__(self): + def __init__(self) -> None: """ Constructs an empty message table """