From fdcdaecc2c498309b4736549d59e35404dc2fc76 Mon Sep 17 00:00:00 2001 From: SARAMALI15792 Date: Tue, 31 Mar 2026 11:54:37 +0500 Subject: [PATCH] feat: validate message_length_limit is non-negative Add validation to ensure message_length_limit is a non-negative integer (>= 0). Negative values now raise InvalidCommandArgumentError instead of being treated as "no limit". Changes: - Add validation in Check and Commit __init__ to reject negative values - Change condition from `<= 0` to `== 0` for "no limit" check - Update CommitArgs and CheckArgs to accept int | None for CLI override - Add comprehensive test coverage for negative value rejection - Update documentation to clarify non-negative requirement Behavior: - message_length_limit < 0: raises InvalidCommandArgumentError - message_length_limit = 0: disables length limit (no limit) - message_length_limit > 0: enforces the specified limit - CLI None value: falls back to config setting Fixes #1909 Co-Authored-By: Claude Opus 4.6 --- commitizen/commands/check.py | 16 ++++-- commitizen/commands/commit.py | 25 ++++++--- docs/config/check.md | 1 + docs/config/commit.md | 11 ++++ tests/commands/test_check_command.py | 37 +++++++++++-- tests/commands/test_commit_command.py | 80 +++++++++++++++++++++------ 6 files changed, 136 insertions(+), 34 deletions(-) diff --git a/commitizen/commands/check.py b/commitizen/commands/check.py index ab5e671d67..1764f63b7d 100644 --- a/commitizen/commands/check.py +++ b/commitizen/commands/check.py @@ -21,7 +21,7 @@ class CheckArgs(TypedDict, total=False): commit_msg: str rev_range: str allow_abort: bool - message_length_limit: int + message_length_limit: int | None allowed_prefixes: list[str] message: str use_default_range: bool @@ -46,9 +46,17 @@ def __init__(self, config: BaseConfig, arguments: CheckArgs, *args: object) -> N ) self.use_default_range = bool(arguments.get("use_default_range")) - self.max_msg_length = arguments.get( - "message_length_limit", config.settings.get("message_length_limit", 0) + + message_length_limit = arguments.get("message_length_limit") + self.message_length_limit: int = ( + message_length_limit + if message_length_limit is not None + else config.settings["message_length_limit"] ) + if self.message_length_limit < 0: + raise InvalidCommandArgumentError( + "message_length_limit must be a non-negative integer" + ) # we need to distinguish between None and [], which is a valid value allowed_prefixes = arguments.get("allowed_prefixes") @@ -100,7 +108,7 @@ def __call__(self) -> None: pattern=pattern, allow_abort=self.allow_abort, allowed_prefixes=self.allowed_prefixes, - max_msg_length=self.max_msg_length, + max_msg_length=self.message_length_limit, commit_hash=commit.rev, ) ).is_valid diff --git a/commitizen/commands/commit.py b/commitizen/commands/commit.py index 6668c0d65b..05fa7448c7 100644 --- a/commitizen/commands/commit.py +++ b/commitizen/commands/commit.py @@ -18,6 +18,7 @@ CommitMessageLengthExceededError, CustomError, DryRunExit, + InvalidCommandArgumentError, NoAnswersError, NoCommitBackupError, NotAGitProjectError, @@ -35,7 +36,7 @@ class CommitArgs(TypedDict, total=False): dry_run: bool edit: bool extra_cli_args: str - message_length_limit: int + message_length_limit: int | None no_retry: bool signoff: bool write_message_to_file: Path | None @@ -54,6 +55,17 @@ def __init__(self, config: BaseConfig, arguments: CommitArgs) -> None: self.arguments = arguments self.backup_file_path = get_backup_file_path() + message_length_limit = arguments.get("message_length_limit") + self.message_length_limit: int = ( + message_length_limit + if message_length_limit is not None + else config.settings["message_length_limit"] + ) + if self.message_length_limit < 0: + raise InvalidCommandArgumentError( + "message_length_limit must be a non-negative integer" + ) + def _read_backup_message(self) -> str | None: # Check the commit backup file exists if not self.backup_file_path.is_file(): @@ -85,19 +97,14 @@ def _get_message_by_prompt_commit_questions(self) -> str: return message def _validate_subject_length(self, message: str) -> None: - message_length_limit = self.arguments.get( - "message_length_limit", self.config.settings.get("message_length_limit", 0) - ) # By the contract, message_length_limit is set to 0 for no limit - if ( - message_length_limit is None or message_length_limit <= 0 - ): # do nothing for no limit + if self.message_length_limit == 0: return subject = message.partition("\n")[0].strip() - if len(subject) > message_length_limit: + if len(subject) > self.message_length_limit: raise CommitMessageLengthExceededError( - f"Length of commit message exceeds limit ({len(subject)}/{message_length_limit}), subject: '{subject}'" + f"Length of commit message exceeds limit ({len(subject)}/{self.message_length_limit}), subject: '{subject}'" ) def manual_edit(self, message: str) -> str: diff --git a/docs/config/check.md b/docs/config/check.md index 2c8dda27b4..d65df1657a 100644 --- a/docs/config/check.md +++ b/docs/config/check.md @@ -22,6 +22,7 @@ List of prefixes that commitizen ignores when verifying messages. - Default: `0` (no limit) Maximum length of the commit message. Setting it to `0` disables the length limit. +This value must be a non-negative integer (`>= 0`). !!! note This option can be overridden by the `-l/--message-length-limit` command line argument. diff --git a/docs/config/commit.md b/docs/config/commit.md index 72fdff947c..551f6b82a3 100644 --- a/docs/config/commit.md +++ b/docs/config/commit.md @@ -24,3 +24,14 @@ Sets the character encoding to be used when parsing commit messages. - Default: `False` Retries failed commit when running `cz commit`. + +## `message_length_limit` + +- Type: `int` +- Default: `0` (no limit) + +Maximum length of the commit message. Setting it to `0` disables the length limit. +This value must be a non-negative integer (`>= 0`). + +!!! note + This option can be overridden by the `-l/--message-length-limit` command line argument. diff --git a/tests/commands/test_check_command.py b/tests/commands/test_check_command.py index f3f860313d..00d9353db4 100644 --- a/tests/commands/test_check_command.py +++ b/tests/commands/test_check_command.py @@ -351,23 +351,27 @@ def test_check_command_with_amend_prefix_default(config, success_mock): success_mock.assert_called_once() -def test_check_command_with_config_message_length_limit(config, success_mock): +def test_check_command_with_config_message_length_limit_and_cli_none( + config, success_mock: MockType +): message = "fix(scope): some commit message" config.settings["message_length_limit"] = len(message) + 1 commands.Check( config=config, - arguments={"message": message}, + arguments={"message": message, "message_length_limit": None}, )() success_mock.assert_called_once() -def test_check_command_with_config_message_length_limit_exceeded(config): +def test_check_command_with_config_message_length_limit_exceeded_and_cli_none( + config, +): message = "fix(scope): some commit message" config.settings["message_length_limit"] = len(message) - 1 with pytest.raises(CommitMessageLengthExceededError): commands.Check( config=config, - arguments={"message": message}, + arguments={"message": message, "message_length_limit": None}, )() @@ -376,7 +380,7 @@ def test_check_command_cli_overrides_config_message_length_limit( ): message = "fix(scope): some commit message" config.settings["message_length_limit"] = len(message) - 1 - for message_length_limit in [len(message) + 1, 0]: + for message_length_limit in [len(message), 0]: success_mock.reset_mock() commands.Check( config=config, @@ -388,6 +392,29 @@ def test_check_command_cli_overrides_config_message_length_limit( success_mock.assert_called_once() +def test_check_command_with_negative_cli_message_length_limit_raises(config): + with pytest.raises(InvalidCommandArgumentError): + commands.Check( + config=config, + arguments={ + "message": "fix(scope): some commit message", + "message_length_limit": -1, + }, + ) + + +def test_check_command_with_negative_config_message_length_limit_raises(config): + config.settings["message_length_limit"] = -1 + with pytest.raises(InvalidCommandArgumentError): + commands.Check( + config=config, + arguments={ + "message": "fix(scope): some commit message", + "message_length_limit": None, + }, + ) + + class ValidationCz(BaseCommitizen): def questions(self) -> list[CzQuestion]: return [ diff --git a/tests/commands/test_commit_command.py b/tests/commands/test_commit_command.py index 2322cb3cb6..e7a3ded2a9 100644 --- a/tests/commands/test_commit_command.py +++ b/tests/commands/test_commit_command.py @@ -12,6 +12,7 @@ CommitMessageLengthExceededError, CustomError, DryRunExit, + InvalidCommandArgumentError, NoAnswersError, NoCommitBackupError, NotAGitProjectError, @@ -336,34 +337,81 @@ def test_commit_when_nothing_added_to_commit(config, mocker: MockFixture, out): error_mock.assert_called_once_with(out) -@pytest.mark.usefixtures("staging_is_clean", "commit_mock") -def test_commit_command_with_config_message_length_limit( - config, success_mock: MockType, prompt_mock_feat: MockType -): +def _commit_first_line_len(prompt_mock_feat: MockType) -> int: prefix = prompt_mock_feat.return_value["prefix"] subject = prompt_mock_feat.return_value["subject"] - message_length = len(f"{prefix}: {subject}") + scope = prompt_mock_feat.return_value["scope"] + + formatted_scope = f"({scope})" if scope else "" + first_line = f"{prefix}{formatted_scope}: {subject}" + return len(first_line) + - commands.Commit(config, {"message_length_limit": message_length})() +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_cli_at_limit_succeeds( + config, success_mock: MockType, prompt_mock_feat: MockType +): + message_len = _commit_first_line_len(prompt_mock_feat) + commands.Commit(config, {"message_length_limit": message_len})() success_mock.assert_called_once() + +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_cli_below_limit_raises( + config, prompt_mock_feat: MockType +): + message_len = _commit_first_line_len(prompt_mock_feat) with pytest.raises(CommitMessageLengthExceededError): - commands.Commit(config, {"message_length_limit": message_length - 1})() + commands.Commit(config, {"message_length_limit": message_len - 1})() - config.settings["message_length_limit"] = message_length - success_mock.reset_mock() - commands.Commit(config, {})() + +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_uses_config_when_cli_unset( + config, success_mock: MockType, prompt_mock_feat: MockType +): + config.settings["message_length_limit"] = _commit_first_line_len(prompt_mock_feat) + commands.Commit(config, {"message_length_limit": None})() success_mock.assert_called_once() - config.settings["message_length_limit"] = message_length - 1 + +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_config_exceeded_when_cli_unset( + config, prompt_mock_feat: MockType +): + config.settings["message_length_limit"] = ( + _commit_first_line_len(prompt_mock_feat) - 1 + ) with pytest.raises(CommitMessageLengthExceededError): - commands.Commit(config, {})() + commands.Commit(config, {"message_length_limit": None})() + - # Test config message length limit is overridden by CLI argument - success_mock.reset_mock() - commands.Commit(config, {"message_length_limit": message_length})() +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_cli_overrides_stricter_config( + config, success_mock: MockType, prompt_mock_feat: MockType +): + message_len = _commit_first_line_len(prompt_mock_feat) + config.settings["message_length_limit"] = message_len - 1 + commands.Commit(config, {"message_length_limit": message_len})() success_mock.assert_called_once() - success_mock.reset_mock() + +@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat") +def test_commit_message_length_cli_zero_disables_limit( + config, success_mock: MockType, prompt_mock_feat: MockType +): + config.settings["message_length_limit"] = ( + _commit_first_line_len(prompt_mock_feat) - 1 + ) commands.Commit(config, {"message_length_limit": 0})() success_mock.assert_called_once() + + +def test_commit_message_length_cli_negative_raises(config): + with pytest.raises(InvalidCommandArgumentError): + commands.Commit(config, {"message_length_limit": -1}) + + +def test_commit_message_length_config_negative_raises_when_cli_unset(config): + config.settings["message_length_limit"] = -1 + with pytest.raises(InvalidCommandArgumentError): + commands.Commit(config, {"message_length_limit": None})