Repository: commitizen-tools/commitizen

Issue:
Aborted Commit Considered to Fail Validation When Run As commit-msg Hook
## Description

Git interprets an empty commit message to mean that a commit should be aborted, but calls all commit-msg hooks anyways. This causes pre-commit hooks that run as commit-msg hooks but don't guard against empty commit messages to spew errors en masse when the commit was already being intentionally aborted.

## Steps to reproduce
1. Configure commitizen to run as a commit-msg hook using pre-commit as described below.
2. `touch foo && git add && git commit`
3. Save an empty commit message.

For step 1, add the following to `.pre-commit-config.yaml`:

```
yaml
  - repo: https://github.com/commitizen-tools/commitizen
    rev: v2.18.1
    hooks:
      - id: commitizen
        stages:
          - commit-msg
```

## Current behavior

[Pre-commit follows git's lead and calls commitizen when the commit message is empty when run as a commit-msg hook](https://github.com/pre-commit/pre-commit/issues/2052). This leads to the following error:

```
commitizen check..............................................................Failed
- hook id: commitizen
- exit code: 14

commit validation: failed!
please enter a commit message in the commitizen format.
commit "": "# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch foo
#
# Changes to be committed:
#       ,,,
pattern: (build|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)(\(\S+\))?!?:(\s.*)
```

The commit is still correctly aborted of course.

## Desired behavior

I expected the commitizen commit-msg hook to silently report success in this case since there is no commit message to check.

## Environment

- commitizen version: 2.18.1
- python version: Python 3.9.5
- operating system: Linux


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#512](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (cd8c895) into [master](https://codecov.io/gh/commitizen-tools/commitizen/commit/8dcf97f7f4dd2e7e02e2bc65dfbdcb08ac0705ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (8dcf97f) will **decrease** coverage by `0.06%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #512      +/-   ##
==========================================
- Coverage   97.85%   97.79%   -0.07%     
==========================================
  Files          39       39              
  Lines        1539     1542       +3     
==========================================
+ Hits         1506     1508       +2     
- Misses         33       34       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `97.79% <100.00%> (-0.07%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [commitizen/cli.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/512/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jbGkucHk=) | `92.42% <ø> (-1.52%)` | :arrow_down: |
| [commitizen/commands/check.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/512/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jb21tYW5kcy9jaGVjay5weQ==) | `100.00% <100.00%> (ø)` | |
| [commitizen/cz/\_\_init\_\_.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/512/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jei9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |
| [commitizen/defaults.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/512/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9kZWZhdWx0cy5weQ==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [354f9ea...cd8c895](https://codecov.io/gh/commitizen-tools/commitizen/pull/512?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

Woile: LGTM but `cz check` is not my speciality, can u take a look @Lee-W ?
Kurt-von-Laven: I am not able to reproduce the failure on Python 3.10 locally on Python 3.10.4. It appears to be unrelated to this pull request, so I think the build may simply need to be re-run. Please let me know if anyone sees a connection I am missing.
Lee-W: hmmm. we might need to take a deeper look at the 3.10 case
Kurt-von-Laven: Looks like the Python 3.10 build indeed simply needed to be re-run since all I changed was the two comments.

Patch:
diff --git a/commitizen/cli.py b/commitizen/cli.py
index bf751b44..d9f1025b 100644
--- a/commitizen/cli.py
+++ b/commitizen/cli.py
@@ -240,6 +240,12 @@ data = {
                         "help": "commit message that needs to be checked",
                         "exclusive_group": "group1",
                     },
+                    {
+                        "name": ["--allow-abort"],
+                        "action": "store_true",
+                        "default": False,
+                        "help": "allow empty commit messages, which typically abort a commit",
+                    },
                 ],
             },
             {
diff --git a/commitizen/commands/check.py b/commitizen/commands/check.py
index a32613e7..fb79e805 100644
--- a/commitizen/commands/check.py
+++ b/commitizen/commands/check.py
@@ -26,6 +26,9 @@ class Check:
         self.commit_msg_file: Optional[str] = arguments.get("commit_msg_file")
         self.commit_msg: Optional[str] = arguments.get("message")
         self.rev_range: Optional[str] = arguments.get("rev_range")
+        self.allow_abort: bool = bool(
+            arguments.get("allow_abort", config.settings["allow_abort"])
+        )
 
         self._valid_command_argument()
 
@@ -33,15 +36,16 @@ class Check:
         self.cz = factory.commiter_factory(self.config)
 
     def _valid_command_argument(self):
-        number_args_provided = (
-            bool(self.commit_msg_file) + bool(self.commit_msg) + bool(self.rev_range)
+        num_exclusive_args_provided = sum(
+            arg is not None
+            for arg in (self.commit_msg_file, self.commit_msg, self.rev_range)
         )
-        if number_args_provided == 0 and not os.isatty(0):
+        if num_exclusive_args_provided == 0 and not os.isatty(0):
             self.commit_msg: Optional[str] = sys.stdin.read()
-        elif number_args_provided != 1:
+        elif num_exclusive_args_provided != 1:
             raise InvalidCommandArgumentError(
                 (
-                    "One and only one argument is required for check command! "
+                    "Only one of --rev-range, --message, and --commit-msg-file is permitted by check command! "
                     "See 'cz check -h' for more information"
                 )
             )
@@ -60,7 +64,7 @@ class Check:
         ill_formated_commits = [
             commit
             for commit in commits
-            if not Check.validate_commit_message(commit.message, pattern)
+            if not self.validate_commit_message(commit.message, pattern)
         ]
         displayed_msgs_content = "\n".join(
             [
@@ -79,7 +83,8 @@ class Check:
 
     def _get_commits(self):
         # Get commit message from file (--commit-msg-file)
-        if self.commit_msg_file:
+        if self.commit_msg_file is not None:
+            # Enter this branch if commit_msg_file is "".
             with open(self.commit_msg_file, "r", encoding="utf-8") as commit_file:
                 msg = commit_file.read()
             msg = self._filter_comments(msg)
@@ -87,7 +92,8 @@ class Check:
             commit_title = msg.split("\n")[0]
             commit_body = "\n".join(msg.split("\n")[1:])
             return [git.GitCommit(rev="", title=commit_title, body=commit_body)]
-        elif self.commit_msg:
+        elif self.commit_msg is not None:
+            # Enter this branch if commit_msg is "".
             self.commit_msg = self._filter_comments(self.commit_msg)
             return [git.GitCommit(rev="", title="", body=self.commit_msg)]
 
@@ -98,8 +104,9 @@ class Check:
         lines = [line for line in msg.split("\n") if not line.startswith("#")]
         return "\n".join(lines)
 
-    @staticmethod
-    def validate_commit_message(commit_msg: str, pattern: str) -> bool:
+    def validate_commit_message(self, commit_msg: str, pattern: str) -> bool:
+        if not commit_msg:
+            return self.allow_abort
         if (
             commit_msg.startswith("Merge")
             or commit_msg.startswith("Revert")
diff --git a/commitizen/cz/__init__.py b/commitizen/cz/__init__.py
index f141e1c2..e14cb9f7 100644
--- a/commitizen/cz/__init__.py
+++ b/commitizen/cz/__init__.py
@@ -20,7 +20,7 @@ def discover_plugins(path: Iterable[str] = None) -> Dict[str, Type[BaseCommitize
         Dict[str, Type[BaseCommitizen]]: Registry with found plugins
     """
     plugins = {}
-    for finder, name, ispkg in pkgutil.iter_modules(path):
+    for _finder, name, _ispkg in pkgutil.iter_modules(path):
         try:
             if name.startswith("cz_"):
                 plugins[name] = importlib.import_module(name).discover_this
diff --git a/commitizen/defaults.py b/commitizen/defaults.py
index 0b217bf7..35a518ad 100644
--- a/commitizen/defaults.py
+++ b/commitizen/defaults.py
@@ -31,6 +31,7 @@ class Settings(TypedDict, total=False):
     version_files: List[str]
     tag_format: Optional[str]
     bump_message: Optional[str]
+    allow_abort: bool
     changelog_file: str
     changelog_incremental: bool
     changelog_start_rev: Optional[str]
@@ -56,6 +57,7 @@ DEFAULT_SETTINGS: Settings = {
     "version_files": [],
     "tag_format": None,  # example v$version
     "bump_message": None,  # bumped v$current_version to $new_version
+    "allow_abort": False,
     "changelog_file": "CHANGELOG.md",
     "changelog_incremental": False,
     "changelog_start_rev": None,
diff --git a/docs/check.md b/docs/check.md
index e1f0344e..759d60d9 100644
--- a/docs/check.md
+++ b/docs/check.md
@@ -7,7 +7,11 @@ If you want to setup an automatic check before every git commit, please refer to
 [Automatically check message before commit](auto_check.md).
 
 ## Usage
-There are three arguments that you can use one of them to check commit message.
+There are three mutually exclusive ways to use `cz check`:
+
+- with `--rev-range` to check a range of pre-existing commits
+- with `--message` or by piping the message to it to check a given string
+- or with `--commit-msg-file` to read the commit message from a file
 
 ### Git Rev Range
 If you'd like to check a commit's message after it has already been created, then you can specify the range of commits to check with `--rev-range REV_RANGE`.
@@ -46,3 +50,12 @@ $ cz check --commit-msg-file COMMIT_MSG_FILE
 
 In this option, COMMIT_MSG_FILE is the path of the temporal file that contains the commit message.
 This argument can be useful when cooperating with git hook, please check [Automatically check message before commit](auto_check.md) for more information about how to use this argument with git hook.
+
+### Allow Abort
+
+```bash
+cz check --message MESSAGE --allow-abort
+```
+
+Empty commit messages typically instruct Git to abort a commit, so you can pass `--allow-abort` to
+permit them. Since `git commit` accepts an `--allow-empty-message` flag (primarily for wrapper scripts), you may wish to disallow such commits in CI. `--allow-abort` may be used in conjunction with any of the other options.
diff --git a/docs/config.md b/docs/config.md
index 119db6f7..6a7838e2 100644
--- a/docs/config.md
+++ b/docs/config.md
@@ -131,6 +131,7 @@ commitizen:
 | `update_changelog_on_bump` | `bool` | `false`                     | Create changelog when running `cz bump`                                                                                                                                                         |
 | `annotated_tag`            | `bool` | `false`                     | Use annotated tags instead of lightweight tags. [See difference][annotated-tags-vs-lightweight]                                                                                                 |
 | `bump_message`             | `str`  | `None`                      | Create custom commit message, useful to skip ci. [See more][bump_message]                                                                                                                       |
+| `allow_abort`              | `bool` | `false`                     | Disallow empty commit messages, useful in ci. [See more][allow_abort]                                                                                                                           |
 | `changelog_file`           | `str`  | `CHANGELOG.md`              | filename of exported changelog                                                                                                                                                                  |
 | `changelog_incremental`    | `bool` | `false`                     | Update changelog with the missing versions. This is good if you don't want to replace previous versions in the file. Note: when doing `cz bump --changelog` this is automatically set to `true` |
 | `changelog_start_rev`      | `str`  | `None`                      | Start from a given git rev to generate the changelog                                                                                                                                            |
@@ -141,6 +142,7 @@ commitizen:
 [version_files]: bump.md#version_files
 [tag_format]: bump.md#tag_format
 [bump_message]: bump.md#bump_message
+[allow_abort]: check.md#allow-abort
 [additional-features]: https://github.com/tmbo/questionary#additional-features
 [customization]: customization.md
 [shortcuts]: customization.md#shortcut-keys


Test patch:
diff --git a/tests/commands/test_check_command.py b/tests/commands/test_check_command.py
index 26db697b..e5f62d0a 100644
--- a/tests/commands/test_check_command.py
+++ b/tests/commands/test_check_command.py
@@ -200,14 +200,15 @@ def test_check_a_range_of_git_commits_and_failed(config, mocker):
         error_mock.assert_called_once()
 
 
-def test_check_command_with_invalid_argment(config):
+def test_check_command_with_invalid_argument(config):
     with pytest.raises(InvalidCommandArgumentError) as excinfo:
         commands.Check(
             config=config,
             arguments={"commit_msg_file": "some_file", "rev_range": "HEAD~10..master"},
         )
-    assert "One and only one argument is required for check command!" in str(
-        excinfo.value
+    assert (
+        "Only one of --rev-range, --message, and --commit-msg-file is permitted by check command!"
+        in str(excinfo.value)
     )
 
 
@@ -257,6 +258,46 @@ def test_check_command_with_invalid_message(config, mocker):
         error_mock.assert_called_once()
 
 
+def test_check_command_with_empty_message(config, mocker):
+    error_mock = mocker.patch("commitizen.out.error")
+    check_cmd = commands.Check(config=config, arguments={"message": ""})
+
+    with pytest.raises(InvalidCommitMessageError):
+        check_cmd()
+        error_mock.assert_called_once()
+
+
+def test_check_command_with_allow_abort_arg(config, mocker):
+    success_mock = mocker.patch("commitizen.out.success")
+    check_cmd = commands.Check(
+        config=config, arguments={"message": "", "allow_abort": True}
+    )
+
+    check_cmd()
+    success_mock.assert_called_once()
+
+
+def test_check_command_with_allow_abort_config(config, mocker):
+    success_mock = mocker.patch("commitizen.out.success")
+    config.settings["allow_abort"] = True
+    check_cmd = commands.Check(config=config, arguments={"message": ""})
+
+    check_cmd()
+    success_mock.assert_called_once()
+
+
+def test_check_command_override_allow_abort_config(config, mocker):
+    error_mock = mocker.patch("commitizen.out.error")
+    config.settings["allow_abort"] = True
+    check_cmd = commands.Check(
+        config=config, arguments={"message": "", "allow_abort": False}
+    )
+
+    with pytest.raises(InvalidCommitMessageError):
+        check_cmd()
+        error_mock.assert_called_once()
+
+
 def test_check_command_with_pipe_message(mocker, capsys):
     testargs = ["cz", "check"]
     mocker.patch.object(sys, "argv", testargs)
diff --git a/tests/test_conf.py b/tests/test_conf.py
index 786af257..f051a1f5 100644
--- a/tests/test_conf.py
+++ b/tests/test_conf.py
@@ -40,6 +40,7 @@ _settings = {
     "version": "1.0.0",
     "tag_format": None,
     "bump_message": None,
+    "allow_abort": False,
     "version_files": ["commitizen/__version__.py", "pyproject.toml"],
     "style": [["pointer", "reverse"], ["question", "underline"]],
     "changelog_file": "CHANGELOG.md",
@@ -54,6 +55,7 @@ _new_settings = {
     "version": "2.0.0",
     "tag_format": None,
     "bump_message": None,
+    "allow_abort": False,
     "version_files": ["commitizen/__version__.py", "pyproject.toml"],
     "style": [["pointer", "reverse"], ["question", "underline"]],
     "changelog_file": "CHANGELOG.md",