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..499c9933 100644 --- a/commit_check/rule_builder.py +++ b/commit_check/rule_builder.py @@ -7,6 +7,7 @@ COMMIT_RULES, BRANCH_RULES, PUSH_RULES, + RULES_BY_CHECK, RuleCatalogEntry, ) from commit_check import ( @@ -31,6 +32,18 @@ class ValidationRule: allowed: list[str] | None = None ignored: list[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.""" result: dict[str, Any] = { @@ -39,6 +52,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: diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py index 10da2e88..71d4eb05 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://docs.commit-check.com/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,35 @@ 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 +] + +#: 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/commit_check/util.py b/commit_check/util.py index f0be979a..ee62f270 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,19 @@ 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..a850316f --- /dev/null +++ b/docs/rules.rst @@ -0,0 +1,316 @@ +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. + +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://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: + +.. code-block:: text + + [FAIL] CC003 subject_imperative: 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..53bb24a4 --- /dev/null +++ b/tests/rules_catalog_test.py @@ -0,0 +1,124 @@ +"""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, + RULES_BY_CHECK, + 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_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.""" + 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): + """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" + + +class TestRuleIdPropagation: + """Built rules carry their catalog identity through to output.""" + + @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" + assert rule.docs_url == f"{RULES_DOCS_URL}#cc003" + + @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() + 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.""" + 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") + assert rule.rule_id is None + assert rule.docs_url 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" + )