diff --git a/docs/writing_checkers.md b/docs/writing_checkers.md index 17ecca0..2768a8d 100644 --- a/docs/writing_checkers.md +++ b/docs/writing_checkers.md @@ -87,13 +87,15 @@ Return `None` from `check_row` to skip a row that the SQL pulled in but doesn't ## Wire it up -Three things, none of them automatic: +Two things, neither of them in `src/`: -1. **Add the import** to `src/pgsleuth/checkers/__init__.py`. **The CLI imports that barrel exactly once at startup; checkers not imported there are silently invisible.** This is the single most common new-checker mistake. +1. **Add a docs page** at `docs/rules/.md` covering rationale, an example of the smell, the fix SQL, and "when to ignore." Each `Issue` carries a `docs_url` that points here automatically — no need to wire it. -2. **Add a docs page** at `docs/rules/.md` covering rationale, an example of the smell, the fix SQL, and "when to ignore." Each `Issue` carries a `docs_url` that points here automatically — no need to wire it. +2. **Add a row** to the `## Checks` table in the README, linking to the docs page. -3. **Add a row** to the `## Checks` table in the README, linking to the docs page. +The checker module itself is auto-discovered: `src/pgsleuth/checkers/__init__.py` walks the directory and imports every submodule on package import, so the `register(MyChecker)` call at the bottom of your file runs without any barrel edit. **The single thing you must remember: call `register(MyChecker)` at the bottom of the module.** Skip that and the rule won't appear in `pgsleuth list-checkers` even though the file exists. The `tests/test_auto_discovery.py` suite catches this case. + +If you add a *helper* module that isn't a checker (shared SQL, utility code), prefix the filename with `_` so auto-discovery skips it — e.g. `_partition_helpers.py`. (`base.py` is also skipped explicitly.) ## Pick the severity diff --git a/src/pgsleuth/checkers/__init__.py b/src/pgsleuth/checkers/__init__.py index d4ea1a7..9cb28ec 100644 --- a/src/pgsleuth/checkers/__init__.py +++ b/src/pgsleuth/checkers/__init__.py @@ -1,17 +1,22 @@ -"""Checker package. Importing this module loads all built-in checkers and -registers them with the global registry.""" +"""Checker package — auto-discovers every checker module on import. -from pgsleuth.checkers import ( # noqa: F401 - column_value_at_risk, - fk_type_mismatch, - json_over_jsonb, - missing_fk_index, - missing_primary_key, - not_valid_constraints, - primary_key_type, - redundant_index, - sequence_drift, - three_state_boolean, - timestamp_without_tz, - varchar_length, -) +Each checker module ends with ``register(MyChecker)``; importing the module +runs that call and adds the class to the global registry. This file walks +the package directory and imports every submodule so that registration +happens automatically. + +The upshot for contributors: adding a new checker is *one new file* in this +directory. There is no barrel to edit and no "I added the file but the +checker didn't show up" footgun. + +``base.py`` and any ``_``-prefixed module are skipped — they're framework +or shared-helper code, not checkers. +""" + +import importlib +import pkgutil + +for _info in pkgutil.iter_modules(__path__): + if _info.name == "base" or _info.name.startswith("_"): + continue + importlib.import_module(f"{__name__}.{_info.name}") diff --git a/tests/test_auto_discovery.py b/tests/test_auto_discovery.py new file mode 100644 index 0000000..78ebc21 --- /dev/null +++ b/tests/test_auto_discovery.py @@ -0,0 +1,40 @@ +"""Verify pgsleuth.checkers auto-discovery wires every checker module. + +These tests run without a database — they only inspect the import graph +and the in-memory registry. +""" + +from __future__ import annotations + +import pkgutil + +import pgsleuth.checkers # noqa: F401 — triggers auto-discovery +from pgsleuth.checkers.base import registry + + +def _checker_module_names() -> set[str]: + """Module names that should each register at least one Checker.""" + return { + f"pgsleuth.checkers.{info.name}" + for info in pkgutil.iter_modules(pgsleuth.checkers.__path__) + if info.name != "base" and not info.name.startswith("_") + } + + +def test_registry_is_populated(): + """Sanity: importing pgsleuth.checkers yields a non-empty registry.""" + assert len(registry.names()) > 0 + + +def test_every_checker_module_registers_at_least_one_checker(): + """Each non-framework module under pgsleuth.checkers must call + register(MyChecker) on import. If a module is silently not registering, + the rule is invisible to the CLI even though the file exists.""" + expected = _checker_module_names() + registered = {cls.__module__ for cls in registry.all()} + + missing = expected - registered + assert not missing, ( + f"these checker modules did not register a Checker — " + f"forgot `register(MyChecker)` at the bottom? {sorted(missing)}" + )