Fixes and enforces the static analyzer

- The static analyzer now must pass on the `parser/` module
- The hope was to get the entire project under the analyzer by now. That
  hasn't happened, but given how important the parser is, we should be
  at least ensuring it passes on every commit going forward.

.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

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
 

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

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.

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
         """