make --hook-type and stages match

pre_commit/clientlib.py

@@ -6,6 +6,7 @@ import re
 import shlex
 import sys
 from typing import Any
+from typing import NamedTuple
 from typing import Sequence
 
 import cfgv
@@ -20,6 +21,20 @@ logger = logging.getLogger('pre_commit')
 
 check_string_regex = cfgv.check_and(cfgv.check_string, cfgv.check_regex)
 
+HOOK_TYPES = (
+    'commit-msg',
+    'post-checkout',
+    'post-commit',
+    'post-merge',
+    'post-rewrite',
+    'pre-commit',
+    'pre-merge-commit',
+    'pre-push',
+    'prepare-commit-msg',
+)
+# `manual` is not invoked by any installed git hook.  See #719
+STAGES = (*HOOK_TYPES, 'manual')
+
 
 def check_type_tag(tag: str) -> None:
     if tag not in ALL_TAGS:
@@ -43,6 +58,46 @@ def check_min_version(version: str) -> None:
         )
 
 
+_STAGES = {
+    'commit': 'pre-commit',
+    'merge-commit': 'pre-merge-commit',
+    'push': 'pre-push',
+}
+
+
+def transform_stage(stage: str) -> str:
+    return _STAGES.get(stage, stage)
+
+
+class StagesMigrationNoDefault(NamedTuple):
+    key: str
+    default: Sequence[str]
+
+    def check(self, dct: dict[str, Any]) -> None:
+        if self.key not in dct:
+            return
+
+        val = dct[self.key]
+        cfgv.check_array(cfgv.check_any)(val)
+
+        val = [transform_stage(v) for v in val]
+        cfgv.check_array(cfgv.check_one_of(STAGES))(val)
+
+    def apply_default(self, dct: dict[str, Any]) -> None:
+        if self.key not in dct:
+            return
+        dct[self.key] = [transform_stage(v) for v in dct[self.key]]
+
+    def remove_default(self, dct: dict[str, Any]) -> None:
+        raise NotImplementedError
+
+
+class StagesMigration(StagesMigrationNoDefault):
+    def apply_default(self, dct: dict[str, Any]) -> None:
+        dct.setdefault(self.key, self.default)
+        super().apply_default(dct)
+
+
 MANIFEST_HOOK_DICT = cfgv.Map(
     'Hook', 'id',
 
@@ -70,7 +125,7 @@ MANIFEST_HOOK_DICT = cfgv.Map(
     cfgv.Optional('log_file', cfgv.check_string, ''),
     cfgv.Optional('minimum_pre_commit_version', cfgv.check_string, '0'),
     cfgv.Optional('require_serial', cfgv.check_bool, False),
-    cfgv.Optional('stages', cfgv.check_array(cfgv.check_one_of(C.STAGES)), []),
+    StagesMigration('stages', []),
     cfgv.Optional('verbose', cfgv.check_bool, False),
 )
 MANIFEST_SCHEMA = cfgv.Array(MANIFEST_HOOK_DICT)
@@ -241,7 +296,9 @@ CONFIG_HOOK_DICT = cfgv.Map(
         cfgv.OptionalNoDefault(item.key, item.check_fn)
         for item in MANIFEST_HOOK_DICT.items
         if item.key != 'id'
+        if item.key != 'stages'
     ),
+    StagesMigrationNoDefault('stages', []),
     OptionalSensibleRegexAtHook('files', cfgv.check_string),
     OptionalSensibleRegexAtHook('exclude', cfgv.check_string),
 )
@@ -290,17 +347,13 @@ CONFIG_SCHEMA = cfgv.Map(
     cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)),
     cfgv.Optional(
         'default_install_hook_types',
-        cfgv.check_array(cfgv.check_one_of(C.HOOK_TYPES)),
+        cfgv.check_array(cfgv.check_one_of(HOOK_TYPES)),
         ['pre-commit'],
     ),
     cfgv.OptionalRecurse(
         'default_language_version', DEFAULT_LANGUAGE_VERSION, {},
     ),
-    cfgv.Optional(
-        'default_stages',
-        cfgv.check_array(cfgv.check_one_of(C.STAGES)),
-        C.STAGES,
-    ),
+    StagesMigration('default_stages', STAGES),
     cfgv.Optional('files', check_string_regex, ''),
     cfgv.Optional('exclude', check_string_regex, '^$'),
     cfgv.Optional('fail_fast', cfgv.check_bool, False),

pre_commit/commands/hook_impl.py

