pyproject.toml (1)

71-75: Optional: pin pytest minimum version for pyproject-based config.

Small guard to avoid older pytest parsing differences.

Apply this diff:

 [tool.pytest.ini_options]
+# Ensure a modern pytest that fully supports pyproject config
+minversion = "7.0"
 # Silence PytestUnknownMarkWarning for custom marks used in tests
 markers = [
     "benchmark: performance-related tests (no-op marker in this project)",
 ]

commit_check/author.py (1)

28-31: Optional: assert known author check types.

Preempt accidental misuse and avoid calling git with an empty format string.

Apply this diff:

 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, "")
+    format_str = _AUTHOR_FORMAT_MAP.get(check_type, "")
+    if not format_str:  # defensive
+        return ""
     return str(get_commit_info(format_str))

commit_check/branch.py (2)

33-39: Optional: tolerate missing 'error'/'suggest' keys.

Use .get with defaults to avoid KeyError on partial configs.

Apply this diff:

-    print_error_message(
-        check['check'], regex,
-        check['error'], branch_name,
-    )
-    if check.get('suggest'):
-        print_suggestion(check['suggest'])
+    print_error_message(
+        check.get('check', 'branch'), regex,
+        check.get('error', ''), branch_name,
+    )
+    if check.get('suggest'):
+        print_suggestion(check['suggest'])

---

71-77: Optional: same defensive .get usage for merge_base.

Apply this diff:

-    print_error_message(
-        merge_check['check'], regex,
-        merge_check['error'], current_branch,
-    )
-    if merge_check.get('suggest'):
-        print_suggestion(merge_check['suggest'])
+    print_error_message(
+        merge_check.get('check', 'merge_base'), regex,
+        merge_check.get('error', ''), current_branch,
+    )
+    if merge_check.get('suggest'):
+        print_suggestion(merge_check['suggest'])

commit_check/commit.py (1)

30-36: Centralize _find_check to avoid duplication across modules

This duplicates commit_check/author.py::_find_check. Consider moving a single find_check helper to commit_check.util and importing it here and in other modules.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-07T15:16:47.288Z
Learning: Ensure author information in commits is valid: proper author name and email formats

commit_check/branch.py (1)

4-4: Python 3.9 compat: imported _find_check likely uses PEP 604 unions

util._find_check is annotated as dict | None in the repo snippets; this breaks on Python 3.9. Please switch to Optional[dict] in commit_check/util.py (and elsewhere) or drop the annotation.

Run to locate offenders:

#!/bin/bash
# Find PEP 604 unions that will fail on Python 3.9
rg -nP --type=py '\b[A-Za-z_][A-Za-z0-9_\[\],\s]*\|\s*None\b|->\s*[A-Za-z_][A-Za-z0-9_\[\],\s]*\|\s*None\b' -C2

commit_check/author.py (1)

26-31: Harden failure printing: guard header flag and avoid KeyError

Use getattr to avoid AttributeError and .get() to avoid KeyError if config misses fields.

 def _print_failure(check: dict, regex: str, actual: str) -> None:
-    if not print_error_header.has_been_called:
-        print_error_header()
-    print_error_message(check['check'], regex, check['error'], actual)
+    if not getattr(print_error_header, "has_been_called", False):
+        print_error_header()
+    print_error_message(str(check.get('check', "")), regex, str(check.get('error', "")), actual)
     if check.get('suggest'):
         print_suggestion(check['suggest'])

commit_check/commit.py (2)

20-26: Unify failure handling: guard header flag and avoid KeyError

Mirror the robustness used elsewhere.

 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 not getattr(print_error_header, "has_been_called", False):
+        print_error_header()  # pragma: no cover
+    print_error_message(str(check.get('check', "")), regex, str(check.get('error', "")), actual)
     if check.get('suggest'):
         print_suggestion(check['suggest'])

---

90-95: Micro-opt: defer git hash lookup until failure

Avoids a subprocess on success path.

-    commit_hash = get_commit_info("H")
     if re.search(regex, commit_msg):
         return PASS
 
-    _print_failure(check, regex, commit_hash)
+    commit_hash = get_commit_info("H")
+    _print_failure(check, regex, commit_hash)
     return FAIL

commit_check/branch.py (4)

22-24: Polish the skip message

More concise and consistent wording.

-        print(
-            f"{YELLOW}Not found regex for branch naming. skip checking.{RESET_COLOR}",
-        )
+        print(f"{YELLOW}No branch regex configured; skipping check.{RESET_COLOR}")

---

28-29: Consider fullmatch for exact branch rule compliance

