Merge pull request #4 from joelspadin/type-checking

Add linting and type checking

.github/workflows/ci.yml

@@ -0,0 +1,28 @@
+name: CI
+
+on:
+  pull_request:
+  push:
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: ">=3.10"
+          cache: "pip"
+      - name: Install dependencies
+        run: |
+          pip install .
+          pip install pylint
+
+      - name: pre-commit
+        uses: pre-commit/[email protected]
+
+      - name: pyright
+        uses: jakebailey/pyright-action@v2
+
+      - name: pylint
+        run: pylint zmk

.pre-commit-config.yaml

@@ -1,19 +1,24 @@
 fail_fast: false
 repos:
-  - repo: https://github.com/psf/black
-    rev: "23.9.1"
+  - repo: https://github.com/asottile/pyupgrade
+    rev: v3.17.0
     hooks:
-      - id: black
+      - id: pyupgrade
+        args: [--py310-plus]
   - repo: https://github.com/pycqa/isort
     rev: "5.13.2"
     hooks:
       - id: isort
+  - repo: https://github.com/psf/black
+    rev: "24.8.0"
+    hooks:
+      - id: black
   - repo: https://github.com/Lucas-C/pre-commit-hooks
-    rev: v1.5.2
+    rev: v1.5.5
     hooks:
       - id: remove-tabs
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.6.0
     hooks:
       - id: trailing-whitespace
       - id: check-yaml

.vscode/extensions.json

@@ -0,0 +1,7 @@
+{
+  "recommendations": [
+    "ms-python.black-formatter",
+    "ms-python.isort",
+    "ms-python.python"
+  ]
+}

.vscode/settings.json

@@ -21,5 +21,7 @@
     "**/templates/common/**": "mako"
   },
   "isort.check": true,
-  "isort.args": ["--profile", "black"]
+  "isort.args": ["--settings-path", "${workspaceFolder}"],
+  "python.analysis.importFormat": "relative",
+  "python.analysis.typeCheckingMode": "standard"
 }

README.md

@@ -239,3 +239,23 @@ For example, to point ZMK CLI to an existing repo at `~/Documents/zmk-config`, r
 ```sh
 zmk config user.home ~/Documents/zmk-config
 ```
