From ea66ef4ce1f8d5e9eeeed6c886c5bb98a94e1e5f Mon Sep 17 00:00:00 2001 From: Ajesh Sen Thapa Date: Tue, 12 Mar 2024 17:43:36 +0545 Subject: [PATCH] feat: enhanced error message - Removed old `commitlint.py`. - Added new module `linter` for linting and validation. - Added detailed validation on `linter` for message enhancement. - Added new error messages. - Added option `--quick` for quick linting. - Moved old validation to quick linting. - Added `pytest.ini` for adding src as a python path. - Updated test for both detailed and quick with same test data. --- .github/workflows/ci.yaml | 2 +- github_actions/event.py | 1 + github_actions/run.py | 1 + pytest.ini | 2 + src/commitlint/__init__.py | 6 +- src/commitlint/cli.py | 55 ++-- src/commitlint/commitlint.py | 124 --------- src/commitlint/constants.py | 25 ++ src/commitlint/git_helpers.py | 1 + src/commitlint/linter/__init__.py | 7 + src/commitlint/linter/_linter.py | 70 +++++ src/commitlint/linter/quick.py | 46 ++++ src/commitlint/linter/utils.py | 57 ++++ src/commitlint/linter/validators.py | 254 ++++++++++++++++++ src/commitlint/messages.py | 31 ++- tests/test_cli.py | 117 ++++++-- .../test_check_commit_message.py | 94 ------- tests/test_git_helpers.py | 10 +- .../__init__.py | 0 tests/test_linter/_test_data.py | 87 ++++++ tests/test_linter/test_linter.py | 17 ++ tests/test_linter/test_quick.py | 32 +++ tests/test_linter/test_utils/__init__.py | 0 .../test_utils}/test_is_ingored.py | 2 +- .../test_utils}/test_remove_comments.py | 2 +- 25 files changed, 757 insertions(+), 286 deletions(-) create mode 100644 pytest.ini delete mode 100644 src/commitlint/commitlint.py create mode 100644 src/commitlint/linter/__init__.py create mode 100644 src/commitlint/linter/_linter.py create mode 100644 src/commitlint/linter/quick.py create mode 100644 src/commitlint/linter/utils.py create mode 100644 src/commitlint/linter/validators.py delete mode 100644 tests/test_commitlint/test_check_commit_message.py rename tests/{test_commitlint => test_linter}/__init__.py (100%) create mode 100644 tests/test_linter/_test_data.py create mode 100644 tests/test_linter/test_linter.py create mode 100644 tests/test_linter/test_quick.py create mode 100644 tests/test_linter/test_utils/__init__.py rename tests/{test_commitlint => test_linter/test_utils}/test_is_ingored.py (95%) rename tests/{test_commitlint => test_linter/test_utils}/test_remove_comments.py (95%) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a76b295..34229ab 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -68,4 +68,4 @@ jobs: uses: liskin/gh-problem-matcher-wrap@v3 with: linters: isort - run: isort --line-length=88 --check src/ + run: isort --line-length=88 --check --profile black src/ diff --git a/github_actions/event.py b/github_actions/event.py index 4546806..61a5c54 100644 --- a/github_actions/event.py +++ b/github_actions/event.py @@ -5,6 +5,7 @@ This module relies on the presence of specific environment variables set by GitHub Actions. """ + import json import os from typing import Any, Dict diff --git a/github_actions/run.py b/github_actions/run.py index af299c5..4cf2a48 100644 --- a/github_actions/run.py +++ b/github_actions/run.py @@ -2,6 +2,7 @@ This script contains actions to be taken based on GitHub events, specifically for push and pull_request events. """ + import os import subprocess import sys diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..fcccae1 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +pythonpath = src diff --git a/src/commitlint/__init__.py b/src/commitlint/__init__.py index bc45112..a73de82 100644 --- a/src/commitlint/__init__.py +++ b/src/commitlint/__init__.py @@ -1,5 +1,5 @@ -"""Main module for commitlint""" +"""Main module for commitlint.""" -from .commitlint import check_commit_message +from .linter import lint_commit_message -__all__ = ["check_commit_message"] +__all__ = ["lint_commit_message"] diff --git a/src/commitlint/cli.py b/src/commitlint/cli.py index 3eeea02..fc61b13 100644 --- a/src/commitlint/cli.py +++ b/src/commitlint/cli.py @@ -19,10 +19,11 @@ from typing import List from .__version__ import __version__ -from .commitlint import check_commit_message, remove_comments from .exceptions import CommitlintException from .git_helpers import get_commit_message_of_hash, get_commit_messages_of_hash_range -from .messages import VALIDATION_SUCCESSFUL +from .linter import lint_commit_message +from .linter.utils import remove_comments +from .messages import VALIDATION_FAILED, VALIDATION_SUCCESSFUL def get_args() -> argparse.Namespace: @@ -57,28 +58,43 @@ def get_args() -> argparse.Namespace: # --to-hash is optional parser.add_argument("--to-hash", type=str, help="To commit hash", default="HEAD") + # feature options + parser.add_argument( + "--quick", action="store_true", help="Check the commit message quickly." + ) + # parsing args args = parser.parse_args() return args -def _show_errors(commit_message: str, errors: List[str]) -> None: +def _show_errors( + commit_message: str, + errors: List[str], + short: bool = False, +) -> None: """ Display a formatted error message for a list of errors. Args: + commit_message (str): The commit message to display. errors (List[str]): A list of error messages to be displayed. + short (bool): Display the error message in short. + """ error_count = len(errors) commit_message = remove_comments(commit_message) - sys.stderr.write( - f"⧗ Input:\n{commit_message}\n\n✖ Found {error_count} error(s).\n\n" - ) - for index, error in enumerate(errors): - end_char = "" if index == error_count - 1 else "\n" - sys.stderr.write(f"- {error}\n{end_char}") + sys.stderr.write(f"⧗ Input:\n{commit_message}\n\n") + + if short: + sys.stderr.write(f"{VALIDATION_FAILED}\n") + return + + sys.stderr.write(f"✖ Found {error_count} error(s).\n") + for error in errors: + sys.stderr.write(f"- {error}\n") def _get_commit_message_from_file(filepath: str) -> str: @@ -101,26 +117,27 @@ def _get_commit_message_from_file(filepath: str) -> str: return commit_message -def _handle_commit_message(commit_message: str) -> None: +def _handle_commit_message(commit_message: str, quick: bool) -> None: """ Handles a single commit message, checks its validity, and prints the result. Args: commit_message (str): The commit message to be handled. + quick (bool): True for quick linting. Raises: SystemExit: If the commit message is invalid. """ - success, errors = check_commit_message(commit_message) + success, errors = lint_commit_message(commit_message, quick=quick) if success: sys.stdout.write(f"{VALIDATION_SUCCESSFUL}\n") else: - _show_errors(commit_message, errors) + _show_errors(commit_message, errors, short=quick) sys.exit(1) -def _handle_multiple_commit_messages(commit_messages: List[str]) -> None: +def _handle_multiple_commit_messages(commit_messages: List[str], quick: bool) -> None: """ Handles multiple commit messages, checks their validity, and prints the result. @@ -132,10 +149,10 @@ def _handle_multiple_commit_messages(commit_messages: List[str]) -> None: """ has_error = False for commit_message in commit_messages: - success, errors = check_commit_message(commit_message) + success, errors = lint_commit_message(commit_message, quick=quick) if not success: has_error = True - _show_errors(commit_message, errors) + _show_errors(commit_message, errors, short=quick) sys.stderr.write("\n") if has_error: @@ -153,18 +170,18 @@ def main() -> None: try: if args.file: commit_message = _get_commit_message_from_file(args.file) - _handle_commit_message(commit_message) + _handle_commit_message(commit_message, quick=args.quick) elif args.hash: commit_message = get_commit_message_of_hash(args.hash) - _handle_commit_message(commit_message) + _handle_commit_message(commit_message, quick=args.quick) elif args.from_hash: commit_messages = get_commit_messages_of_hash_range( args.from_hash, args.to_hash ) - _handle_multiple_commit_messages(commit_messages) + _handle_multiple_commit_messages(commit_messages, quick=args.quick) else: commit_message = args.commit_message.strip() - _handle_commit_message(commit_message) + _handle_commit_message(commit_message, quick=args.quick) except CommitlintException as ex: sys.stderr.write(f"{ex}\n") sys.exit(1) diff --git a/src/commitlint/commitlint.py b/src/commitlint/commitlint.py deleted file mode 100644 index 7b88954..0000000 --- a/src/commitlint/commitlint.py +++ /dev/null @@ -1,124 +0,0 @@ -""" -This module provides functionality for validating commit messages according -to conventional commit standards. - -Usage: ------- - -```python -from commitlint import check_commit_message - -commit_message = "feat(module): add module documentation" -success, errors = check_commit_message(commit_message) -``` -""" -import re -from typing import List, Tuple - -from .constants import COMMIT_HEADER_MAX_LENGTH -from .messages import HEADER_LENGTH_ERROR, INCORRECT_FORMAT_ERROR - -CONVENTIONAL_COMMIT_PATTERN = ( - r"(?s)" # To explicitly make . match new line - r"(?Pbuild|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)" - r"(?P\(\S+\))?!?:" - r"(?: (?P[^\n\r]+))" - r"((\n\n(?P.*))|(\s*))?$" -) - -IGNORED_PATTERN = ( - r"^((Merge pull request)|(Merge (.*?) into (.*?)|(Merge branch (.*?)))(?:\r?\n)*$)|" - r"^(Merge tag (.*?))(?:\r?\n)*$|" - r"^(R|r)evert (.*)|" - r"^(Merged (.*?)(in|into) (.*)|Merged PR (.*): (.*))$|" - r"^Merge remote-tracking branch(\s*)(.*)$|" - r"^Automatic merge(.*)$|" - r"^Auto-merged (.*?) into (.*)$" -) - - -def is_ignored(commit_message: str) -> bool: - """ - Checks if a commit message should be ignored. - - Some commit messages like merge, revert, auto merge, etc is ignored - from linting. - - Args: - commit_message (str): The commit message to check. - - Returns: - bool: True if the commit message should be ignored, False otherwise. - """ - return bool(re.match(IGNORED_PATTERN, commit_message)) - - -def remove_comments(msg: str) -> str: - """Removes comments from the commit message. - - For `git commit --verbose`, excluding the diff generated message, - for example: - - ```bash - ... - # ------------------------ >8 ------------------------ - # Do not modify or remove the line above. - # Everything below it will be ignored. - diff --git a/... b/... - ... - ``` - - Args: - msg(str): The commit message to remove comments. - - Returns: - str: The commit message without comments. - """ - - lines: List[str] = [] - for line in msg.split("\n"): - if "# ------------------------ >8 ------------------------" in line: - # ignoring all the verbose message below this line - break - if not line.startswith("#"): - lines.append(line) - - return "\n".join(lines) - - -def check_commit_message(commit_message: str) -> Tuple[bool, List[str]]: - """ - Checks the validity of a commit message. Returns success and error list. - - Args: - commit_message (str): The commit message to be validated. - - Returns: - Tuple[bool, List[str]]: Returns success as a first element and list - of errors on the second elements. If success is true, errors will be - empty. - """ - # default values - success = True - errors: List[str] = [] - - # removing unnecessary commit comments - commit_message = remove_comments(commit_message) - - # checking if commit message should be ignored - if is_ignored(commit_message): - return success, errors - - # checking the length of header - header = commit_message.split("\n").pop() - if len(header) > COMMIT_HEADER_MAX_LENGTH: - success = False - errors.append(HEADER_LENGTH_ERROR) - - # matching commit message with the commit pattern - pattern_match = re.match(CONVENTIONAL_COMMIT_PATTERN, commit_message) - if pattern_match is None: - success = False - errors.append(INCORRECT_FORMAT_ERROR) - - return success, errors diff --git a/src/commitlint/constants.py b/src/commitlint/constants.py index d66dc9f..c87faf1 100644 --- a/src/commitlint/constants.py +++ b/src/commitlint/constants.py @@ -1,3 +1,28 @@ """This module defines constants used throughout the application.""" COMMIT_HEADER_MAX_LENGTH = 72 + +COMMIT_TYPES = ( + "build", + "ci", + "docs", + "feat", + "fix", + "perf", + "refactor", + "style", + "test", + "chore", + "revert", + "bump", +) + +IGNORE_COMMIT_PATTERNS = ( + r"^((Merge pull request)|(Merge (.*?) into (.*?)|(Merge branch (.*?)))(?:\r?\n)*$)|" + r"^(Merge tag (.*?))(?:\r?\n)*$|" + r"^(R|r)evert (.*)|" + r"^(Merged (.*?)(in|into) (.*)|Merged PR (.*): (.*))$|" + r"^Merge remote-tracking branch(\s*)(.*)$|" + r"^Automatic merge(.*)$|" + r"^Auto-merged (.*?) into (.*)$" +) diff --git a/src/commitlint/git_helpers.py b/src/commitlint/git_helpers.py index 9b11135..c99f5c3 100644 --- a/src/commitlint/git_helpers.py +++ b/src/commitlint/git_helpers.py @@ -1,6 +1,7 @@ """ This module contains the git related helper functions. """ + import subprocess from typing import List diff --git a/src/commitlint/linter/__init__.py b/src/commitlint/linter/__init__.py new file mode 100644 index 0000000..2ce35df --- /dev/null +++ b/src/commitlint/linter/__init__.py @@ -0,0 +1,7 @@ +"""Main module for commit linters and validators""" + +from ._linter import lint_commit_message + +__all__ = [ + "lint_commit_message", +] diff --git a/src/commitlint/linter/_linter.py b/src/commitlint/linter/_linter.py new file mode 100644 index 0000000..fe4ed00 --- /dev/null +++ b/src/commitlint/linter/_linter.py @@ -0,0 +1,70 @@ +""" +This module provides detailed functionality for linting commit messages according +to conventional commit standards. +""" + +from typing import List, Tuple + +from .quick import quick_lint_commit_message +from .utils import is_ignored, remove_comments +from .validators import HeaderLengthValidator, PatternValidator + + +def lint_commit_message( + commit_message: str, quick: bool = False +) -> Tuple[bool, List[str]]: + """ + Lints a commit message. + + Args: + commit_message (str): The commit message to be linted. + quick (bool, optional): Whether to perform a quick linting (default is True). + + Returns: + Tuple[bool, List[str]]: A tuple containing a boolean indicating whether the + message passed linting and a list of linting errors or warnings, if any. + + """ + + # perform processing and pre checks + # removing unnecessary commit comments + commit_message = remove_comments(commit_message) + + # checking if commit message should be ignored + if is_ignored(commit_message): + return True, [] + + # for quick check + if quick: + return quick_lint_commit_message(commit_message) + + return _detailed_lint_commit_message(commit_message) + + +def _detailed_lint_commit_message(commit_message: str) -> Tuple[bool, List[str]]: + """ + Lints of a commit message in a detail. + + Args: + commit_message (str): The commit message to be linted. + + Returns: + Tuple[bool, List[str]]: Returns success as a first element and list of errors + on the second elements. If success is true, errors will be empty. + """ + # default values + success = True + errors: List[str] = [] + + # checking the length of header + header_length_validator = HeaderLengthValidator(commit_message) + if not header_length_validator.is_valid(): + success = False + errors.extend(header_length_validator.errors()) + + pattern_validator = PatternValidator(commit_message) + if not pattern_validator.is_valid(): + success = False + errors.extend(pattern_validator.errors()) + + return success, errors diff --git a/src/commitlint/linter/quick.py b/src/commitlint/linter/quick.py new file mode 100644 index 0000000..841b14e --- /dev/null +++ b/src/commitlint/linter/quick.py @@ -0,0 +1,46 @@ +""" +This module contains the basic linter for commit message. +It is called quick because it just fails or pass. +""" + +import re +from typing import List, Tuple + +from ..constants import COMMIT_TYPES +from ..messages import INCORRECT_FORMAT_ERROR +from .validators import HeaderLengthValidator + +RE_TYPES = "|".join(COMMIT_TYPES) + +CONVENTIONAL_COMMIT_PATTERN = ( + r"(?s)" # To explicitly make . match new line + rf"(?P{RE_TYPES})" + r"(?P\(\S+\))?!?:" + r"(?: (?P[^\s][^\n\r]+[^\.]))" + r"((\n\n(?P.*))|(\s*))?$" +) + + +def quick_lint_commit_message(commit_message: str) -> Tuple[bool, List[str]]: + """ + Checks the validity of a commit message. Returns success and error list. + + Args: + commit_message (str): The commit message to be validated. + + Returns: + Tuple[bool, List[str]]: Returns success as a first element and list + of errors on the second elements. If success is true, errors will be + empty. + """ + # checking the length of header + header_length_validator = HeaderLengthValidator(commit_message) + if not header_length_validator.is_valid(): + return False, header_length_validator.errors() + + # matching commit message with the commit pattern + pattern_match = re.match(CONVENTIONAL_COMMIT_PATTERN, commit_message) + if pattern_match is None: + return False, [INCORRECT_FORMAT_ERROR] + + return True, [] diff --git a/src/commitlint/linter/utils.py b/src/commitlint/linter/utils.py new file mode 100644 index 0000000..5bd2aa1 --- /dev/null +++ b/src/commitlint/linter/utils.py @@ -0,0 +1,57 @@ +""" +Contains the utility methods for linters. +""" + +import re +from typing import List + +from ..constants import IGNORE_COMMIT_PATTERNS + + +def is_ignored(commit_message: str) -> bool: + """ + Checks if a commit message should be ignored. + + Some commit messages like merge, revert, auto merge, etc is ignored + from linting. + + Args: + commit_message (str): The commit message to check. + + Returns: + bool: True if the commit message should be ignored, False otherwise. + """ + return bool(re.match(IGNORE_COMMIT_PATTERNS, commit_message)) + + +def remove_comments(msg: str) -> str: + """Removes comments from the commit message. + + For `git commit --verbose`, excluding the diff generated message, + for example: + + ```bash + ... + # ------------------------ >8 ------------------------ + # Do not modify or remove the line above. + # Everything below it will be ignored. + diff --git a/... b/... + ... + ``` + + Args: + msg(str): The commit message to remove comments. + + Returns: + str: The commit message without comments. + """ + + lines: List[str] = [] + for line in msg.split("\n"): + if "# ------------------------ >8 ------------------------" in line: + # ignoring all the verbose message below this line + break + if not line.startswith("#"): + lines.append(line) + + return "\n".join(lines) diff --git a/src/commitlint/linter/validators.py b/src/commitlint/linter/validators.py new file mode 100644 index 0000000..854fbab --- /dev/null +++ b/src/commitlint/linter/validators.py @@ -0,0 +1,254 @@ +"""Commit message validation module. + +This module provides functionality to validate commit messages according to +conventional commit standards. +""" + +import re +from abc import ABC, abstractmethod +from typing import List, Union + +from ..constants import COMMIT_HEADER_MAX_LENGTH, COMMIT_TYPES +from ..messages import ( + COMMIT_TYPE_INVALID_ERROR, + COMMIT_TYPE_MISSING_ERROR, + COMMIT_TYPE_SPACE_AFTER_ERROR, + DESCRIPTION_FULL_STOP_ERROR, + DESCRIPTION_LINE_BREAK_ERROR, + DESCRIPTION_MISSING_ERROR, + DESCRIPTION_MULTIPLE_SPACE_START_ERROR, + DESCRIPTION_NO_LEADING_SPACE_ERROR, + HEADER_LENGTH_ERROR, + INCORRECT_FORMAT_ERROR, + SCOPE_EMPTY_ERROR, + SCOPE_SPACE_AFTER_ERROR, + SCOPE_WHITESPACE_ERROR, +) + +CONVENTIONAL_COMMIT_PATTERN = ( + r"(?s)" # To explicitly make . match new line + r"(?P\w+\s*)?" + r"(?:\((?P[^\)]*)\)(?P\s*))?" + r"!?(?P:\s?)?" + r"(?:(?P[^\n\r]+))?" + r"(?P\n?\n?)" + r"(((?P.*))|(\s*))?$" +) + + +class CommitValidator(ABC): + """Abstract Base validator for commit message.""" + + def __init__(self, commit_message: str) -> None: + self._commit_message = commit_message + self._errors: List[str] = [] + + # start validation + self.validate() + + @abstractmethod + def validate(self) -> None: + """Performs the validation.""" + raise NotImplementedError # pragma: no cover + + def add_error(self, error: str) -> None: + """Adds an error to the list of errors.""" + self._errors.append(error) + + def is_valid(self) -> bool: + """Checks if there are any errors.""" + return len(self._errors) == 0 + + def errors(self) -> List[str]: + """Get the list of errors.""" + return self._errors + + @property + def commit_message(self) -> str: + """Gets the commit message.""" + return self._commit_message + + +class HeaderLengthValidator(CommitValidator): + """Validator for checking commit header length.""" + + def validate(self) -> None: + """ + Validates the length of the commit header. + + Returns: + None + """ + header = self._commit_message.split("\n").pop() + if len(header) > COMMIT_HEADER_MAX_LENGTH: + self.add_error(HEADER_LENGTH_ERROR) + + +class PatternValidator(CommitValidator): + """Validator for commit message using the conventional commit regex pattern.""" + + def validate(self) -> None: + """ + Validates the commit message using the regex pattern. + + Returns: + None + """ + + # Matching commit message with the commit pattern + pattern_match = re.match(CONVENTIONAL_COMMIT_PATTERN, self.commit_message) + if pattern_match is None or pattern_match.group("colon") is None: + self.add_error(INCORRECT_FORMAT_ERROR) + return + + self.re_match = pattern_match + + validators = [ + self.validate_commit_type, + self.validate_commit_type_no_space_after, + self.validate_scope, + self.validate_scope_no_space_after, + self.validate_description, + self.validate_description_no_multiple_whitespace, + self.validate_description_no_line_break, + self.validate_description_no_full_stop_at_end, + ] + + for validator in validators: + error = validator() + if error: + self.add_error(error) + + def validate_commit_type(self) -> Union[None, str]: + """ + Validates the commit type. + + Returns: + Union[None, str]: If the commit type is valid, returns None; otherwise, + returns an error message. + """ + commit_type = self.re_match.group("type") + if commit_type is None: + return COMMIT_TYPE_MISSING_ERROR + + commit_type = commit_type.strip() + if commit_type not in COMMIT_TYPES: + return COMMIT_TYPE_INVALID_ERROR % commit_type + + return None + + def validate_commit_type_no_space_after(self) -> Union[None, str]: + """ + Validates that there is no space after the commit type. + + Returns: + Union[None, str]: If there is no space after the commit type, returns + None; otherwise, returns an error message. + """ + commit_type = self.re_match.group("type") + + if commit_type and commit_type.endswith(" "): + commit_type = commit_type.strip() + return COMMIT_TYPE_SPACE_AFTER_ERROR + + return None + + def validate_scope(self) -> Union[None, str]: + """ + Validates the commit scope. + + Returns: + Union[None, str]: If the commit scope is valid, returns None; otherwise, + returns an error message. + """ + scope = self.re_match.group("scope") + if scope is not None: + if scope == "": + return SCOPE_EMPTY_ERROR + + if " " in scope: + return SCOPE_WHITESPACE_ERROR + + return None + + def validate_scope_no_space_after(self) -> Union[None, str]: + """ + Validates that there is no space after the commit scope. + + Returns: + Union[None, str]: If there is no space after the commit scope, returns + None; otherwise, returns an error message. + """ + scope_after_space = self.re_match.group("scope_after_space") + if scope_after_space and " " in scope_after_space: + return SCOPE_SPACE_AFTER_ERROR + + return None + + def validate_description(self) -> Union[None, str]: + """ + Validates the commit description. + + Returns: + Union[None, str]: If the description is valid, returns None; otherwise, + returns an error message. + """ + if not self.re_match.group("description"): + return DESCRIPTION_MISSING_ERROR + + if not self.re_match.group("colon").endswith(" "): + return DESCRIPTION_NO_LEADING_SPACE_ERROR + + return None + + def validate_description_no_multiple_whitespace( + self, + ) -> Union[None, str]: + """ + Validates that there are no multiple whitespace characters at the beginning of + the description. + + Returns: + Union[None, str]: If the description has no multiple whitespace characters + at the beginning, returns None; otherwise, returns an error message. + """ + if self.re_match.group("description") and self.re_match.group( + "description" + ).startswith(" "): + return DESCRIPTION_MULTIPLE_SPACE_START_ERROR + + return None + + def validate_description_no_line_break( + self, + ) -> Union[None, str]: + """ + Validates that the description has no line break at the end. + + Returns: + Union[None, str]: If there is no line break at the end of the + description, returns None; otherwise, returns an error message. + """ + if self.re_match.group("body_separation") == "\n" and self.re_match.group( + "body" + ): + return DESCRIPTION_LINE_BREAK_ERROR + + return None + + def validate_description_no_full_stop_at_end( + self, + ) -> Union[None, str]: + """ + Validates that the description doesn't have full stop at the end. + + Returns: + Union[None, str]: If there is no full stop at the end of the description, + returns None; otherwise, returns an error message. + """ + if self.re_match.group("description") and self.re_match.group( + "description" + ).strip().endswith("."): + return DESCRIPTION_FULL_STOP_ERROR + + return None diff --git a/src/commitlint/messages.py b/src/commitlint/messages.py index 86e3920..3f27c74 100644 --- a/src/commitlint/messages.py +++ b/src/commitlint/messages.py @@ -2,22 +2,29 @@ This module provides constant messages used in the application for various scenarios. """ -from .constants import COMMIT_HEADER_MAX_LENGTH +from .constants import COMMIT_HEADER_MAX_LENGTH, COMMIT_TYPES VALIDATION_SUCCESSFUL = "Commit validation: successful!" +VALIDATION_FAILED = "Commit validation: failed!" -CORRECT_OUTPUT_FORMAT = ( - "Correct commit format:\n" - "---------------------------------------\n" - "(): \n" - "---------------------------------------\n" - "For more details visit " - "https://www.conventionalcommits.org/en/v1.0.0/" -) INCORRECT_FORMAT_ERROR = ( - "Commit message does not follow conventional commits format." - f"\n{CORRECT_OUTPUT_FORMAT}" + "Commit message does not follow the Conventional Commits format." ) HEADER_LENGTH_ERROR = ( - f"Header must not be longer than {COMMIT_HEADER_MAX_LENGTH} characters." + f"Header length cannot exceed {COMMIT_HEADER_MAX_LENGTH} characters." +) +COMMIT_TYPE_MISSING_ERROR = "Type is missing." +COMMIT_TYPE_INVALID_ERROR = ( + f"Invalid type '%s'. Type must be one of: {', '.join(COMMIT_TYPES)}." +) +COMMIT_TYPE_SPACE_AFTER_ERROR = "There cannot be a space after the type." +SCOPE_EMPTY_ERROR = "Scope cannot be empty." +SCOPE_SPACE_AFTER_ERROR = "There cannot be a space after the scope." +SCOPE_WHITESPACE_ERROR = "Scope cannot contain spaces." +DESCRIPTION_NO_LEADING_SPACE_ERROR = "Description must have a leading space." +DESCRIPTION_MULTIPLE_SPACE_START_ERROR = ( + "Description cannot start with multiple spaces." ) +DESCRIPTION_LINE_BREAK_ERROR = "Description cannot contain line breaks." +DESCRIPTION_MISSING_ERROR = "Description is missing." +DESCRIPTION_FULL_STOP_ERROR = "Description cannot end with full stop." diff --git a/tests/test_cli.py b/tests/test_cli.py index 12b59e5..39d2ad1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,9 +3,13 @@ from unittest.mock import MagicMock, call, mock_open, patch -from src.commitlint.cli import get_args, main -from src.commitlint.exceptions import CommitlintException -from src.commitlint.messages import INCORRECT_FORMAT_ERROR, VALIDATION_SUCCESSFUL +from commitlint.cli import get_args, main +from commitlint.exceptions import CommitlintException +from commitlint.messages import ( + INCORRECT_FORMAT_ERROR, + VALIDATION_FAILED, + VALIDATION_SUCCESSFUL, +) class TestCLIGetArgs: @@ -64,17 +68,26 @@ def test__get_args__with_to_hash(self, *_): assert args.file is None assert args.hash is None + @patch( + "argparse.ArgumentParser.parse_args", + return_value=MagicMock(quick=True), + ) + def test__get_args__with_quick(self, *_): + args = get_args() + assert args.quick is True + class TestCLIMain: # main: commit_message @patch( - "src.commitlint.cli.get_args", + "commitlint.cli.get_args", return_value=MagicMock( commit_message="feat: valid commit message", file=None, hash=None, from_hash=None, + quick=False, ), ) @patch("sys.stdout.write") @@ -87,12 +100,32 @@ def test__main__valid_commit_message( mock_stdout_write.assert_called_with(f"{VALIDATION_SUCCESSFUL}\n") @patch( - "src.commitlint.cli.get_args", + "commitlint.cli.get_args", + return_value=MagicMock( + commit_message="feat: valid commit message", + file=None, + hash=None, + from_hash=None, + quick=True, + ), + ) + @patch("sys.stdout.write") + def test__main__valid_commit_message_using_quick( + self, + mock_stdout_write, + *_, + ): + main() + mock_stdout_write.assert_called_with(f"{VALIDATION_SUCCESSFUL}\n") + + @patch( + "commitlint.cli.get_args", return_value=MagicMock( commit_message="Invalid commit message", file=None, hash=None, from_hash=None, + quick=False, ), ) @patch("sys.stderr.write") @@ -107,16 +140,44 @@ def test__main__invalid_commit_message( mock_sys_exit.assert_called_with(1) mock_stderr_write.assert_has_calls( [ - call("⧗ Input:\nInvalid commit message\n\n✖ Found 1 error(s).\n\n"), + call("⧗ Input:\nInvalid commit message\n\n"), + call("✖ Found 1 error(s).\n"), call(f"- {INCORRECT_FORMAT_ERROR}\n"), ] ) + @patch( + "commitlint.cli.get_args", + return_value=MagicMock( + commit_message="Invalid commit message", + file=None, + hash=None, + from_hash=None, + quick=True, + ), + ) + @patch("sys.stderr.write") + @patch("sys.exit") + def test__main__invalid_commit_message_using_quick( + self, + mock_sys_exit, + mock_stderr_write, + *_, + ): + main() + mock_sys_exit.assert_called_with(1) + mock_stderr_write.assert_has_calls( + [ + call("⧗ Input:\nInvalid commit message\n\n"), + call(f"{VALIDATION_FAILED}\n"), + ] + ) + # main: file @patch( - "src.commitlint.cli.get_args", - return_value=MagicMock(file="path/to/file.txt"), + "commitlint.cli.get_args", + return_value=MagicMock(file="path/to/file.txt", quick=False), ) @patch("sys.stdout.write") @patch("builtins.open", mock_open(read_data="feat: valid commit message")) @@ -125,12 +186,12 @@ def test__main__valid_commit_message_with_file(self, mock_stdout_write, *_): mock_stdout_write.assert_called_with(f"{VALIDATION_SUCCESSFUL}\n") @patch( - "src.commitlint.cli.get_args", - return_value=MagicMock(file="path/to/file.txt"), + "commitlint.cli.get_args", + return_value=MagicMock(file="path/to/file.txt", quick=False), ) @patch("sys.stderr.write") @patch("sys.exit") - @patch("builtins.open", mock_open(read_data="Invalid commit message")) + @patch("builtins.open", mock_open(read_data="Invalid commit message 2")) def test__main__invalid_commit_message_with_file( self, mock_sys_exit, mock_stderr_write, *_ ): @@ -138,7 +199,8 @@ def test__main__invalid_commit_message_with_file( mock_sys_exit.assert_called_with(1) mock_stderr_write.assert_has_calls( [ - call("⧗ Input:\nInvalid commit message\n\n✖ Found 1 error(s).\n\n"), + call("⧗ Input:\nInvalid commit message 2\n\n"), + call("✖ Found 1 error(s).\n"), call(f"- {INCORRECT_FORMAT_ERROR}\n"), ] ) @@ -146,10 +208,10 @@ def test__main__invalid_commit_message_with_file( # main: hash @patch( - "src.commitlint.cli.get_args", - return_value=MagicMock(file=None, hash="commit_hash"), + "commitlint.cli.get_args", + return_value=MagicMock(file=None, hash="commit_hash", quick=False), ) - @patch("src.commitlint.cli.get_commit_message_of_hash") + @patch("commitlint.cli.get_commit_message_of_hash") @patch("sys.stdout.write") def test__main__valid_commit_message_with_hash( self, mock_stdout_write, mock_get_commit_message_of_hash, *_ @@ -159,10 +221,10 @@ def test__main__valid_commit_message_with_hash( mock_stdout_write.assert_called_with(f"{VALIDATION_SUCCESSFUL}\n") @patch( - "src.commitlint.cli.get_args", - return_value=MagicMock(file=None, hash="commit_hash"), + "commitlint.cli.get_args", + return_value=MagicMock(file=None, hash="commit_hash", quick=False), ) - @patch("src.commitlint.cli.get_commit_message_of_hash") + @patch("commitlint.cli.get_commit_message_of_hash") @patch("sys.stderr.write") @patch("sys.exit") def test__main__invalid_commit_message_with_hash( @@ -173,7 +235,8 @@ def test__main__invalid_commit_message_with_hash( mock_sys_exit.assert_called_with(1) mock_stderr_write.assert_has_calls( [ - call("⧗ Input:\nInvalid commit message\n\n✖ Found 1 error(s).\n\n"), + call("⧗ Input:\nInvalid commit message\n\n"), + call("✖ Found 1 error(s).\n"), call(f"- {INCORRECT_FORMAT_ERROR}\n"), ] ) @@ -181,15 +244,16 @@ def test__main__invalid_commit_message_with_hash( # main: from_hash and to_hash @patch( - "src.commitlint.cli.get_args", + "commitlint.cli.get_args", return_value=MagicMock( file=None, hash=None, from_hash="start_commit_hash", to_hash="end_commit_hash", + quick=False, ), ) - @patch("src.commitlint.cli.get_commit_messages_of_hash_range") + @patch("commitlint.cli.get_commit_messages_of_hash_range") @patch("sys.stdout.write") def test__main__valid_commit_message_with_hash_range( self, mock_stdout_write, mock_get_commit_messages, *_ @@ -202,16 +266,17 @@ def test__main__valid_commit_message_with_hash_range( mock_stdout_write.assert_called_with(f"{VALIDATION_SUCCESSFUL}\n") @patch( - "src.commitlint.cli.get_args", + "commitlint.cli.get_args", return_value=MagicMock( file=None, hash=None, from_hash="invalid_start_hash", to_hash="end_commit_hash", + quick=False, ), ) @patch("sys.stderr.write") - @patch("src.commitlint.cli.get_commit_messages_of_hash_range") + @patch("commitlint.cli.get_commit_messages_of_hash_range") @patch("sys.exit") def test__main__invalid_commit_message_with_hash_range( self, mock_sys_exit, mock_get_commit_messages, *_ @@ -232,14 +297,14 @@ def test__main__invalid_commit_message_with_hash_range( ), ) @patch( - "src.commitlint.cli.check_commit_message", + "commitlint.cli.lint_commit_message", ) @patch("sys.stderr.write") @patch("sys.exit") def test__main__handle_exceptions( - self, mock_sys_exit, mock_stderr_write, mock_check_commit_message, *_ + self, mock_sys_exit, mock_stderr_write, mock_lint_commit_message, *_ ): - mock_check_commit_message.side_effect = CommitlintException("Test message") + mock_lint_commit_message.side_effect = CommitlintException("Test message") main() mock_sys_exit.assert_called_with(1) mock_stderr_write.assert_called_with("Test message\n") diff --git a/tests/test_commitlint/test_check_commit_message.py b/tests/test_commitlint/test_check_commit_message.py deleted file mode 100644 index af784e2..0000000 --- a/tests/test_commitlint/test_check_commit_message.py +++ /dev/null @@ -1,94 +0,0 @@ -# type: ignore -# pylint: disable=all - -from src.commitlint import check_commit_message -from src.commitlint.constants import COMMIT_HEADER_MAX_LENGTH -from src.commitlint.messages import HEADER_LENGTH_ERROR, INCORRECT_FORMAT_ERROR - - -def test__check_commit_message__header_length_error(): - commit_message = "feat: " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1) - success, errors = check_commit_message(commit_message) - assert success is False - assert HEADER_LENGTH_ERROR in errors - - -def test__check_commit_message__header_length_valid(): - commit_message = "feat: " + "a" * (COMMIT_HEADER_MAX_LENGTH - 1) - success, errors = check_commit_message(commit_message) - assert success is False - assert HEADER_LENGTH_ERROR in errors - - -def test__check_commit_message__incorrect_format_error(): - commit_message = "This is an invalid commit message" - success, errors = check_commit_message(commit_message) - assert success is False - assert INCORRECT_FORMAT_ERROR in errors - - -def test__check_commit_message__incorrect_format_error_and_health_length_invalid(): - commit_message = "Test " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1) - success, errors = check_commit_message(commit_message) - assert success is False - assert HEADER_LENGTH_ERROR in errors - assert INCORRECT_FORMAT_ERROR in errors - - -def test__check_commit_message__valid(): - commit_message = "feat: add new feature" - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] - - -def test__check_commit_message__valid_with_scope(): - commit_message = "feat(scope): add new feature" - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] - - -def test__check_commit_message__empty_scope_error(): - commit_message = "feat(): add new feature" - success, errors = check_commit_message(commit_message) - assert success is False - assert INCORRECT_FORMAT_ERROR in errors - - -def test__check_commit_message__valid_with_body(): - commit_message = "fix(scope): fix a bug\n\nThis is the body of the commit message." - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] - - -def test__check_commit_message__header_line_error(): - commit_message = "feat(): add new feature\ntest" - success, errors = check_commit_message(commit_message) - assert success is False - assert INCORRECT_FORMAT_ERROR in errors - - -def test__check_commit_message__with_comments(): - commit_message = "feat(scope): add new feature\n#this is a comment" - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] - - -def test__check_commit_message__with_diff(): - commit_message = ( - "fix: fixed a bug\n\nthis is body\n" - "# ------------------------ >8 ------------------------\nDiff message" - ) - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] - - -def test__check_commit_message__ignored(): - commit_message = "Merge pull request #123" - success, errors = check_commit_message(commit_message) - assert success is True - assert errors == [] diff --git a/tests/test_git_helpers.py b/tests/test_git_helpers.py index c11bc72..01b3ecb 100644 --- a/tests/test_git_helpers.py +++ b/tests/test_git_helpers.py @@ -5,11 +5,11 @@ import pytest -from src.commitlint.exceptions import ( +from commitlint.exceptions import ( GitCommitNotFoundException, GitInvalidCommitRangeException, ) -from src.commitlint.git_helpers import ( +from commitlint.git_helpers import ( get_commit_message_of_hash, get_commit_messages_of_hash_range, ) @@ -19,7 +19,7 @@ @pytest.fixture def mock_subprocess(): - with patch("src.commitlint.git_helpers.subprocess") as mock_subprocess: + with patch("commitlint.git_helpers.subprocess") as mock_subprocess: mock_subprocess.PIPE = subprocess.PIPE mock_subprocess.CalledProcessError = subprocess.CalledProcessError yield mock_subprocess @@ -52,7 +52,7 @@ def test_get_commit_message_of_hash_failure(mock_subprocess): @patch( - "src.commitlint.git_helpers.get_commit_message_of_hash", + "commitlint.git_helpers.get_commit_message_of_hash", return_value="From commit message", ) def test_get_commit_messages_of_hash_range_success( @@ -82,7 +82,7 @@ def test_get_commit_messages_of_hash_range_success( @patch( - "src.commitlint.git_helpers.get_commit_message_of_hash", + "commitlint.git_helpers.get_commit_message_of_hash", return_value="From commit message", ) def test_get_commit_messages_of_hash_range_failure( diff --git a/tests/test_commitlint/__init__.py b/tests/test_linter/__init__.py similarity index 100% rename from tests/test_commitlint/__init__.py rename to tests/test_linter/__init__.py diff --git a/tests/test_linter/_test_data.py b/tests/test_linter/_test_data.py new file mode 100644 index 0000000..ee27553 --- /dev/null +++ b/tests/test_linter/_test_data.py @@ -0,0 +1,87 @@ +# type: ignore +# pylint: disable=all +from typing import List, Tuple + +from commitlint.constants import COMMIT_HEADER_MAX_LENGTH +from commitlint.messages import ( + COMMIT_TYPE_INVALID_ERROR, + COMMIT_TYPE_MISSING_ERROR, + COMMIT_TYPE_SPACE_AFTER_ERROR, + DESCRIPTION_FULL_STOP_ERROR, + DESCRIPTION_LINE_BREAK_ERROR, + DESCRIPTION_MISSING_ERROR, + DESCRIPTION_MULTIPLE_SPACE_START_ERROR, + DESCRIPTION_NO_LEADING_SPACE_ERROR, + HEADER_LENGTH_ERROR, + INCORRECT_FORMAT_ERROR, + SCOPE_EMPTY_ERROR, + SCOPE_SPACE_AFTER_ERROR, + SCOPE_WHITESPACE_ERROR, +) + +# LINTER_TEST_DATA contains test data for the functions `lint_commit_message` and +# `quick_lint_commit_message`. The purpose of creating this dataset is to ensure +# consistent results for both detailed and quick commit message linting processes. +# +# The data consists of tuples, each representing a test case with the following +# structure: +# (commit_message, expected_success, expected_errors) +# +# - commit_message: The commit message to be tested. +# - expected_success: A boolean indicating whether the commit message is expected to +# pass or fail the linting. +# - expected_errors: A list of error messages expected for detailed linting. +# +LINTER_TEST_DATA: Tuple[Tuple[str, bool, List[str], List[str]], ...] = ( + # success + ("feat: add new feature", True, []), + ("feat: add new feature\n\nthis is body", True, []), + ("feat: add new feature\n\nthis is body\n\ntest", True, []), + ("feat: add new feature\n", True, []), + ("build(deps-dev): bump @babel/traverse from 7.22.17 to 7.24.0", True, []), + ("feat(scope): add new feature\n#this is a comment", True, []), + ( + "fix: fixed a bug\n\nthis is body\n" + "# ------------------------ >8 ------------------------\nDiff message", + True, + [], + ), + ("Merge pull request #123", True, []), + # incorrect format check + ("feat add new feature", False, [INCORRECT_FORMAT_ERROR]), + # header length check + ("feat: " + "a" * (COMMIT_HEADER_MAX_LENGTH - 1), False, [HEADER_LENGTH_ERROR]), + ("feat: " + "a" * (COMMIT_HEADER_MAX_LENGTH - 1), False, [HEADER_LENGTH_ERROR]), + ( + "Test " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1), + False, + [HEADER_LENGTH_ERROR, INCORRECT_FORMAT_ERROR], + ), + # commit type check + (": add new feature", False, [COMMIT_TYPE_MISSING_ERROR]), + ("(invalid): add new feature", False, [COMMIT_TYPE_MISSING_ERROR]), + ("invalid: add new feature", False, [COMMIT_TYPE_INVALID_ERROR % "invalid"]), + ("feat (test): add new feature", False, [COMMIT_TYPE_SPACE_AFTER_ERROR]), + ( + "invalid (test): add new feature", + False, + [COMMIT_TYPE_INVALID_ERROR % "invalid", COMMIT_TYPE_SPACE_AFTER_ERROR], + ), + # scope check + ("feat(): add new feature", False, [SCOPE_EMPTY_ERROR]), + ("feat( ): add new feature", False, [SCOPE_WHITESPACE_ERROR]), + ("feat(hello world): add new feature", False, [SCOPE_WHITESPACE_ERROR]), + ("feat(test) : add new feature", False, [SCOPE_SPACE_AFTER_ERROR]), + ( + "feat (test) : add new feature", + False, + [COMMIT_TYPE_SPACE_AFTER_ERROR, SCOPE_SPACE_AFTER_ERROR], + ), + # description check + ("feat:add new feature", False, [DESCRIPTION_NO_LEADING_SPACE_ERROR]), + ("feat: add new feature", False, [DESCRIPTION_MULTIPLE_SPACE_START_ERROR]), + ("feat: add new feature\nhello baby", False, [DESCRIPTION_LINE_BREAK_ERROR]), + ("feat(test):", False, [DESCRIPTION_MISSING_ERROR]), + ("feat(test): ", False, [DESCRIPTION_MISSING_ERROR]), + ("feat(test): add new feature.", False, [DESCRIPTION_FULL_STOP_ERROR]), +) diff --git a/tests/test_linter/test_linter.py b/tests/test_linter/test_linter.py new file mode 100644 index 0000000..277b689 --- /dev/null +++ b/tests/test_linter/test_linter.py @@ -0,0 +1,17 @@ +# type: ignore +# pylint: disable=all +import pytest + +from commitlint.linter import lint_commit_message + +from ._test_data import LINTER_TEST_DATA + + +@pytest.mark.parametrize( + "commit_message, success, errors", + LINTER_TEST_DATA, +) +def test__lint_commit_message(commit_message, success, errors): + _success, _errors = lint_commit_message(commit_message, quick=False) + assert _success is success + assert _errors == errors diff --git a/tests/test_linter/test_quick.py b/tests/test_linter/test_quick.py new file mode 100644 index 0000000..feabf94 --- /dev/null +++ b/tests/test_linter/test_quick.py @@ -0,0 +1,32 @@ +# type: ignore +# pylint: disable=all +import pytest + +from commitlint.constants import COMMIT_HEADER_MAX_LENGTH +from commitlint.linter import lint_commit_message +from commitlint.messages import HEADER_LENGTH_ERROR, INCORRECT_FORMAT_ERROR + +from ._test_data import LINTER_TEST_DATA + + +@pytest.mark.parametrize( + "commit_message, success, _", + LINTER_TEST_DATA, +) +def test__quick_lint_commit_message(commit_message, success, _): + _success, _errors = lint_commit_message(commit_message, quick=True) + assert _success is success + + +def test__quick_lint_commit_message_returns_header_length_error_message(): + commit_message = "Test " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1) + success, errors = lint_commit_message(commit_message, quick=True) + assert success is False + assert errors == [HEADER_LENGTH_ERROR] + + +def test__quick_lint_commit_message_returns_invalid_format_error_message(): + commit_message = "Test invalid commit message" + success, errors = lint_commit_message(commit_message, quick=True) + assert success is False + assert errors == [INCORRECT_FORMAT_ERROR] diff --git a/tests/test_linter/test_utils/__init__.py b/tests/test_linter/test_utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_commitlint/test_is_ingored.py b/tests/test_linter/test_utils/test_is_ingored.py similarity index 95% rename from tests/test_commitlint/test_is_ingored.py rename to tests/test_linter/test_utils/test_is_ingored.py index 1c2b135..a7ff796 100644 --- a/tests/test_commitlint/test_is_ingored.py +++ b/tests/test_linter/test_utils/test_is_ingored.py @@ -3,7 +3,7 @@ import pytest -from src.commitlint.commitlint import is_ignored +from commitlint.linter.utils import is_ignored @pytest.mark.parametrize( diff --git a/tests/test_commitlint/test_remove_comments.py b/tests/test_linter/test_utils/test_remove_comments.py similarity index 95% rename from tests/test_commitlint/test_remove_comments.py rename to tests/test_linter/test_utils/test_remove_comments.py index 31ddef1..aeac3ef 100644 --- a/tests/test_commitlint/test_remove_comments.py +++ b/tests/test_linter/test_utils/test_remove_comments.py @@ -1,7 +1,7 @@ # type: ignore # pylint: disable=all -from src.commitlint.commitlint import remove_comments +from commitlint.linter.utils import remove_comments def test__remove_comments__no_comments():