From a99061b5dd8c04dcfd736a64f66bf09d2db2f7cd Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Mon, 3 Aug 2026 09:37:08 +0000 Subject: [PATCH 1/5] feat: add stable rule IDs and a rules reference Every check that can report a failure now has a stable rule ID (CC001, CC003, ...) plus a link to its documentation. IDs are assigned in the catalog and attached centrally when rules are built, so new rules inherit their identity automatically. Rule IDs and docs links now appear in the default, compact, and JSON output, giving users a durable handle to reference and look up. Add docs/rules.rst documenting every rule, and a test that fails if a rule ships without a corresponding entry in that page. --- commit_check/engine.py | 15 +- commit_check/rule_builder.py | 31 +++- commit_check/rules_catalog.py | 62 ++++++- commit_check/util.py | 29 +++- docs/index.md | 1 + docs/rules.rst | 303 ++++++++++++++++++++++++++++++++++ tests/rules_catalog_test.py | 106 ++++++++++++ 7 files changed, 534 insertions(+), 13 deletions(-) create mode 100644 docs/rules.rst create mode 100644 tests/rules_catalog_test.py diff --git a/commit_check/engine.py b/commit_check/engine.py index a1797314..2a4a72f7 100644 --- a/commit_check/engine.py +++ b/commit_check/engine.py @@ -58,15 +58,19 @@ class CheckOutcome: value: str = "" error: str = "" suggest: str = "" + rule_id: str = "" + docs_url: str = "" def to_dict(self) -> dict[str, str]: """Serialise to a plain dict (suitable for JSON encoding).""" return { + "rule_id": self.rule_id, "check": self.check, "status": self.status, "value": self.value, "error": self.error, "suggest": self.suggest, + "docs_url": self.docs_url, } @@ -873,9 +877,18 @@ def validate_all_detailed(self, context: ValidationContext) -> list[CheckOutcome value=failure.get("value", ""), error=failure.get("error", ""), suggest=failure.get("suggest", ""), + rule_id=rule.rule_id or "", + docs_url=rule.docs_url or "", ) ) else: - outcomes.append(CheckOutcome(check=rule.check, status="pass")) + outcomes.append( + CheckOutcome( + check=rule.check, + status="pass", + rule_id=rule.rule_id or "", + docs_url=rule.docs_url or "", + ) + ) return outcomes diff --git a/commit_check/rule_builder.py b/commit_check/rule_builder.py index 6e7f7b13..5d3a6cfb 100644 --- a/commit_check/rule_builder.py +++ b/commit_check/rule_builder.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Any -from dataclasses import dataclass +from dataclasses import dataclass, replace from commit_check.rules_catalog import ( COMMIT_RULES, BRANCH_RULES, @@ -30,6 +30,8 @@ class ValidationRule: value: Any = None allowed: list[str] | None = None ignored: list[str] | None = None + rule_id: str | None = None + docs_url: str | None = None def to_dict(self) -> dict[str, Any]: """Convert to dictionary for backward compatibility.""" @@ -39,6 +41,10 @@ def to_dict(self) -> dict[str, Any]: "error": self.error or "", "suggest": self.suggest or "", } + if self.rule_id: + result["rule_id"] = self.rule_id + if self.docs_url: + result["docs_url"] = self.docs_url if self.value is not None: result["value"] = self.value if self.allowed: @@ -66,6 +72,23 @@ def build_all_rules(self) -> list[ValidationRule]: rules.extend(self._build_push_rules()) return rules + @staticmethod + def _attach_rule_metadata( + rule: ValidationRule, catalog_entry: RuleCatalogEntry + ) -> ValidationRule: + """Attach the catalog's stable rule ID and docs link to a built rule. + + Applied centrally so every rule inherits its identity from the catalog, + regardless of which ``_build_*`` method constructed it. + """ + if not catalog_entry.rule_id: + return rule + return replace( + rule, + rule_id=catalog_entry.rule_id, + docs_url=catalog_entry.docs_url, + ) + def _build_commit_rules(self) -> list[ValidationRule]: """Build commit-related validation rules.""" rules = [] @@ -73,7 +96,7 @@ def _build_commit_rules(self) -> list[ValidationRule]: for catalog_entry in COMMIT_RULES: rule = self._build_single_rule(catalog_entry, self.commit_config) if rule: - rules.append(rule) + rules.append(self._attach_rule_metadata(rule, catalog_entry)) return rules @@ -84,7 +107,7 @@ def _build_branch_rules(self) -> list[ValidationRule]: for catalog_entry in BRANCH_RULES: rule = self._build_single_rule(catalog_entry, self.branch_config) if rule: - rules.append(rule) + rules.append(self._attach_rule_metadata(rule, catalog_entry)) return rules @@ -95,7 +118,7 @@ def _build_push_rules(self) -> list[ValidationRule]: for catalog_entry in PUSH_RULES: rule = self._build_push_rule(catalog_entry) if rule: - rules.append(rule) + rules.append(self._attach_rule_metadata(rule, catalog_entry)) return rules diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py index 10da2e88..be3a57b6 100644 --- a/commit_check/rules_catalog.py +++ b/commit_check/rules_catalog.py @@ -1,8 +1,28 @@ -"""Centralized catalog of all commit-check rules, regexes, and error messages.""" +"""Centralized catalog of all commit-check rules, regexes, and error messages. + +Every user-facing rule has a **stable rule ID** (e.g. ``CC003``) that never +changes once released. Rule IDs give users a durable handle to reference in +documentation, error output, and machine-readable results. + +ID ranges +--------- +========= ================================== +``CC0xx`` Commit message rules +``CC1xx`` Author (name / email) rules +``CC2xx`` Branch rules +``CC3xx`` Push rules +========= ================================== + +Internal bookkeeping entries that never produce a diagnostic (such as +``ignore_authors``) intentionally have no rule ID. +""" from __future__ import annotations from dataclasses import dataclass +#: Base URL of the rules reference documentation. +RULES_DOCS_URL = "https://commit-check.github.io/commit-check/rules.html" + @dataclass(frozen=True) class RuleCatalogEntry: @@ -10,101 +30,130 @@ class RuleCatalogEntry: regex: str | None = None error: str | None = None suggest: str | None = None + rule_id: str | None = None + + @property + def name(self) -> str: + """Human-readable rule name, e.g. ``subject-imperative``.""" + return self.check.replace("_", "-") + + @property + def docs_url(self) -> str | None: + """Link to this rule's section in the rules reference, if it has an ID.""" + if not self.rule_id: + return None + return f"{RULES_DOCS_URL}#{self.rule_id.lower()}" # Commit message rules COMMIT_RULES = [ RuleCatalogEntry( + rule_id="CC001", check="message", regex=None, # Built dynamically from config error="The commit message should follow Conventional Commits. See https://www.conventionalcommits.org", suggest="Use (): with allowed types", ), RuleCatalogEntry( + rule_id="CC002", check="subject_capitalized", regex=None, error="Subject must start with a capital letter", suggest="Capitalize the first word of the subject", ), RuleCatalogEntry( + rule_id="CC003", check="subject_imperative", regex=None, error="Commit message should use imperative mood (e.g., 'fix bug' not 'fixed bug', 'add feature' not 'adding feature')", suggest="Change the first verb to imperative form, e.g., 'fix' instead of 'fixed'/'fixes'/'fixing'", ), RuleCatalogEntry( + rule_id="CC004", check="subject_max_length", regex=None, error="Subject must be at most {max_len} characters", suggest="Keep the subject concise (<= configured max)", ), RuleCatalogEntry( + rule_id="CC005", check="subject_min_length", regex=None, error="Subject must be at least {min_len} characters", suggest="Provide a meaningful subject (>= configured min)", ), RuleCatalogEntry( + rule_id="CC006", check="allow_merge_commits", regex=None, error="Merge commits are not allowed", suggest="Rebase or squash your changes instead of merging", ), RuleCatalogEntry( + rule_id="CC007", check="allow_revert_commits", regex=None, error="Revert commits are not allowed", suggest="Avoid using 'revert' commits; rewrite history if necessary", ), RuleCatalogEntry( + rule_id="CC008", check="allow_empty_commits", regex=None, error="Empty commit messages are not allowed", suggest="Provide a non-empty subject", ), RuleCatalogEntry( + rule_id="CC009", check="allow_fixup_commits", regex=None, error="Fixup commits are not allowed", suggest="Use interactive rebase to clean up fixup commits", ), RuleCatalogEntry( + rule_id="CC010", check="allow_wip_commits", regex=None, error="WIP commits are not allowed", suggest="Complete the work before committing or remove 'WIP'", ), RuleCatalogEntry( + rule_id="CC011", check="require_body", regex=None, error="Commit body is required", suggest="Add a body explaining the change", ), RuleCatalogEntry( + rule_id="CC101", check="author_name", regex=r"^[A-Za-zÀ-ÖØ-öø-ÿ\u0100-\u017F\u0180-\u024F ,.'\-]+$|.*(\[bot])", error="The committer name seems invalid", suggest="git config user.name 'Your Name'", ), RuleCatalogEntry( + rule_id="CC102", check="author_email", regex=r"^.+@.+$", error="The committer's email seems invalid", suggest="git config user.email yourname@example.com", ), RuleCatalogEntry( + # Internal bookkeeping entry - never produces a diagnostic. check="ignore_authors", regex=None, error=None, suggest=None, ), RuleCatalogEntry( + rule_id="CC012", check="require_signed_off_by", regex=r"Signed-off-by: .+ <.+@.+>", error="Signed-off-by not found in latest commit", suggest="git commit --amend --signoff or use --signoff on commit", ), RuleCatalogEntry( + rule_id="CC013", check="ai_attribution", regex=None, error="AI attribution policy violation", @@ -115,6 +164,7 @@ class RuleCatalogEntry: # Push rules PUSH_RULES = [ RuleCatalogEntry( + rule_id="CC301", check="no_force_push", regex=None, error="Force push is not allowed", @@ -125,21 +175,31 @@ class RuleCatalogEntry: # Branch rules BRANCH_RULES = [ RuleCatalogEntry( + rule_id="CC201", check="branch", regex=None, # Built dynamically from config error="The branch should follow Conventional Branch. See https://conventionalbranch.org", suggest="Use / with allowed types or add branch name to allow_branch_names in config, or use ignore_authors in config branch section to bypass", ), RuleCatalogEntry( + rule_id="CC202", check="merge_base", regex=None, # Provided by config error="Current branch is not rebased onto target branch", suggest="Rebase or merge with the target branch", ), RuleCatalogEntry( + # Internal bookkeeping entry - never produces a diagnostic. check="ignore_authors", regex=None, error=None, suggest=None, ), ] + +#: All catalog entries that represent a user-facing, documented rule. +ALL_RULES = [ + entry + for entry in (*COMMIT_RULES, *BRANCH_RULES, *PUSH_RULES) + if entry.rule_id is not None +] diff --git a/commit_check/util.py b/commit_check/util.py index f0be979a..bf7d9afa 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -19,15 +19,25 @@ def _print_failure( compact: bool = False, ) -> None: """Print a standardized failure message.""" + rule_id = check.get("rule_id", "") if compact: compact_value = actual.splitlines()[0] if actual else actual - print(f"[FAIL] {check['check']}: {compact_value}") + label = f"{rule_id} {check['check']}" if rule_id else check["check"] + print(f"[FAIL] {label}: {compact_value}") return if not no_banner and not print_error_header.has_been_called: print_error_header() - print_error_message(check["check"], check.get("error", ""), actual) + print_error_message( + check["check"], + check.get("error", ""), + actual, + rule_id=rule_id, + ) if check.get("suggest"): print_suggestion(check["suggest"]) + docs_url = check.get("docs_url", "") + if docs_url: + print(f"Docs: {docs_url}") def get_branch_name() -> str: @@ -284,16 +294,21 @@ def print_error_header(): print(" ") -def print_error_message(check_type: str, error: str, reason: str): +def print_error_message( + check_type: str, error: str, reason: str, rule_id: str = "" +): """Print error message. - :param check_type: - :param error: - :param reason: + + :param check_type: the check that failed, e.g. ``subject_imperative`` + :param error: the human-readable explanation of the failure + :param reason: the offending value + :param rule_id: stable rule ID, e.g. ``CC003`` (omitted when empty) :returns: Give error messages to user """ + prefix = f"{YELLOW}{rule_id}{RESET_COLOR} " if rule_id else "" print( - f"Type {YELLOW}{check_type}{RESET_COLOR} check failed ==> {RED}{reason}{RESET_COLOR} ", + f"{prefix}{YELLOW}{check_type}{RESET_COLOR} check failed ==> {RED}{reason}{RESET_COLOR} ", end="", ) print("") diff --git a/docs/index.md b/docs/index.md index ab3fbfc3..69e2336c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -6,6 +6,7 @@ self what-is-new configuration +rules example migration troubleshoot diff --git a/docs/rules.rst b/docs/rules.rst new file mode 100644 index 00000000..ccd49d29 --- /dev/null +++ b/docs/rules.rst @@ -0,0 +1,303 @@ +Rules Reference +=============== + +Every check that can report a failure has a **stable rule ID**. Rule IDs never +change once released, so they are safe to reference in documentation, code +review comments, and tooling. + +Rule IDs appear in commit-check output and in ``--format json`` results: + +.. code-block:: text + + CC003 subject_imperative check failed ==> docs: revamped the profile + +ID ranges +--------- + +.. list-table:: + :header-rows: 1 + + * - Range + - Category + * - ``CC0xx`` + - Commit message + * - ``CC1xx`` + - Author + * - ``CC2xx`` + - Branch + * - ``CC3xx`` + - Push + +All rules +--------- + +.. list-table:: + :header-rows: 1 + + * - ID + - Name + - Description + * - :ref:`CC001 ` + - ``message`` + - The commit message should follow Conventional Commits + * - :ref:`CC002 ` + - ``subject-capitalized`` + - Subject must start with a capital letter + * - :ref:`CC003 ` + - ``subject-imperative`` + - Commit message should use imperative mood (e.g., 'fix bug' not 'fixed bug', 'add feature' not 'adding feature') + * - :ref:`CC004 ` + - ``subject-max-length`` + - Subject must be at most {max_len} characters + * - :ref:`CC005 ` + - ``subject-min-length`` + - Subject must be at least {min_len} characters + * - :ref:`CC006 ` + - ``allow-merge-commits`` + - Merge commits are not allowed + * - :ref:`CC007 ` + - ``allow-revert-commits`` + - Revert commits are not allowed + * - :ref:`CC008 ` + - ``allow-empty-commits`` + - Empty commit messages are not allowed + * - :ref:`CC009 ` + - ``allow-fixup-commits`` + - Fixup commits are not allowed + * - :ref:`CC010 ` + - ``allow-wip-commits`` + - WIP commits are not allowed + * - :ref:`CC011 ` + - ``require-body`` + - Commit body is required + * - :ref:`CC012 ` + - ``require-signed-off-by`` + - Signed-off-by not found in latest commit + * - :ref:`CC013 ` + - ``ai-attribution`` + - AI attribution policy violation + * - :ref:`CC101 ` + - ``author-name`` + - The committer name seems invalid + * - :ref:`CC102 ` + - ``author-email`` + - The committer's email seems invalid + * - :ref:`CC201 ` + - ``branch`` + - The branch should follow Conventional Branch + * - :ref:`CC202 ` + - ``merge-base`` + - Current branch is not rebased onto target branch + * - :ref:`CC301 ` + - ``no-force-push`` + - Force push is not allowed + +Commit message rules +-------------------- + +.. _cc001: + +CC001 — message +~~~~~~~~~~~~~~~ + +**Config key:** ``message`` + +**Message:** The commit message should follow Conventional Commits. See https://www.conventionalcommits.org + +**How to fix:** Use (): with allowed types + +.. _cc002: + +CC002 — subject-capitalized +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``subject_capitalized`` + +**Message:** Subject must start with a capital letter + +**How to fix:** Capitalize the first word of the subject + +.. _cc003: + +CC003 — subject-imperative +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``subject_imperative`` + +**Message:** Commit message should use imperative mood (e.g., 'fix bug' not 'fixed bug', 'add feature' not 'adding feature') + +**How to fix:** Change the first verb to imperative form, e.g., 'fix' instead of 'fixed'/'fixes'/'fixing' + +.. _cc004: + +CC004 — subject-max-length +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``subject_max_length`` + +**Message:** Subject must be at most {max_len} characters + +**How to fix:** Keep the subject concise (<= configured max) + +.. _cc005: + +CC005 — subject-min-length +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``subject_min_length`` + +**Message:** Subject must be at least {min_len} characters + +**How to fix:** Provide a meaningful subject (>= configured min) + +.. _cc006: + +CC006 — allow-merge-commits +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``allow_merge_commits`` + +**Message:** Merge commits are not allowed + +**How to fix:** Rebase or squash your changes instead of merging + +.. _cc007: + +CC007 — allow-revert-commits +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``allow_revert_commits`` + +**Message:** Revert commits are not allowed + +**How to fix:** Avoid using 'revert' commits; rewrite history if necessary + +.. _cc008: + +CC008 — allow-empty-commits +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``allow_empty_commits`` + +**Message:** Empty commit messages are not allowed + +**How to fix:** Provide a non-empty subject + +.. _cc009: + +CC009 — allow-fixup-commits +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``allow_fixup_commits`` + +**Message:** Fixup commits are not allowed + +**How to fix:** Use interactive rebase to clean up fixup commits + +.. _cc010: + +CC010 — allow-wip-commits +~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``allow_wip_commits`` + +**Message:** WIP commits are not allowed + +**How to fix:** Complete the work before committing or remove 'WIP' + +.. _cc011: + +CC011 — require-body +~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``require_body`` + +**Message:** Commit body is required + +**How to fix:** Add a body explaining the change + +.. _cc012: + +CC012 — require-signed-off-by +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``require_signed_off_by`` + +**Message:** Signed-off-by not found in latest commit + +**How to fix:** git commit --amend --signoff or use --signoff on commit + +.. _cc013: + +CC013 — ai-attribution +~~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``ai_attribution`` + +**Message:** AI attribution policy violation + +**How to fix:** This project forbids AI-assisted commits. Remove AI trailers and re-commit. + +Author rules +------------ + +.. _cc101: + +CC101 — author-name +~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``author_name`` + +**Message:** The committer name seems invalid + +**How to fix:** git config user.name 'Your Name' + +.. _cc102: + +CC102 — author-email +~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``author_email`` + +**Message:** The committer's email seems invalid + +**How to fix:** git config user.email yourname@example.com + +Branch rules +------------ + +.. _cc201: + +CC201 — branch +~~~~~~~~~~~~~~ + +**Config key:** ``branch`` + +**Message:** The branch should follow Conventional Branch. See https://conventionalbranch.org + +**How to fix:** Use / with allowed types or add branch name to allow_branch_names in config, or use ignore_authors in config branch section to bypass + +.. _cc202: + +CC202 — merge-base +~~~~~~~~~~~~~~~~~~ + +**Config key:** ``merge_base`` + +**Message:** Current branch is not rebased onto target branch + +**How to fix:** Rebase or merge with the target branch + +Push rules +---------- + +.. _cc301: + +CC301 — no-force-push +~~~~~~~~~~~~~~~~~~~~~ + +**Config key:** ``no_force_push`` + +**Message:** Force push is not allowed + +**How to fix:** Use a normal push instead of --force or --force-with-lease diff --git a/tests/rules_catalog_test.py b/tests/rules_catalog_test.py new file mode 100644 index 00000000..1a6ec617 --- /dev/null +++ b/tests/rules_catalog_test.py @@ -0,0 +1,106 @@ +"""Tests for stable rule IDs in the rules catalog.""" + +import re +from pathlib import Path + +import pytest + +from commit_check.rules_catalog import ( + ALL_RULES, + BRANCH_RULES, + COMMIT_RULES, + PUSH_RULES, + RULES_DOCS_URL, + RuleCatalogEntry, +) +from commit_check.rule_builder import RuleBuilder + +ALL_ENTRIES = [*COMMIT_RULES, *BRANCH_RULES, *PUSH_RULES] + + +class TestRuleIds: + """The rule ID contract: stable, unique, well-formed.""" + + @pytest.mark.benchmark + def test_rule_ids_are_unique(self): + """No two rules may share an ID.""" + ids = [e.rule_id for e in ALL_ENTRIES if e.rule_id] + assert len(ids) == len(set(ids)), "duplicate rule IDs found" + + @pytest.mark.benchmark + def test_rule_ids_are_well_formed(self): + """Rule IDs look like CC001.""" + for entry in ALL_ENTRIES: + if entry.rule_id: + assert re.fullmatch(r"CC\d{3}", entry.rule_id), entry.rule_id + + @pytest.mark.benchmark + def test_diagnostic_rules_all_have_ids(self): + """Any rule that can report a failure must have an ID. + + Entries without an error message are internal bookkeeping (e.g. + ``ignore_authors``) and intentionally carry no ID. + """ + for entry in ALL_ENTRIES: + if entry.error: + assert entry.rule_id, f"{entry.check} can fail but has no rule ID" + + @pytest.mark.benchmark + def test_docs_url_derives_from_id(self): + entry = RuleCatalogEntry(check="subject_imperative", rule_id="CC003") + assert entry.docs_url == f"{RULES_DOCS_URL}#cc003" + + @pytest.mark.benchmark + def test_no_docs_url_without_id(self): + assert RuleCatalogEntry(check="ignore_authors").docs_url is None + + @pytest.mark.benchmark + def test_name_is_kebab_case(self): + assert RuleCatalogEntry(check="subject_imperative").name == "subject-imperative" + + +class TestRuleIdPropagation: + """Built rules carry their catalog identity through to output.""" + + @pytest.mark.benchmark + def test_built_rule_has_id_and_docs_url(self): + rules = RuleBuilder({"commit": {"subject_imperative": True}}).build_all_rules() + rule = next(r for r in rules if r.check == "subject_imperative") + assert rule.rule_id == "CC003" + assert rule.docs_url == f"{RULES_DOCS_URL}#cc003" + + @pytest.mark.benchmark + def test_to_dict_includes_id_and_docs_url(self): + rules = RuleBuilder({"commit": {"subject_imperative": True}}).build_all_rules() + rule = next(r for r in rules if r.check == "subject_imperative") + as_dict = rule.to_dict() + assert as_dict["rule_id"] == "CC003" + assert as_dict["docs_url"].endswith("#cc003") + + @pytest.mark.benchmark + def test_internal_entries_have_no_id(self): + """ignore_authors is bookkeeping - it must not leak a rule ID.""" + rules = RuleBuilder( + {"commit": {"ignore_authors": ["bot"]}} + ).build_all_rules() + rule = next((r for r in rules if r.check == "ignore_authors"), None) + if rule is not None: + assert rule.rule_id is None + + +class TestRulesDocumentation: + """Anti-drift guard: every documented rule stays documented.""" + + @pytest.mark.benchmark + def test_every_rule_is_documented(self): + """Each rule ID must have an anchor in the rules reference page. + + This prevents shipping a new rule without documenting it. + """ + docs = Path(__file__).parent.parent / "docs" / "rules.rst" + content = docs.read_text(encoding="utf-8") + for entry in ALL_RULES: + anchor = f".. _{entry.rule_id.lower()}:" + assert anchor in content, ( + f"{entry.rule_id} ({entry.check}) is missing from docs/rules.rst" + ) From 72be1c24a3c5f307deffa83e89eb427e4a1098e4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:12:30 +0000 Subject: [PATCH 2/5] ci: auto fixes from pre-commit.com hooks --- commit_check/util.py | 4 +--- tests/rules_catalog_test.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/commit_check/util.py b/commit_check/util.py index bf7d9afa..ee62f270 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -294,9 +294,7 @@ def print_error_header(): print(" ") -def print_error_message( - check_type: str, error: str, reason: str, rule_id: str = "" -): +def print_error_message(check_type: str, error: str, reason: str, rule_id: str = ""): """Print error message. :param check_type: the check that failed, e.g. ``subject_imperative`` diff --git a/tests/rules_catalog_test.py b/tests/rules_catalog_test.py index 1a6ec617..1708b5c2 100644 --- a/tests/rules_catalog_test.py +++ b/tests/rules_catalog_test.py @@ -80,9 +80,7 @@ def test_to_dict_includes_id_and_docs_url(self): @pytest.mark.benchmark def test_internal_entries_have_no_id(self): """ignore_authors is bookkeeping - it must not leak a rule ID.""" - rules = RuleBuilder( - {"commit": {"ignore_authors": ["bot"]}} - ).build_all_rules() + rules = RuleBuilder({"commit": {"ignore_authors": ["bot"]}}).build_all_rules() rule = next((r for r in rules if r.check == "ignore_authors"), None) if rule is not None: assert rule.rule_id is None From 37d0c9084e5ed3fe40121b54f7cf261cdc809297 Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Mon, 3 Aug 2026 10:20:28 +0000 Subject: [PATCH 3/5] test: strengthen internal-entry assertion and document full output Assert unconditionally that the ignore_authors catalog entry and its built rule carry no rule ID; the previous conditional guard let the test pass vacuously if the rule were never built. Show the complete default output in the rules reference, including the Docs link, and document the compact form alongside it. --- docs/rules.rst | 15 ++++++++++++++- tests/rules_catalog_test.py | 14 +++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/docs/rules.rst b/docs/rules.rst index ccd49d29..5b108670 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -5,11 +5,24 @@ Every check that can report a failure has a **stable rule ID**. Rule IDs never change once released, so they are safe to reference in documentation, code review comments, and tooling. -Rule IDs appear in commit-check output and in ``--format json`` results: +Rule IDs appear in commit-check output and in ``--format json`` results. + +Default output leads with the rule ID and ends with a link to that rule's +section on this page: .. code-block:: text CC003 subject_imperative check failed ==> docs: revamped the profile + Commit message should use imperative mood (e.g., 'fix bug' not 'fixed bug') + Suggest: Change the first verb to imperative form, e.g., 'fix' instead of 'fixed' + Docs: https://commit-check.github.io/commit-check/rules.html#cc003 + +``--compact`` prints one line per failure, keeping the rule ID and omitting the +explanation, suggestion, and documentation link: + +.. code-block:: text + + [FAIL] CC003 subject_imperative: docs: revamped the profile ID ranges --------- diff --git a/tests/rules_catalog_test.py b/tests/rules_catalog_test.py index 1708b5c2..f466fa4f 100644 --- a/tests/rules_catalog_test.py +++ b/tests/rules_catalog_test.py @@ -47,15 +47,18 @@ def test_diagnostic_rules_all_have_ids(self): @pytest.mark.benchmark def test_docs_url_derives_from_id(self): + """The docs URL is derived from the rule ID, not stored separately.""" entry = RuleCatalogEntry(check="subject_imperative", rule_id="CC003") assert entry.docs_url == f"{RULES_DOCS_URL}#cc003" @pytest.mark.benchmark def test_no_docs_url_without_id(self): + """An entry without a rule ID has no docs URL to link to.""" assert RuleCatalogEntry(check="ignore_authors").docs_url is None @pytest.mark.benchmark def test_name_is_kebab_case(self): + """The display name is the kebab-case form of the config key.""" assert RuleCatalogEntry(check="subject_imperative").name == "subject-imperative" @@ -64,6 +67,7 @@ class TestRuleIdPropagation: @pytest.mark.benchmark def test_built_rule_has_id_and_docs_url(self): + """A rule built from the catalog carries its ID and docs URL.""" rules = RuleBuilder({"commit": {"subject_imperative": True}}).build_all_rules() rule = next(r for r in rules if r.check == "subject_imperative") assert rule.rule_id == "CC003" @@ -71,6 +75,7 @@ def test_built_rule_has_id_and_docs_url(self): @pytest.mark.benchmark def test_to_dict_includes_id_and_docs_url(self): + """Serialised rules expose the ID and docs URL to consumers.""" rules = RuleBuilder({"commit": {"subject_imperative": True}}).build_all_rules() rule = next(r for r in rules if r.check == "subject_imperative") as_dict = rule.to_dict() @@ -80,10 +85,13 @@ def test_to_dict_includes_id_and_docs_url(self): @pytest.mark.benchmark def test_internal_entries_have_no_id(self): """ignore_authors is bookkeeping - it must not leak a rule ID.""" + entry = next(e for e in ALL_ENTRIES if e.check == "ignore_authors") + assert entry.rule_id is None + rules = RuleBuilder({"commit": {"ignore_authors": ["bot"]}}).build_all_rules() - rule = next((r for r in rules if r.check == "ignore_authors"), None) - if rule is not None: - assert rule.rule_id is None + rule = next(r for r in rules if r.check == "ignore_authors") + assert rule.rule_id is None + assert rule.docs_url is None class TestRulesDocumentation: From 5049b1f8ac641af1b3ef4aa7917187c2ee1d60b0 Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Mon, 3 Aug 2026 10:29:45 +0000 Subject: [PATCH 4/5] perf: derive rule identity from the catalog instead of copying it Attaching the rule ID with dataclasses.replace() rebuilt every rule on each build_all_rules() call, roughly doubling its cost (19us -> 41us) and showing up as a broad benchmark regression. Expose rule_id and docs_url as properties backed by a check-name lookup into the catalog. Identity still lives in one place, so rules cannot carry a stale copy, and building them no longer does extra work. Add a test asserting the lookup's key assumption that identified check names are unique. --- commit_check/rule_builder.py | 40 +++++++++++++++-------------------- commit_check/rules_catalog.py | 4 ++++ tests/rules_catalog_test.py | 12 +++++++++++ 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/commit_check/rule_builder.py b/commit_check/rule_builder.py index 5d3a6cfb..499c9933 100644 --- a/commit_check/rule_builder.py +++ b/commit_check/rule_builder.py @@ -2,11 +2,12 @@ from __future__ import annotations from typing import Any -from dataclasses import dataclass, replace +from dataclasses import dataclass from commit_check.rules_catalog import ( COMMIT_RULES, BRANCH_RULES, PUSH_RULES, + RULES_BY_CHECK, RuleCatalogEntry, ) from commit_check import ( @@ -30,8 +31,18 @@ class ValidationRule: value: Any = None allowed: list[str] | None = None ignored: list[str] | None = None - rule_id: str | None = None - docs_url: str | None = None + + @property + def rule_id(self) -> str | None: + """Stable rule ID from the catalog, e.g. ``CC003``.""" + entry = RULES_BY_CHECK.get(self.check) + return entry.rule_id if entry else None + + @property + def docs_url(self) -> str | None: + """Link to this rule's section in the rules reference.""" + entry = RULES_BY_CHECK.get(self.check) + return entry.docs_url if entry else None def to_dict(self) -> dict[str, Any]: """Convert to dictionary for backward compatibility.""" @@ -72,23 +83,6 @@ def build_all_rules(self) -> list[ValidationRule]: rules.extend(self._build_push_rules()) return rules - @staticmethod - def _attach_rule_metadata( - rule: ValidationRule, catalog_entry: RuleCatalogEntry - ) -> ValidationRule: - """Attach the catalog's stable rule ID and docs link to a built rule. - - Applied centrally so every rule inherits its identity from the catalog, - regardless of which ``_build_*`` method constructed it. - """ - if not catalog_entry.rule_id: - return rule - return replace( - rule, - rule_id=catalog_entry.rule_id, - docs_url=catalog_entry.docs_url, - ) - def _build_commit_rules(self) -> list[ValidationRule]: """Build commit-related validation rules.""" rules = [] @@ -96,7 +90,7 @@ def _build_commit_rules(self) -> list[ValidationRule]: for catalog_entry in COMMIT_RULES: rule = self._build_single_rule(catalog_entry, self.commit_config) if rule: - rules.append(self._attach_rule_metadata(rule, catalog_entry)) + rules.append(rule) return rules @@ -107,7 +101,7 @@ def _build_branch_rules(self) -> list[ValidationRule]: for catalog_entry in BRANCH_RULES: rule = self._build_single_rule(catalog_entry, self.branch_config) if rule: - rules.append(self._attach_rule_metadata(rule, catalog_entry)) + rules.append(rule) return rules @@ -118,7 +112,7 @@ def _build_push_rules(self) -> list[ValidationRule]: for catalog_entry in PUSH_RULES: rule = self._build_push_rule(catalog_entry) if rule: - rules.append(self._attach_rule_metadata(rule, catalog_entry)) + rules.append(rule) return rules diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py index be3a57b6..e6c1c28c 100644 --- a/commit_check/rules_catalog.py +++ b/commit_check/rules_catalog.py @@ -203,3 +203,7 @@ def docs_url(self) -> str | None: for entry in (*COMMIT_RULES, *BRANCH_RULES, *PUSH_RULES) if entry.rule_id is not None ] + +#: Lookup from check name to its catalog entry, for rules that have an ID. +#: Rule identity lives only here, so built rules can never carry a stale copy. +RULES_BY_CHECK = {entry.check: entry for entry in ALL_RULES} diff --git a/tests/rules_catalog_test.py b/tests/rules_catalog_test.py index f466fa4f..53bb24a4 100644 --- a/tests/rules_catalog_test.py +++ b/tests/rules_catalog_test.py @@ -7,6 +7,7 @@ from commit_check.rules_catalog import ( ALL_RULES, + RULES_BY_CHECK, BRANCH_RULES, COMMIT_RULES, PUSH_RULES, @@ -27,6 +28,17 @@ def test_rule_ids_are_unique(self): ids = [e.rule_id for e in ALL_ENTRIES if e.rule_id] assert len(ids) == len(set(ids)), "duplicate rule IDs found" + @pytest.mark.benchmark + def test_identified_checks_are_unique(self): + """Check names of identified rules must be unique. + + Rule identity is looked up by check name, so a duplicate would + silently shadow one of the rules. + """ + checks = [e.check for e in ALL_RULES] + assert len(checks) == len(set(checks)) + assert len(RULES_BY_CHECK) == len(ALL_RULES) + @pytest.mark.benchmark def test_rule_ids_are_well_formed(self): """Rule IDs look like CC001.""" From 3f4f6eec7c3da062ecdcbe73b5e64352b4ec4341 Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Mon, 3 Aug 2026 11:08:33 +0000 Subject: [PATCH 5/5] docs: point rule links at the documentation domain Rule IDs print a link to their reference section, and that link ships in terminal output and JSON results. Settle it on the project's own domain before the first release that carries it, so the URLs do not need redirecting later. --- commit_check/rules_catalog.py | 2 +- docs/rules.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py index e6c1c28c..71d4eb05 100644 --- a/commit_check/rules_catalog.py +++ b/commit_check/rules_catalog.py @@ -21,7 +21,7 @@ from dataclasses import dataclass #: Base URL of the rules reference documentation. -RULES_DOCS_URL = "https://commit-check.github.io/commit-check/rules.html" +RULES_DOCS_URL = "https://docs.commit-check.com/rules.html" @dataclass(frozen=True) diff --git a/docs/rules.rst b/docs/rules.rst index 5b108670..a850316f 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -15,7 +15,7 @@ section on this page: CC003 subject_imperative check failed ==> docs: revamped the profile Commit message should use imperative mood (e.g., 'fix bug' not 'fixed bug') Suggest: Change the first verb to imperative form, e.g., 'fix' instead of 'fixed' - Docs: https://commit-check.github.io/commit-check/rules.html#cc003 + Docs: https://docs.commit-check.com/rules.html#cc003 ``--compact`` prints one line per failure, keeping the rule ID and omitting the explanation, suggestion, and documentation link: