diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 8975202b..03e330a0 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -107,7 +107,7 @@ rm -f test_commit.txt test_commit_invalid.txt test_complex_commit.txt ### Integration Testing ```bash # Test as pre-commit hook -pre-commit try-repo . --verbose --hook-stage prepare-commit-msg +pre-commit try-repo . --verbose --hook-stage commit-msg # Test wheel installation python3 -m pip install dist/*.whl # After running nox -s build diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 05e07ab5..09e97b82 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,8 +5,8 @@ ci: autoupdate_schedule: quarterly # https://pre-commit.com/ -# prepare-commit-msg is used by hook id: check-message -default_install_hook_types: [pre-commit, prepare-commit-msg] +# commit-msg is used by hook id: check-message +default_install_hook_types: [pre-commit, commit-msg] repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v6.0.0 diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 6a735963..30b133f9 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -5,7 +5,7 @@ args: [--message] pass_filenames: true language: python - stages: [commit-msg, prepare-commit-msg] + stages: [commit-msg] - id: check-branch name: check branch naming description: ensures branch naming to match regex diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b2b71606..045335c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,7 @@ We appreciate your contributions to make Commit Check even better! ### Debug commit-check pre-commit hook ```bash -pre-commit try-repo ./../commit-check/ check-message --verbose --hook-stage prepare-commit-msg --commit-msg-filename .git/COMMIT_EDITMSG +pre-commit try-repo ./../commit-check/ check-message --verbose --hook-stage commit-msg --commit-msg-filename .git/COMMIT_EDITMSG ``` ### Debug commit-check wheel package diff --git a/README.rst b/README.rst index ea520d26..a51df27a 100644 --- a/README.rst +++ b/README.rst @@ -71,7 +71,7 @@ Running as pre-commit hook - repo: https://github.com/commit-check/commit-check rev: the tag or revision hooks: # support hooks - - id: check-message # requires prepare-commit-msg hook + - id: check-message # requires commit-msg hook - id: check-branch - id: check-author-name - id: check-author-email diff --git a/commit_check/author.py b/commit_check/author.py index e3cfd208..01083fa2 100644 --- a/commit_check/author.py +++ b/commit_check/author.py @@ -1,34 +1,69 @@ """Check git author name and email""" import re +from typing import Optional from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import get_commit_info, has_commits, print_error_header, print_error_message, print_suggestion +from commit_check.util import ( + get_commit_info, + has_commits, + print_error_header, + print_error_message, + print_suggestion, +) -def check_author(checks: list, check_type: str) -> int: - if has_commits() is False: - return PASS # pragma: no cover +_AUTHOR_FORMAT_MAP = { + "author_name": "an", + "author_email": "ae", +} + +def _find_check(checks: list, check_type: str) -> Optional[dict]: + """Return the first check dict matching check_type, else None.""" for check in checks: - if check['check'] == check_type: - if check['regex'] == "": - print( - f"{YELLOW}Not found regex for {check_type}. skip checking.{RESET_COLOR}", - ) - return PASS - if check_type == "author_name": - format_str = "an" - if check_type == 'author_email': - format_str = "ae" - config_value = str(get_commit_info(format_str)) - result = re.match(check['regex'], config_value) - if result is None: - if not print_error_header.has_been_called: - print_error_header() - print_error_message( - check['check'], check['regex'], - check['error'], config_value, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL + if check.get("check") == check_type: + return check + return None + + +def _get_author_value(check_type: str) -> str: + """Fetch the author value from git for the given check type.""" + format_str = _AUTHOR_FORMAT_MAP.get(check_type, "") + return str(get_commit_info(format_str)) + + +def _evaluate_check(check: dict, value: str) -> int: + """Evaluate a single author check against the provided value.""" + regex = check.get("regex", "") + if regex == "": + print(f"{YELLOW}Not found regex for {check.get('check')}. skip checking.{RESET_COLOR}") + return PASS + + if re.match(regex, value) is None: + if not print_error_header.has_been_called: + print_error_header() + check_name = str(check.get("check", "")) + error_msg = str(check.get("error", "")) + print_error_message(check_name, regex, error_msg, value) + if check.get("suggest"): + print_suggestion(check["suggest"]) + return FAIL return PASS + + +def check_author(checks: list, check_type: str) -> int: + """Validate author name or email according to configured regex.""" + if has_commits() is False: + return PASS # pragma: no cover + + check = _find_check(checks, check_type) + if not check: + return PASS + + # If regex is empty, skip without fetching author info + regex = check.get("regex", "") + if regex == "": + print(f"{YELLOW}Not found regex for {check_type}. skip checking.{RESET_COLOR}") + return PASS + + value = _get_author_value(check_type) + return _evaluate_check(check, value) diff --git a/commit_check/branch.py b/commit_check/branch.py index b7446cc5..ff7c9af2 100644 --- a/commit_check/branch.py +++ b/commit_check/branch.py @@ -4,27 +4,39 @@ from commit_check.util import get_branch_name, git_merge_base, print_error_header, print_error_message, print_suggestion, has_commits -def check_branch(checks: list) -> int: +def _find_branch_check(checks: list) -> dict | None: + """Return the first branch check config or None if not present.""" for check in checks: - if check['check'] == 'branch': - if check['regex'] == "": - print( - f"{YELLOW}Not found regex for branch naming. skip checking.{RESET_COLOR}", - ) - return PASS - branch_name = get_branch_name() - result = re.match(check['regex'], branch_name) - if result is None: - if not print_error_header.has_been_called: - print_error_header() # pragma: no cover - print_error_message( - check['check'], check['regex'], - check['error'], branch_name, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL - return PASS + if check.get('check') == 'branch': + return check + return None + + +def check_branch(checks: list) -> int: + check = _find_branch_check(checks) + if not check: + return PASS + + regex = check.get('regex', "") + if regex == "": + print( + f"{YELLOW}Not found regex for branch naming. skip checking.{RESET_COLOR}", + ) + return PASS + + branch_name = get_branch_name() + if re.match(regex, branch_name): + return PASS + + if not print_error_header.has_been_called: + print_error_header() # pragma: no cover + print_error_message( + check['check'], regex, + check['error'], branch_name, + ) + if check.get('suggest'): + print_suggestion(check['suggest']) + return FAIL def check_merge_base(checks: list) -> int: @@ -36,24 +48,30 @@ def check_merge_base(checks: list) -> int: if has_commits() is False: return PASS # pragma: no cover - for check in checks: - if check['check'] == 'merge_base': - if check['regex'] == "": - print( - f"{YELLOW}Not found target branch for checking merge base. skip checking.{RESET_COLOR}", - ) - return PASS - target_branch = check['regex'] if "origin/" in check['regex'] else f"origin/{check['regex']}" - current_branch = get_branch_name() - result = git_merge_base(target_branch, current_branch) - if result != 0: - if not print_error_header.has_been_called: - print_error_header() # pragma: no cover - print_error_message( - check['check'], check['regex'], - check['error'], current_branch, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL - return PASS + # locate merge_base rule, if any + merge_check = next((c for c in checks if c.get('check') == 'merge_base'), None) + if not merge_check: + return PASS + + regex = merge_check.get('regex', "") + if regex == "": + print( + f"{YELLOW}Not found target branch for checking merge base. skip checking.{RESET_COLOR}", + ) + return PASS + + target_branch = regex if "origin/" in regex else f"origin/{regex}" + current_branch = get_branch_name() + result = git_merge_base(target_branch, current_branch) + if result == 0: + return PASS + + if not print_error_header.has_been_called: + print_error_header() # pragma: no cover + print_error_message( + merge_check['check'], regex, + merge_check['error'], current_branch, + ) + if merge_check.get('suggest'): + print_suggestion(merge_check['suggest']) + return FAIL diff --git a/commit_check/commit.py b/commit_check/commit.py index d9ef0ab2..fab7cb2a 100644 --- a/commit_check/commit.py +++ b/commit_check/commit.py @@ -27,117 +27,120 @@ def read_commit_msg(commit_msg_file) -> str: return str(get_commit_info("s") + "\n\n" + get_commit_info("b")) +def _find_check(checks: list, kind: str) -> dict | None: + """Return the first check config matching kind, else None.""" + for check in checks: + if check.get('check') == kind: + return check + return None + + +def _ensure_msg_file(commit_msg_file: str | None) -> str: + """Return a commit message file path, falling back to the default when empty.""" + if not commit_msg_file: + return get_default_commit_msg_file() + return commit_msg_file + + +def _print_failure(check: dict, regex: str, actual: str) -> None: + if not print_error_header.has_been_called: + print_error_header() # pragma: no cover + print_error_message(check['check'], regex, check['error'], actual) + if check.get('suggest'): + print_suggestion(check['suggest']) + + def check_commit_msg(checks: list, commit_msg_file: str = "") -> int: """Check commit message against the provided checks.""" if has_commits() is False: - return PASS # pragma: no cover + return PASS # pragma: no cover - if commit_msg_file is None or commit_msg_file == "": - commit_msg_file = get_default_commit_msg_file() + check = _find_check(checks, 'message') + if not check: + return PASS - commit_msg = read_commit_msg(commit_msg_file) + regex = check.get('regex', "") + if regex == "": + print(f"{YELLOW}Not found regex for commit message. skip checking.{RESET_COLOR}") + return PASS - for check in checks: - if check['check'] == 'message': - if check['regex'] == "": - print( - f"{YELLOW}Not found regex for commit message. skip checking.{RESET_COLOR}", - ) - return PASS - - result = re.match(check['regex'], commit_msg) - if result is None: - if not print_error_header.has_been_called: - print_error_header() # pragma: no cover - print_error_message( - check['check'], check['regex'], - check['error'], commit_msg, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL - - return PASS + path = _ensure_msg_file(commit_msg_file) + commit_msg = read_commit_msg(path) + + if re.match(regex, commit_msg): + return PASS + + _print_failure(check, regex, commit_msg) + return FAIL def check_commit_signoff(checks: list, commit_msg_file: str = "") -> int: if has_commits() is False: - return PASS # pragma: no cover + return PASS # pragma: no cover - if commit_msg_file is None or commit_msg_file == "": - commit_msg_file = get_default_commit_msg_file() + check = _find_check(checks, 'commit_signoff') + if not check: + return PASS - for check in checks: - if check['check'] == 'commit_signoff': - if check['regex'] == "": - print( - f"{YELLOW}Not found regex for commit signoff. skip checking.{RESET_COLOR}", - ) - return PASS - - commit_msg = read_commit_msg(commit_msg_file) - - # Extract the subject line (first line of commit message) - subject = commit_msg.split('\n')[0].strip() - - # Skip if merge commit - if subject.startswith('Merge'): - return PASS - - commit_hash = get_commit_info("H") - result = re.search(check['regex'], commit_msg) - if result is None: - if not print_error_header.has_been_called: - print_error_header() # pragma: no cover - print_error_message( - check['check'], check['regex'], - check['error'], commit_hash, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL - - return PASS + regex = check.get('regex', "") + if regex == "": + print(f"{YELLOW}Not found regex for commit signoff. skip checking.{RESET_COLOR}") + return PASS + + path = _ensure_msg_file(commit_msg_file) + commit_msg = read_commit_msg(path) + + # Extract the subject line (first line of commit message) + subject = commit_msg.split('\n')[0].strip() + + # Skip if merge commit + if subject.startswith('Merge'): + return PASS + + commit_hash = get_commit_info("H") + if re.search(regex, commit_msg): + return PASS + + if not print_error_header.has_been_called: + print_error_header() # pragma: no cover + print_error_message(check['check'], regex, check['error'], commit_hash) + if check.get('suggest'): + print_suggestion(check['suggest']) + return FAIL def check_imperative(checks: list, commit_msg_file: str = "") -> int: """Check if commit message uses imperative mood.""" if has_commits() is False: - return PASS # pragma: no cover + return PASS # pragma: no cover - if commit_msg_file is None or commit_msg_file == "": - commit_msg_file = get_default_commit_msg_file() + check = _find_check(checks, 'imperative') + if not check: + return PASS - for check in checks: - if check['check'] == 'imperative': - commit_msg = read_commit_msg(commit_msg_file) - - # Extract the subject line (first line of commit message) - subject = commit_msg.split('\n')[0].strip() - - # Skip if empty or merge commit - if not subject or subject.startswith('Merge'): - return PASS - - # For conventional commits, extract description after the colon - if ':' in subject: - description = subject.split(':', 1)[1].strip() - else: - description = subject - - # Check if the description uses imperative mood - if not _is_imperative(description): - if not print_error_header.has_been_called: - print_error_header() # pragma: no cover - print_error_message( - check['check'], 'imperative mood pattern', - check['error'], subject, - ) - if check['suggest']: - print_suggestion(check['suggest']) - return FAIL - - return PASS + path = _ensure_msg_file(commit_msg_file) + commit_msg = read_commit_msg(path) + + # Extract the subject line (first line of commit message) + subject = commit_msg.split('\n')[0].strip() + + # Skip if empty or merge commit + if not subject or subject.startswith('Merge'): + return PASS + + # For conventional commits, extract description after the colon + description = subject.split(':', 1)[1].strip() if ':' in subject else subject + + # Check if the description uses imperative mood + if _is_imperative(description): + return PASS + + if not print_error_header.has_been_called: + print_error_header() # pragma: no cover + print_error_message(check['check'], 'imperative mood pattern', check['error'], subject) + if check.get('suggest'): + print_suggestion(check['suggest']) + return FAIL def _is_imperative(description: str) -> bool: diff --git a/pyproject.toml b/pyproject.toml index 8f956392..b2e38487 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,3 +67,9 @@ omit = [ # don't include tests in coverage "tests/*", ] + +[tool.pytest.ini_options] +# Silence PytestUnknownMarkWarning for custom marks used in tests +markers = [ + "benchmark: performance-related tests (no-op marker in this project)", +]