From 8fc9baa1aa88863e94de92b32408cbddb4085b24 Mon Sep 17 00:00:00 2001 From: Suman Gole Date: Wed, 4 Dec 2024 14:19:17 +0545 Subject: [PATCH] fix: skip the header length check and configure header length --- src/commitlint/cli.py | 57 +++++++++++++++++++++++--- src/commitlint/linter/_linter.py | 33 ++++++++++++--- src/commitlint/linter/validators.py | 43 ++++++++++++-------- tests/test_cli.py | 43 ++++++++++++++++++++ tests/test_linter/test__linter.py | 63 ++++++++++++++++++++++++++--- 5 files changed, 204 insertions(+), 35 deletions(-) diff --git a/src/commitlint/cli.py b/src/commitlint/cli.py index f34e6c8..ed44194 100644 --- a/src/commitlint/cli.py +++ b/src/commitlint/cli.py @@ -59,7 +59,6 @@ def get_args() -> argparse.Namespace: group.add_argument("--from-hash", type=str, help="From commit hash") # --to-hash is optional parser.add_argument("--to-hash", type=str, help="To commit hash", default="HEAD") - # feature options parser.add_argument( "--skip-detail", @@ -85,6 +84,14 @@ def get_args() -> argparse.Namespace: default=False, ) + output_group.add_argument( + "--max-header-length", help="commit message header max length" + ) + output_group.add_argument( + "--disable-header-length-check", + action="store_true", + help="disable header max length check", + ) # --verbose option is optional output_group.add_argument( "-v", @@ -152,10 +159,13 @@ def _get_commit_message_from_file(filepath: str) -> str: return commit_message +# pylint: disable=too-many-arguments def _handle_commit_message( commit_message: str, skip_detail: bool, hide_input: bool, + max_header_length: int, + disable_max_header_length_check: bool = False, strip_comments: bool = False, ) -> None: """ @@ -171,7 +181,13 @@ def _handle_commit_message( Raises: SystemExit: If the commit message is invalid. """ - success, errors = lint_commit_message(commit_message, skip_detail, strip_comments) + success, errors = lint_commit_message( + commit_message=commit_message, + skip_detail=skip_detail, + strip_comments=strip_comments, + max_header_length=max_header_length, + disable_max_header_length=disable_max_header_length_check, + ) if success: console.success(VALIDATION_SUCCESSFUL) @@ -182,7 +198,11 @@ def _handle_commit_message( def _handle_multiple_commit_messages( - commit_messages: List[str], skip_detail: bool, hide_input: bool + commit_messages: List[str], + skip_detail: bool, + hide_input: bool, + disable_max_header_length_check: bool, + max_header_length: int, ) -> None: """ Handles multiple commit messages, checks their validity, and prints the result. @@ -198,7 +218,12 @@ def _handle_multiple_commit_messages( has_error = False for commit_message in commit_messages: - success, errors = lint_commit_message(commit_message, skip_detail) + success, errors = lint_commit_message( + commit_message, + max_header_length=max_header_length, + skip_detail=skip_detail, + disable_max_header_length=disable_max_header_length_check, + ) if success: console.verbose("lint success") continue @@ -224,6 +249,14 @@ def main() -> None: config.verbose = args.verbose console.verbose("starting commitlint") + + if args.max_header_length and args.disable_header_length_check: + console.error( + "--max-header-length and " + "--disable-header-length-check can't be passed together" + ) + raise CommitlintException + try: if args.file: console.verbose("commit message source: file") @@ -232,13 +265,19 @@ def main() -> None: commit_message, skip_detail=args.skip_detail, hide_input=args.hide_input, + disable_max_header_length_check=args.disable_header_length_check, + max_header_length=args.max_header_length, strip_comments=True, ) elif args.hash: console.verbose("commit message source: hash") commit_message = get_commit_message_of_hash(args.hash) _handle_commit_message( - commit_message, skip_detail=args.skip_detail, hide_input=args.hide_input + commit_message, + skip_detail=args.skip_detail, + hide_input=args.hide_input, + max_header_length=args.max_header_length, + disable_max_header_length_check=args.disable_header_length_check, ) elif args.from_hash: console.verbose("commit message source: hash range") @@ -249,12 +288,18 @@ def main() -> None: commit_messages, skip_detail=args.skip_detail, hide_input=args.hide_input, + max_header_length=args.max_header_length, + disable_max_header_length_check=args.disable_header_length_check, ) else: console.verbose("commit message source: direct message") commit_message = args.commit_message.strip() _handle_commit_message( - commit_message, skip_detail=args.skip_detail, hide_input=args.hide_input + commit_message, + skip_detail=args.skip_detail, + hide_input=args.hide_input, + max_header_length=args.max_header_length, + disable_max_header_length_check=args.disable_header_length_check, ) except CommitlintException as ex: console.error(f"{ex}") diff --git a/src/commitlint/linter/_linter.py b/src/commitlint/linter/_linter.py index a2a3455..3ffa15c 100644 --- a/src/commitlint/linter/_linter.py +++ b/src/commitlint/linter/_linter.py @@ -8,6 +8,7 @@ from .. import console from .utils import is_ignored, remove_comments from .validators import ( + CommitValidator, HeaderLengthValidator, PatternValidator, SimplePatternValidator, @@ -16,12 +17,18 @@ def lint_commit_message( - commit_message: str, skip_detail: bool = False, strip_comments: bool = False + commit_message: str, + max_header_length: int, + skip_detail: bool = False, + disable_max_header_length: bool = False, + strip_comments: bool = False, ) -> Tuple[bool, List[str]]: """ Lints a commit message. Args: + disable_max_header_length: flag to disable the max header length check + max_header_length: maximum length of commit message header commit_message (str): The commit message to be linted. skip_detail (bool, optional): Whether to skip the detailed error linting (default is False). @@ -47,16 +54,30 @@ def lint_commit_message( console.verbose("commit message ignored, skipping lint") return True, [] + validator_instances: list[CommitValidator] = [] + + if not disable_max_header_length: + validator_instances.append( + HeaderLengthValidator( + commit_message=commit_message, + **{"max_header_length": max_header_length}, # type: ignore + ) + ) + # for skip_detail check if skip_detail: console.verbose("running simple validators for linting") + + validator_instances.append( + SimplePatternValidator(commit_message=commit_message) + ) + return run_validators( - commit_message, - validator_classes=[HeaderLengthValidator, SimplePatternValidator], + validator_classes=validator_instances, fail_fast=True, ) + validator_instances.append(PatternValidator(commit_message=commit_message)) + console.verbose("running detailed validators for linting") - return run_validators( - commit_message, validator_classes=[HeaderLengthValidator, PatternValidator] - ) + return run_validators(validator_classes=validator_instances) diff --git a/src/commitlint/linter/validators.py b/src/commitlint/linter/validators.py index b7240f4..2f92a85 100644 --- a/src/commitlint/linter/validators.py +++ b/src/commitlint/linter/validators.py @@ -6,7 +6,7 @@ import re from abc import ABC, abstractmethod -from typing import List, Tuple, Type, Union +from typing import Any, List, Tuple, Union, cast from .. import console from ..constants import COMMIT_HEADER_MAX_LENGTH, COMMIT_TYPES @@ -30,15 +30,15 @@ class CommitValidator(ABC): """Abstract Base validator for commit message.""" - def __init__(self, commit_message: str) -> None: + def __init__(self, commit_message: str, **kwargs: dict[str, Any]) -> None: self._commit_message = commit_message self._errors: List[str] = [] # start validation - self.validate() + self.validate(**kwargs) @abstractmethod - def validate(self) -> None: + def validate(self, **kwargs: dict[str, Any]) -> None: """Performs the validation.""" raise NotImplementedError # pragma: no cover @@ -50,6 +50,7 @@ def is_valid(self) -> bool: """Checks if there are any errors.""" return len(self._errors) == 0 + @property def errors(self) -> List[str]: """Get the list of errors.""" return self._errors @@ -63,15 +64,23 @@ def commit_message(self) -> str: class HeaderLengthValidator(CommitValidator): """Validator for checking commit header length.""" - def validate(self) -> None: + def validate(self, **kwargs: dict[str, Any]) -> None: """ Validates the length of the commit header. Returns: None """ + max_header_length = kwargs.get("max_header_length") + + if max_header_length: + header_length = cast(int, max_header_length) + else: + header_length = COMMIT_HEADER_MAX_LENGTH + header = self.commit_message.split("\n")[0] - if len(header) > COMMIT_HEADER_MAX_LENGTH: + + if len(header) > int(header_length): self.add_error(HEADER_LENGTH_ERROR) @@ -90,7 +99,7 @@ class SimplePatternValidator(CommitValidator): r"((\n\n(?P.*))|(\s*))?$" ) - def validate(self) -> None: + def validate(self, **kwargs: dict[str, Any]) -> None: """ Validates the commit message using the regex pattern. @@ -118,7 +127,7 @@ class PatternValidator(CommitValidator): r"(((?P.*))|(\s*))?$" ) - def validate(self) -> None: + def validate(self, **kwargs: dict[str, Any]) -> None: """ Validates the commit message using the regex pattern. @@ -286,8 +295,7 @@ def validate_description_no_full_stop_at_end( def run_validators( - commit_message: str, - validator_classes: List[Type[CommitValidator]], + validator_classes: List[CommitValidator], fail_fast: bool = False, ) -> Tuple[bool, List[str]]: """Runs the provided validators for the commit message. @@ -307,17 +315,18 @@ def run_validators( success = True errors: List[str] = [] - for validator_class in validator_classes: - console.verbose(f"running validator {validator_class.__name__}") - validator = validator_class(commit_message) - if not validator.is_valid(): - console.verbose(f"{validator_class.__name__}: validation failed") + for validator_instance in validator_classes: + console.verbose(f"running validator {validator_instance.__class__.__name__}") + if not validator_instance.is_valid(): + console.verbose( + f"{validator_instance.__class__.__name__}: validation failed" + ) if fail_fast: console.verbose(f"fail_fast: {fail_fast}, skipping further validations") # returning immediately if any error occurs. - return False, validator.errors() + return False, validator_instance.errors success = False - errors.extend(validator.errors()) + errors.extend(validator_instance.errors) return success, errors diff --git a/tests/test_cli.py b/tests/test_cli.py index 17cc7b1..0f922fb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -459,3 +459,46 @@ def test__invalid_commit_message_with_hash_range_in_quiet( mock_stderr_write.assert_not_called() mock_stdout_write.assert_not_called() + + +class TestCliHeaderLength: + @patch( + "commitlint.cli.get_args", + return_value=ArgsMock( + commit_message="feat: add method 'formatEnglishDate' \ + and 'formatEnglishDateInNepali' (#165)", + quiet=True, + max_header_length=20, + ), + ) + @patch("sys.stdout.write") + @patch("sys.stderr.write") + def test__main__quiet_option_with_header_max_length( + self, mock_stderr_write, mock_stdout_write, *_ + ): + with pytest.raises(SystemExit): + main() + + mock_stderr_write.assert_not_called() + mock_stdout_write.assert_not_called() + + @patch( + "commitlint.cli.get_args", + return_value=ArgsMock( + commit_message="feat: add method 'formatEnglishDate' \ + and 'formatEnglishDateInNepali' (#165)", + quiet=True, + max_header_length=20, + disable_header_length_check=True, + ), + ) + @patch("sys.stdout.write") + @patch("sys.stderr.write") + def test__with_both_header_max_length_and_disabled_max_header_length_check( + self, mock_stderr_write, mock_stdout_write, *_ + ): + with pytest.raises(CommitlintException): + main() + + mock_stderr_write.assert_not_called() + mock_stdout_write.assert_not_called() diff --git a/tests/test_linter/test__linter.py b/tests/test_linter/test__linter.py index e994f64..bfcf06f 100644 --- a/tests/test_linter/test__linter.py +++ b/tests/test_linter/test__linter.py @@ -19,20 +19,35 @@ def fixture_data(request): def test_lint_commit_message(fixture_data): commit_message, expected_success, expected_errors = fixture_data - success, errors = lint_commit_message(commit_message, skip_detail=False) + success, errors = lint_commit_message( + commit_message, + skip_detail=False, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) assert success == expected_success assert errors == expected_errors def test__lint_commit_message__skip_detail(fixture_data): commit_message, expected_success, _ = fixture_data - success, _ = lint_commit_message(commit_message, skip_detail=True) + success, _ = lint_commit_message( + commit_message, + skip_detail=True, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) assert success == expected_success def test__lint_commit_message__remove_comments_if_strip_comments_is_True(): commit_message = "feat(scope): add new feature\n#this is a comment" - success, errors = lint_commit_message(commit_message, strip_comments=True) + success, errors = lint_commit_message( + commit_message, + strip_comments=True, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) assert success is True assert errors == [] @@ -43,19 +58,55 @@ def test__lint_commit_message__calls_remove_comments_if_strip_comments_is_True( ): commit_message = "feat(scope): add new feature" mock_remove_comments.return_value = commit_message - lint_commit_message(commit_message, strip_comments=True) + lint_commit_message( + commit_message, + strip_comments=True, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) mock_remove_comments.assert_called_once() def test__lint_commit_message__skip_detail_returns_header_length_error_message(): commit_message = "Test " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1) - success, errors = lint_commit_message(commit_message, skip_detail=True) + success, errors = lint_commit_message( + commit_message, + skip_detail=True, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) assert success is False assert errors == [HEADER_LENGTH_ERROR] def test__lint_commit_message__skip_detail_returns_invalid_format_error_message(): commit_message = "Test invalid commit message" - success, errors = lint_commit_message(commit_message, skip_detail=True) + success, errors = lint_commit_message( + commit_message, + skip_detail=True, + disable_max_header_length=False, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) assert success is False assert errors == [INCORRECT_FORMAT_ERROR] + + +def test__disable_header_length_check(): + commit_message = "feat: this is test for disabling the header length check (#77)" + + success, errors = lint_commit_message( + commit_message, + skip_detail=True, + disable_max_header_length=True, + max_header_length=COMMIT_HEADER_MAX_LENGTH, + ) + assert success is True + + +def test__max_header_length_test(): + commit_message = "feat: this is test for disabling the header length check (#77)" + success, errors = lint_commit_message( + commit_message, skip_detail=True, max_header_length=20 + ) + assert success is False + assert errors == [HEADER_LENGTH_ERROR]