@@ -84,7 +84,7 @@ def _ns(
 ) -> argparse.Namespace:
     return argparse.Namespace(
         color=color,
-        hook_stage=hook_type.replace('pre-', ''),
+        hook_stage=hook_type,
         remote_branch=remote_branch,
         local_branch=local_branch,
         from_ref=from_ref,

pre_commit/constants.py

@@ -10,17 +10,4 @@ LOCAL_REPO_VERSION = '1'
 
 VERSION = importlib.metadata.version('pre_commit')
 
-# `manual` is not invoked by any installed git hook.  See #719
-STAGES = (
-    'commit', 'merge-commit', 'prepare-commit-msg', 'commit-msg',
-    'post-commit', 'manual', 'post-checkout', 'push', 'post-merge',
-    'post-rewrite',
-)
-
-HOOK_TYPES = (
-    'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg',
-    'commit-msg', 'post-commit', 'post-checkout', 'post-merge',
-    'post-rewrite',
-)
-
 DEFAULT = 'default'

pre_commit/main.py

@@ -7,6 +7,7 @@ import sys
 from typing import Sequence
 
 import pre_commit.constants as C
+from pre_commit import clientlib
 from pre_commit import git
 from pre_commit.color import add_color_option
 from pre_commit.commands.autoupdate import autoupdate
@@ -52,7 +53,7 @@ def _add_config_option(parser: argparse.ArgumentParser) -> None:
 def _add_hook_type_option(parser: argparse.ArgumentParser) -> None:
     parser.add_argument(
         '-t', '--hook-type',
-        choices=C.HOOK_TYPES, action='append', dest='hook_types',
+        choices=clientlib.HOOK_TYPES, action='append', dest='hook_types',
     )
 
 
@@ -73,7 +74,10 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None:
         help='When hooks fail, run `git diff` directly afterward.',
     )
     parser.add_argument(
-        '--hook-stage', choices=C.STAGES, default='commit',
+        '--hook-stage',
+        choices=clientlib.STAGES,
+        type=clientlib.transform_stage,
+        default='pre-commit',
         help='The stage during which the hook is fired.  One of %(choices)s',
     )
     parser.add_argument(

testing/util.py

@@ -46,7 +46,7 @@ def run_opts(
         to_ref='',
         remote_name='',
         remote_url='',
-        hook_stage='commit',
+        hook_stage='pre-commit',
         show_diff_on_failure=False,
         commit_msg_filename='',
         prepare_commit_message_source='',

tests/clientlib_test.py

@@ -12,6 +12,7 @@ from pre_commit.clientlib import CONFIG_HOOK_DICT
 from pre_commit.clientlib import CONFIG_REPO_DICT
 from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import DEFAULT_LANGUAGE_VERSION
+from pre_commit.clientlib import MANIFEST_HOOK_DICT
 from pre_commit.clientlib import MANIFEST_SCHEMA
 from pre_commit.clientlib import META_HOOK_DICT
 from pre_commit.clientlib import OptionalSensibleRegexAtHook
@@ -416,3 +417,50 @@ def test_warn_additional(schema):
         x for x in schema.items if isinstance(x, cfgv.WarnAdditionalKeys)
     )
     assert allowed_keys == set(warn_additional.keys)
+
+
+def test_stages_migration_for_default_stages():
+    cfg = {
+        'default_stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
+        'repos': [],
+    }
+    cfgv.validate(cfg, CONFIG_SCHEMA)
+    cfg = cfgv.apply_defaults(cfg, CONFIG_SCHEMA)
+    assert cfg['default_stages'] == [
+        'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit',
+    ]
+
+
+def test_manifest_stages_defaulting():
+    dct = {
+        'id': 'fake-hook',
+        'name': 'fake-hook',
+        'entry': 'fake-hook',
+        'language': 'system',
+        'stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
+    }
+    cfgv.validate(dct, MANIFEST_HOOK_DICT)
+    dct = cfgv.apply_defaults(dct, MANIFEST_HOOK_DICT)
+    assert dct['stages'] == [
+        'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit',
+    ]
+
+
+def test_config_hook_stages_defaulting_missing():
+    dct = {'id': 'fake-hook'}
+    cfgv.validate(dct, CONFIG_HOOK_DICT)
+    dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT)
+    assert dct == {'id': 'fake-hook'}
+
+
+def test_config_hook_stages_defaulting():
+    dct = {
+        'id': 'fake-hook',
+        'stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
+    }
+    cfgv.validate(dct, CONFIG_HOOK_DICT)
+    dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT)
+    assert dct == {
+        'id': 'fake-hook',
+        'stages': ['commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit'],
+    }

tests/commands/hook_impl_test.py

@@ -142,7 +142,7 @@ def test_check_args_length_prepare_commit_msg_error():
 def test_run_ns_pre_commit():
     ns = hook_impl._run_ns('pre-commit', True, (), b'')
     assert ns is not None
-    assert ns.hook_stage == 'commit'
+    assert ns.hook_stage == 'pre-commit'
     assert ns.color is True
 
 
@@ -245,7 +245,7 @@ def test_run_ns_pre_push_updating_branch(push_example):
         ns = hook_impl._run_ns('pre-push', False, args, stdin)
 
     assert ns is not None
-    assert ns.hook_stage == 'push'
+    assert ns.hook_stage == 'pre-push'
     assert ns.color is False
     assert ns.remote_name == 'origin'
     assert ns.remote_url == src

tests/commands/run_test.py

@@ -354,13 +354,13 @@ def test_show_diff_on_failure(
         ({'hook': 'bash_hook'}, (b'Bash hook', b'Passed'), 0, True),
         (
             {'hook': 'nope'},
-            (b'No hook with id `nope` in stage `commit`',),
+            (b'No hook with id `nope` in stage `pre-commit`',),
             1,
             True,
         ),
         (
-            {'hook': 'nope', 'hook_stage': 'push'},
-            (b'No hook with id `nope` in stage `push`',),
+            {'hook': 'nope', 'hook_stage': 'pre-push'},
+            (b'No hook with id `nope` in stage `pre-push`',),
             1,
             True,
         ),
@@ -818,7 +818,7 @@ def test_stages(cap_out, store, repo_with_passing_hook):
                 'language': 'pygrep',
                 'stages': [stage],
             }
-            for i, stage in enumerate(('commit', 'push', 'manual'), 1)
+            for i, stage in enumerate(('pre-commit', 'pre-push', 'manual'), 1)
         ],
     }
     add_config_to_repo(repo_with_passing_hook, config)
@@ -833,8 +833,8 @@ def test_stages(cap_out, store, repo_with_passing_hook):
         assert printed.count(b'hook ') == 1
         return printed
 
-    assert _run_for_stage('commit').startswith(b'hook 1...')
-    assert _run_for_stage('push').startswith(b'hook 2...')
+    assert _run_for_stage('pre-commit').startswith(b'hook 1...')
+    assert _run_for_stage('pre-push').startswith(b'hook 2...')
     assert _run_for_stage('manual').startswith(b'hook 3...')
 
 
@@ -1173,7 +1173,7 @@ def test_args_hook_only(cap_out, store, repo_with_passing_hook):
                 ),
                 'language': 'system',
                 'files': r'\.py$',
-                'stages': ['commit'],
+                'stages': ['pre-commit'],
             },
             {
                 'id': 'do_not_commit',

tests/main_test.py

@@ -216,3 +216,9 @@ def test_expected_fatal_error_no_git_repo(in_tmpdir, cap_out, mock_store_dir):
         'Is it installed, and are you in a Git repository directory?'
     )
     assert cap_out_lines[-1] == f'Check the log at {log_file}'
+
+
+def test_hook_stage_migration(mock_store_dir):
+    with mock.patch.object(main, 'run') as mck:
+        main.main(('run', '--hook-stage', 'commit'))
+    assert mck.call_args[0][2].hook_stage == 'pre-commit'

tests/repository_test.py

@@ -417,7 +417,7 @@ def test_local_python_repo(store, local_python_config):
 def test_default_language_version(store, local_python_config):
     config: dict[str, Any] = {
         'default_language_version': {'python': 'fake'},
-        'default_stages': ['commit'],
+        'default_stages': ['pre-commit'],
         'repos': [local_python_config],
     }
 
@@ -434,18 +434,18 @@ def test_default_language_version(store, local_python_config):
 def test_default_stages(store, local_python_config):
     config: dict[str, Any] = {
         'default_language_version': {'python': C.DEFAULT},
-        'default_stages': ['commit'],
+        'default_stages': ['pre-commit'],
         'repos': [local_python_config],
     }
 
     # `stages` was not set, should default
     hook, = all_hooks(config, store)
-    assert hook.stages == ['commit']
+    assert hook.stages == ['pre-commit']
 
     # `stages` is set, should not default
-    config['repos'][0]['hooks'][0]['stages'] = ['push']
+    config['repos'][0]['hooks'][0]['stages'] = ['pre-push']
     hook, = all_hooks(config, store)
-    assert hook.stages == ['push']
+    assert hook.stages == ['pre-push']
 
 
 def test_hook_id_not_present(tempdir_factory, store, caplog):
@@ -513,11 +513,18 @@ def test_manifest_hooks(tempdir_factory, store):
         name='Bash hook',
         pass_filenames=True,
         require_serial=False,
-        stages=(
-            'commit', 'merge-commit', 'prepare-commit-msg', 'commit-msg',
-            'post-commit', 'manual', 'post-checkout', 'push', 'post-merge',
+        stages=[
+            'commit-msg',
+            'post-checkout',
+            'post-commit',
+            'post-merge',
             'post-rewrite',
-        ),
+            'pre-commit',
+            'pre-merge-commit',
+            'pre-push',
+            'prepare-commit-msg',
+            'manual',
+        ],
         types=['file'],
         types_or=[],
         verbose=False,