diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 9ff38c6a..cb7778bb 100644 --- a/pre_commit/clientlib.py +++ b/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), diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index f5995e9a..25d99c29 100644 --- a/pre_commit/commands/hook_impl.py +++ b/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, diff --git a/pre_commit/constants.py b/pre_commit/constants.py index 3f03ceed..79a9bb69 100644 --- a/pre_commit/constants.py +++ b/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' diff --git a/pre_commit/main.py b/pre_commit/main.py index 3915993f..62d171e6 100644 --- a/pre_commit/main.py +++ b/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( diff --git a/testing/util.py b/testing/util.py index 0fee2826..8e3934cf 100644 --- a/testing/util.py +++ b/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='', diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index efb2aa84..568b2e97 100644 --- a/tests/clientlib_test.py +++ b/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'], + } diff --git a/tests/commands/hook_impl_test.py b/tests/commands/hook_impl_test.py index aa321dab..169e1414 100644 --- a/tests/commands/hook_impl_test.py +++ b/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 diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index f1085d9b..885b78d6 100644 --- a/tests/commands/run_test.py +++ b/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', diff --git a/tests/main_test.py b/tests/main_test.py index 51159262..945349fa 100644 --- a/tests/main_test.py +++ b/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' diff --git a/tests/repository_test.py b/tests/repository_test.py index a6c58bc7..903574ce 100644 --- a/tests/repository_test.py +++ b/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,