+
+# Development
+
+If you would like to help improve ZMK CLI, you can clone this repo and install it in editable mode so your changes to the code apply when you run `zmk`. Open a terminal to the root directory of the repository and run:
+
+```sh
+pip install -e ".[dev]"
+pre-commit install
+```
+
+You may optionally run these commands inside a [virtual environment](https://docs.python.org/3/library/venv.html) if you don't want to install ZMK CLI's dependencies globally or if your OS disallows doing this.
+
+After running `pre-commit install`, your code will be checked when you make a commit, but there are some slower checks that do not run automatically. To run these additional checks, run these commands:
+
+```sh
+pyright .
+pylint zmk
+```
+
+Alternatively, you can just create a pull request and GitHub will run the checks and report any errors.

pyproject.toml

@@ -26,6 +26,9 @@ dependencies = [
 ]
 dynamic = ["version"]
 
+[project.optional-dependencies]
+dev = ["pre-commit", "pylint", "pyright"]
+
 [project.urls]
 Documentation = "https://zmk.dev/docs"
 "Source Code" = "https://github.com/zmkfirmware/zmk-cli/"
@@ -42,6 +45,20 @@ build-backend = "setuptools.build_meta"
 [tool.isort]
 profile = "black"
 
+[tool.pylint.MAIN]
+ignore = "_version.py"
+
+[tool.pylint."MESSAGES CONTROL"]
+disable = [
+    "arguments-differ",             # Covered by pyright
+    "fixme",
+    "too-few-public-methods",
+    "too-many-arguments",
+    "too-many-branches",
+    "too-many-instance-attributes",
+    "too-many-locals",
+]
+
 [tool.setuptools]
 packages = ["zmk"]
 

zmk/backports.py

@@ -2,6 +2,8 @@
 Backports from Python > 3.10.
 """
 
+# pyright: reportMissingImports = false
+
 try:
     # pylint: disable=unused-import
     from enum import StrEnum

zmk/build.py

@@ -2,28 +2,30 @@
 Build matrix processing.
 """
 
-import collections.abc
+from collections.abc import Iterable, Mapping, Sequence
 from dataclasses import asdict, dataclass, field
 from pathlib import Path
-from typing import Any, Iterable, Optional
+from typing import Any, Self, TypeVar, cast, overload
 
 import dacite
 
 from .repo import Repo
 from .yaml import YAML
 
+T = TypeVar("T")
+
 
 @dataclass
 class BuildItem:
     """An item in the build matrix"""
 
     board: str
-    shield: Optional[str] = None
-    snippet: Optional[str] = None
-    cmake_args: Optional[str] = None
-    artifact_name: Optional[str] = None
+    shield: str | None = None
+    snippet: str | None = None
+    cmake_args: str | None = None
+    artifact_name: str | None = None
 
-    def __rich__(self):
+    def __rich__(self) -> str:
         parts = []
         parts.append(self.board)
 
@@ -52,28 +54,28 @@ class BuildMatrix:
 
     _path: Path
     _yaml: YAML
-    _data: Any
+    _data: dict[str, Any] | None
 
     @classmethod
-    def from_repo(cls, repo: Repo):
+    def from_repo(cls, repo: Repo) -> Self:
         """Get the build matrix for a repo"""
         return cls(repo.build_matrix_path)
 
-    def __init__(self, path: Path) -> None:
+    def __init__(self, path: Path):
         self._path = path
         self._yaml = YAML(typ="rt")
         self._yaml.indent(mapping=2, sequence=4, offset=2)
         try:
-            self._data = self._yaml.load(self._path)
+            self._data = cast(dict[str, Any], self._yaml.load(self._path))
         except FileNotFoundError:
             self._data = None
 
-    def write(self):
+    def write(self) -> None:
         """Updated the YAML file, creating it if necessary"""
         self._yaml.dump(self._data, self._path)
 
     @property
-    def path(self):
+    def path(self) -> Path:
         """Path to the matrix's YAML file"""
         return self._path
 
@@ -87,7 +89,7 @@ def include(self) -> list[BuildItem]:
         wrapper = dacite.from_dict(_BuildMatrixWrapper, normalized)
         return wrapper.include
 
-    def has_item(self, item: BuildItem):
+    def has_item(self, item: BuildItem) -> bool:
         """Get whether the matrix has a build item"""
         return item in self.include
 
@@ -106,7 +108,7 @@ def append(self, items: BuildItem | Iterable[BuildItem]) -> list[BuildItem]:
             return []
 
         if not self._data:
-            self._data = self._yaml.map()
+            self._data = cast(dict[str, Any], self._yaml.map())
 
         if "include" not in self._data:
             self._data["include"] = self._yaml.seq()
@@ -121,7 +123,7 @@ def remove(self, items: BuildItem | Iterable[BuildItem]) -> list[BuildItem]:
         :return: the items that were removed.
         """
         if not self._data or "include" not in self._data:
-            return False
+            return []
 
         removed = []
         items = [items] if isinstance(items, BuildItem) else items
@@ -138,7 +140,25 @@ def remove(self, items: BuildItem | Iterable[BuildItem]) -> list[BuildItem]:
         return removed
 
 
-def _keys_to_python(data: Any):
+@overload
+def _keys_to_python(data: str) -> str: ...
+
+
+@overload
+def _keys_to_python(
+    data: Sequence[T],
+) -> Sequence[T]: ...
+
+
+@overload
+def _keys_to_python(data: Mapping[str, T]) -> Mapping[str, T]: ...
+
+
+@overload
+def _keys_to_python(data: T) -> T: ...
+
+
+def _keys_to_python(data: Any) -> Any:
     """
     Fix any keys with hyphens to underscores so that dacite.from_dict() will
     work correctly.
@@ -151,10 +171,10 @@ def fix_key(key: str):
         case str():
             return data
 
-        case collections.abc.Sequence():
+        case Sequence():
             return [_keys_to_python(i) for i in data]
 
-        case collections.abc.Mapping():
+        case Mapping():
             return {fix_key(k): _keys_to_python(v) for k, v in data.items()}
 
         case _:

zmk/commands/__init__.py

@@ -7,7 +7,7 @@
 from . import cd, code, config, download, init, keyboard, module, west
 
 
-def register(app: typer.Typer):
+def register(app: typer.Typer) -> None:
     """Register all commands with the app"""
     app.command()(cd.cd)
     app.command()(code.code)

zmk/commands/cd.py

@@ -10,19 +10,19 @@
 import shellingham
 import typer
 
-from ..config import Config
+from ..config import get_config
 from ..exceptions import FatalError, FatalHomeMissing, FatalHomeNotSet
 
 
-def cd(ctx: typer.Context):
+def cd(ctx: typer.Context) -> None:
     """Go to the ZMK config repo."""
     if not sys.stdout.isatty():
         raise FatalError(
             "This command can only be used from an interactive shell. "
             'Use "cd $(zmk config user.home)" instead.'
         )
 
-    cfg = ctx.find_object(Config)
+    cfg = get_config(ctx)
     home = cfg.home_path
 
     if home is None: