Repository: pre-commit/pre-commit
Pull request #3093 (2023-12-09 21:04:48)

python 3.9+: use removeprefix


Patch:
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
index a49465e..9f41bf4 100644
--- a/pre_commit/clientlib.py
+++ b/pre_commit/clientlib.py
@@ -102,13 +102,6 @@ class StagesMigration(StagesMigrationNoDefault):
 MANIFEST_HOOK_DICT = cfgv.Map(
     'Hook', 'id',
 
-    # check first in case it uses some newer, incompatible feature
-    cfgv.Optional(
-        'minimum_pre_commit_version',
-        cfgv.check_and(cfgv.check_string, check_min_version),
-        '0',
-    ),
-
     cfgv.Required('id', cfgv.check_string),
     cfgv.Required('name', cfgv.check_string),
     cfgv.Required('entry', cfgv.check_string),
@@ -131,6 +124,7 @@ MANIFEST_HOOK_DICT = cfgv.Map(
     cfgv.Optional('description', cfgv.check_string, ''),
     cfgv.Optional('language_version', cfgv.check_string, C.DEFAULT),
     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),
     StagesMigration('stages', []),
     cfgv.Optional('verbose', cfgv.check_bool, False),
@@ -351,13 +345,6 @@ DEFAULT_LANGUAGE_VERSION = cfgv.Map(
 CONFIG_SCHEMA = cfgv.Map(
     'Config', None,
 
-    # check first in case it uses some newer, incompatible feature
-    cfgv.Optional(
-        'minimum_pre_commit_version',
-        cfgv.check_and(cfgv.check_string, check_min_version),
-        '0',
-    ),
-
     cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)),
     cfgv.Optional(
         'default_install_hook_types',
@@ -371,6 +358,11 @@ CONFIG_SCHEMA = cfgv.Map(
     cfgv.Optional('files', check_string_regex, ''),
     cfgv.Optional('exclude', check_string_regex, '^$'),
     cfgv.Optional('fail_fast', cfgv.check_bool, False),
+    cfgv.Optional(
+        'minimum_pre_commit_version',
+        cfgv.check_and(cfgv.check_string, check_min_version),
+        '0',
+    ),
     cfgv.WarnAdditionalKeys(
         (
             'repos',
diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py
index e5bac9f..9f4bf69 100644
--- a/pre_commit/languages/python.py
+++ b/pre_commit/languages/python.py
@@ -65,7 +65,7 @@ def _find_by_py_launcher(
         version: str,
 ) -> str | None:  # pragma: no cover (windows only)
     if version.startswith('python'):
-        num = version[len('python'):]
+        num = version.removeprefix('python')
         cmd = ('py', f'-{num}', '-c', 'import sys; print(sys.executable)')
         env = dict(os.environ, PYTHONIOENCODING='UTF-8')
         try:
@@ -124,7 +124,7 @@ def _sys_executable_matches(version: str) -> bool:
         return False
 
     try:
-        info = tuple(int(p) for p in version[len('python'):].split('.'))
+        info = tuple(int(p) for p in version.removeprefix('python').split('.'))
     except ValueError:
         return False
 
diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py
index 241146c..7b04d6c 100644
--- a/pre_commit/languages/rust.py
+++ b/pre_commit/languages/rust.py
@@ -134,7 +134,7 @@ def install_environment(
 
     packages_to_install: set[tuple[str, ...]] = {('--path', '.')}
     for cli_dep in cli_deps:
-        cli_dep = cli_dep[len('cli:'):]
+        cli_dep = cli_dep.removeprefix('cli:')
         package, _, crate_version = cli_dep.partition(':')
         if crate_version != '':
             packages_to_install.add((package, '--version', crate_version))
diff --git a/pre_commit/repository.py b/pre_commit/repository.py
index aa84185..439a09b 100644
--- a/pre_commit/repository.py
+++ b/pre_commit/repository.py
@@ -12,6 +12,7 @@ from pre_commit.all_languages import languages
 from pre_commit.clientlib import load_manifest
 from pre_commit.clientlib import LOCAL
 from pre_commit.clientlib import META
+from pre_commit.clientlib import parse_version
 from pre_commit.hook import Hook
 from pre_commit.lang_base import environment_dir
 from pre_commit.prefix import Prefix
@@ -123,6 +124,15 @@ def _hook(
     for dct in rest:
         ret.update(dct)
 
+    version = ret['minimum_pre_commit_version']
+    if parse_version(version) > parse_version(C.VERSION):
+        logger.error(
+            f'The hook `{ret["id"]}` requires pre-commit version {version} '
+            f'but version {C.VERSION} is installed.  '
+            f'Perhaps run `pip install --upgrade pre-commit`.',
+        )
+        exit(1)
+
     lang = ret['language']
     if ret['language_version'] == C.DEFAULT:
         ret['language_version'] = root_config['default_language_version'][lang]


Test patch:
diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py
index eaa8a04..568b2e9 100644
--- a/tests/clientlib_test.py
+++ b/tests/clientlib_test.py
@@ -40,51 +40,56 @@ def test_check_type_tag_success():
 
 
 @pytest.mark.parametrize(
-    'cfg',
-    (
-        {
-            'repos': [{
-                'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-                'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
-                'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
-            }],
-        },
-        {
-            'repos': [{
-                'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-                'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
-                'hooks': [
-                    {
-                        'id': 'pyflakes',
-                        'files': '\\.py$',
-                        'args': ['foo', 'bar', 'baz'],
-                    },
-                ],
-            }],
-        },
+    ('config_obj', 'expected'), (
+        (
+            {
+                'repos': [{
+                    'repo': 'git@github.com:pre-commit/pre-commit-hooks',
+                    'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                    'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
+                }],
+            },
+            True,
+        ),
+        (
+            {
+                'repos': [{
+                    'repo': 'git@github.com:pre-commit/pre-commit-hooks',
+                    'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                    'hooks': [
+                        {
+                            'id': 'pyflakes',
+                            'files': '\\.py$',
+                            'args': ['foo', 'bar', 'baz'],
+                        },
+                    ],
+                }],
+            },
+            True,
+        ),
+        (
+            {
+                'repos': [{
+                    'repo': 'git@github.com:pre-commit/pre-commit-hooks',
+                    'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                    'hooks': [
+                        {
+                            'id': 'pyflakes',
+                            'files': '\\.py$',
+                            # Exclude pattern must be a string
+                            'exclude': 0,
+                            'args': ['foo', 'bar', 'baz'],
+                        },
+                    ],
+                }],
+            },
+            False,
+        ),
     ),
 )
-def test_config_valid(cfg):
-    assert is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
-
-
-def test_invalid_config_wrong_type():
-    cfg = {
-        'repos': [{
-            'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-            'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
-            'hooks': [
-                {
-                    'id': 'pyflakes',
-                    'files': '\\.py$',
-                    # Exclude pattern must be a string
-                    'exclude': 0,
-                    'args': ['foo', 'bar', 'baz'],
-                },
-            ],
-        }],
-    }
-    assert not is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
+def test_config_valid(config_obj, expected):
+    ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA)
+    assert ret is expected
 
 
 def test_local_hooks_with_rev_fails():
@@ -193,13 +198,14 @@ def test_warn_mutable_rev_conditional():
     ),
 )
 def test_sensible_regex_validators_dont_pass_none(validator_cls):
-    validator = validator_cls('files', cfgv.check_string)
+    key = 'files'
     with pytest.raises(cfgv.ValidationError) as excinfo:
-        validator.check({'files': None})
+        validator = validator_cls(key, cfgv.check_string)
+        validator.check({key: None})
 
     assert str(excinfo.value) == (
         '\n'
-        '==> At key: files'
+        f'==> At key: {key}'
         '\n'
         '=====> Expected string got NoneType'
     )
@@ -292,36 +298,46 @@ def test_validate_optional_sensible_regex_at_top_level(caplog, regex, warning):
 
 
 @pytest.mark.parametrize(
-    'manifest_obj',
+    ('manifest_obj', 'expected'),
     (
-        [{
-            'id': 'a',
-            'name': 'b',
-            'entry': 'c',
-            'language': 'python',
-            'files': r'\.py$',
-        }],
-        [{
-            'id': 'a',
-            'name': 'b',
-            'entry': 'c',
-            'language': 'python',
-            'language_version': 'python3.4',
-            'files': r'\.py$',
-        }],
-        # A regression in 0.13.5: always_run and files are permissible
-        [{
-            'id': 'a',
-            'name': 'b',
-            'entry': 'c',
-            'language': 'python',
-            'files': '',
-            'always_run': True,
-        }],
+        (
+            [{
+                'id': 'a',
+                'name': 'b',
+                'entry': 'c',
+                'language': 'python',
+                'files': r'\.py$',
+            }],
+            True,
+        ),
+        (
+            [{
+                'id': 'a',
+                'name': 'b',
+                'entry': 'c',
+                'language': 'python',
+                'language_version': 'python3.4',
+                'files': r'\.py$',
+            }],
+            True,
+        ),
+        (
+            # A regression in 0.13.5: always_run and files are permissible
+            [{
+                'id': 'a',
+                'name': 'b',
+                'entry': 'c',
+                'language': 'python',
+                'files': '',
+                'always_run': True,
+            }],
+            True,
+        ),
     ),
 )
-def test_valid_manifests(manifest_obj):
-    assert is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
+def test_valid_manifests(manifest_obj, expected):
+    ret = is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
+    assert ret is expected
 
 
 @pytest.mark.parametrize(
@@ -377,39 +393,8 @@ def test_parse_version():
 
 
 def test_minimum_pre_commit_version_failing():
-    cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
-    with pytest.raises(cfgv.ValidationError) as excinfo:
-        cfgv.validate(cfg, CONFIG_SCHEMA)
-    assert str(excinfo.value) == (
-        f'\n'
-        f'==> At Config()\n'
-        f'==> At key: minimum_pre_commit_version\n'
-        f'=====> pre-commit version 999 is required but version {C.VERSION} '
-        f'is installed.  Perhaps run `pip install --upgrade pre-commit`.'
-    )
-
-
-def test_minimum_pre_commit_version_failing_in_config():
-    cfg = {'repos': [sample_local_config()]}
-    cfg['repos'][0]['hooks'][0]['minimum_pre_commit_version'] = '999'
-    with pytest.raises(cfgv.ValidationError) as excinfo:
-        cfgv.validate(cfg, CONFIG_SCHEMA)
-    assert str(excinfo.value) == (
-        f'\n'
-        f'==> At Config()\n'
-        f'==> At key: repos\n'
-        f"==> At Repository(repo='local')\n"
-        f'==> At key: hooks\n'
-        f"==> At Hook(id='do_not_commit')\n"
-        f'==> At key: minimum_pre_commit_version\n'
-        f'=====> pre-commit version 999 is required but version {C.VERSION} '
-        f'is installed.  Perhaps run `pip install --upgrade pre-commit`.'
-    )
-
-
-def test_minimum_pre_commit_version_failing_before_other_error():
-    cfg = {'repos': 5, 'minimum_pre_commit_version': '999'}
     with pytest.raises(cfgv.ValidationError) as excinfo:
+        cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
         cfgv.validate(cfg, CONFIG_SCHEMA)
     assert str(excinfo.value) == (
         f'\n'
diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py
index 8b0d3ec..9eb0e74 100644
--- a/tests/commands/install_uninstall_test.py
+++ b/tests/commands/install_uninstall_test.py
@@ -349,8 +349,9 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store):
         # We should run both the legacy and pre-commit hooks
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
-        assert output.startswith('legacy hook\n')
-        NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):])
+        legacy = 'legacy hook\n'
+        assert output.startswith(legacy)
+        NORMAL_PRE_COMMIT_RUN.assert_matches(output.removeprefix(legacy))
 
 
 def test_legacy_overwriting_legacy_hook(tempdir_factory, store):
@@ -375,8 +376,9 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store):
         # We should run both the legacy and pre-commit hooks
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
-        assert output.startswith('legacy hook\n')
-        NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):])
+        legacy = 'legacy hook\n'
+        assert output.startswith(legacy)
+        NORMAL_PRE_COMMIT_RUN.assert_matches(output.removeprefix(legacy))
 
 
 def test_install_with_existing_non_utf8_script(tmpdir, store):
diff --git a/tests/repository_test.py b/tests/repository_test.py
index ac065ec..b8dde99 100644
--- a/tests/repository_test.py
+++ b/tests/repository_test.py
@@ -9,6 +9,7 @@ from unittest import mock
 
 import cfgv
 import pytest
+import re_assert
 
 import pre_commit.constants as C
 from pre_commit import lang_base
@@ -26,6 +27,7 @@ from pre_commit.util import cmd_output
 from pre_commit.util import cmd_output_b
 from testing.fixtures import make_config_from_repo
 from testing.fixtures import make_repo
+from testing.fixtures import modify_manifest
 from testing.language_helpers import run_language
 from testing.util import cwd
 from testing.util import get_resource_path
@@ -431,6 +433,32 @@ def test_hook_id_not_present(tempdir_factory, store, caplog):
     )
 
 
+def test_too_new_version(tempdir_factory, store, caplog):
+    path = make_repo(tempdir_factory, 'script_hooks_repo')
+    with modify_manifest(path) as manifest:
+        manifest[0]['minimum_pre_commit_version'] = '999.0.0'
+    config = make_config_from_repo(path)
+    with pytest.raises(SystemExit):
+        _get_hook(config, store, 'bash_hook')
+    _, msg = caplog.messages
+    pattern = re_assert.Matches(
+        r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but '
+        r'version \d+\.\d+\.\d+ is installed.  '
+        r'Perhaps run `pip install --upgrade pre-commit`\.$',
+    )
+    pattern.assert_matches(msg)
+
+
+@pytest.mark.parametrize('version', ('0.1.0', C.VERSION))
+def test_versions_ok(tempdir_factory, store, version):
+    path = make_repo(tempdir_factory, 'script_hooks_repo')
+    with modify_manifest(path) as manifest:
+        manifest[0]['minimum_pre_commit_version'] = version
+    config = make_config_from_repo(path)
+    # Should succeed
+    _get_hook(config, store, 'bash_hook')
+
+
 def test_manifest_hooks(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'script_hooks_repo')
     config = make_config_from_repo(path)
diff --git a/tests/store_test.py b/tests/store_test.py
index eaab940..45ec732 100644
--- a/tests/store_test.py
+++ b/tests/store_test.py
@@ -185,7 +185,7 @@ def test_db_repo_name(store):
 
 def test_local_resources_reflects_reality():
     on_disk = {
-        res[len('empty_template_'):]
+        res.removeprefix('empty_template_')
         for res in os.listdir('pre_commit/resources')
         if res.startswith('empty_template_')
     }