re.match allows partial matches unless the pattern is anchored; fullmatch enforces whole-string validation.

-    if re.match(regex, branch_name):
+    if re.fullmatch(regex, branch_name):
         return PASS

---

49-55: Polish the merge_base skip message

Clearer wording.

-    if regex == "":
-        print(
-            f"{YELLOW}Not found target branch for checking merge base. skip checking.{RESET_COLOR}",
-        )
+    if regex == "":
+        print(f"{YELLOW}No target branch configured for merge_base; skipping check.{RESET_COLOR}")
         return PASS

---

56-56: Avoid false positives when detecting existing origin/ prefix

Use startswith to prevent accidental matches where "origin/" appears elsewhere in the string.

-    target_branch = regex if "origin/" in regex else f"origin/{regex}"
+    target_branch = regex if regex.startswith("origin/") else f"origin/{regex}"

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

commit_check/commit.py (2)

13-18: Replace PEP 604 str | None with Optional[str] or bump supported Python

Project metadata previously targeted Python >=3.9; X | None requires 3.10+. Use Optional[str] to retain 3.9 support, or update requires-python to >=3.10.

+from typing import Optional
@@
-def _ensure_msg_file(commit_msg_file: str | None) -> str:
+def _ensure_msg_file(commit_msg_file: Optional[str]) -> str:

#!/bin/bash
# Confirm declared Python version to decide between Optional vs. 3.10+
rg -n "requires-python|Programming Language :: Python ::" -S pyproject.toml || true

---

55-57: Guard header gating inside util._print_failure to avoid AttributeError

current implementation in util reads print_error_header.has_been_called directly, which can crash on first call. Use getattr/setattr there.

Apply in commit_check/util.py (for reference):

-    if not print_error_header.has_been_called:
-        print_error_header()
+    if not getattr(print_error_header, "has_been_called", False):
+        print_error_header()  # pragma: no cover
+        setattr(print_error_header, "has_been_called", True)

Also applies to: 86-87, 116-118

tests/commit_test.py (1)

52-55: Use PASS constant instead of magic number

Avoid asserting literal 0; assert against PASS for clarity and resilience.

-    assert result == 0
+    assert result == PASS

tests/author_test.py (1)

260-261: Remove duplicate assertion

The second identical assert is redundant.

-            assert m_print_suggestion.call_count == 1

commit_check/commit.py (3)

26-34: Tighten typing and drop unnecessary str()

Add a type for commit_msg_file; no need to wrap concatenated string with str().

-def read_commit_msg(commit_msg_file) -> str:
+def read_commit_msg(commit_msg_file: str) -> str:
@@
-    except FileNotFoundError:
-        # Commit message is composed by subject and body
-        return str(get_commit_info("s") + "\n\n" + get_commit_info("b"))
+    except FileNotFoundError:
+        # Fallback to subject + body when file is absent
+        return get_commit_info("s") + "\n\n" + get_commit_info("b")

---

35-57: Reduce noise: show subject in failure instead of full message

Printing the entire message can be verbose. Passing the subject keeps output focused.

-    if re.match(regex, commit_msg):
+    if re.match(regex, commit_msg):
         return PASS
-
-    _print_failure(check, regex, commit_msg)
+
+    subject = commit_msg.split('\n')[0].strip()
+    _print_failure(check, regex, subject)
     return FAIL

---

82-87: Avoid unnecessary git calls on success path

Only fetch commit hash if the regex fails.

-    commit_hash = get_commit_info("H")
-    if re.search(regex, commit_msg):
-        return PASS
-
-    _print_failure(check, regex, commit_hash)
-    return FAIL
+    if re.search(regex, commit_msg):
+        return PASS
+
+    commit_hash = get_commit_info("H")
+    _print_failure(check, regex, commit_hash)
+    return FAIL

commit_check/branch.py (3)

20-21: Prefer fullmatch for branch naming rules.

re.match allows partial prefixes; most naming policies expect the whole branch to conform.

-    if re.match(regex, branch_name):
+    if re.fullmatch(regex, branch_name):
         return PASS

---

42-47: Consistent warning mechanism.

Consider routing warnings through a shared helper (e.g., util.print_warning(...)) to keep messaging uniform across modules and simplify testing.

---

48-49: Nit: use startswith for remote prefix.

"origin/" in regex can misfire on exotic patterns; regex.startswith("origin/") is stricter.

-    target_branch = regex if "origin/" in regex else f"origin/{regex}"
+    target_branch = regex if regex.startswith("origin/") else f"origin/{regex}"

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro