Merge pull request #643 from hackedd/meta-hooks

Implement support for meta hooks

.pre-commit-config.yaml

@@ -25,3 +25,7 @@ repos:
     sha: v0.6.4
     hooks:
     -   id: add-trailing-comma
+-   repo: meta
+    hooks:
+    -   id: check-useless-excludes
+    -   id: check-files-matches-any

pre_commit/clientlib.py

@@ -98,6 +98,8 @@ def validate_manifest_main(argv=None):
 
 
 _LOCAL_SENTINEL = 'local'
+_META_SENTINEL = 'meta'
+
 CONFIG_HOOK_DICT = schema.Map(
     'Hook', 'id',
 
@@ -121,7 +123,8 @@ CONFIG_REPO_DICT = schema.Map(
 
     schema.Conditional(
         'sha', schema.check_string,
-        condition_key='repo', condition_value=schema.Not(_LOCAL_SENTINEL),
+        condition_key='repo',
+        condition_value=schema.NotIn(_LOCAL_SENTINEL, _META_SENTINEL),
         ensure_absent=True,
     ),
 )
@@ -138,6 +141,10 @@ def is_local_repo(repo_entry):
     return repo_entry['repo'] == _LOCAL_SENTINEL
 
 
+def is_meta_repo(repo_entry):
+    return repo_entry['repo'] == _META_SENTINEL
+
+
 class InvalidConfigError(FatalError):
     pass
 

pre_commit/commands/autoupdate.py

@@ -11,6 +11,7 @@ import pre_commit.constants as C
 from pre_commit import output
 from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import is_local_repo
+from pre_commit.clientlib import is_meta_repo
 from pre_commit.clientlib import load_config
 from pre_commit.commands.migrate_config import migrate_config
 from pre_commit.repository import Repository
@@ -115,7 +116,7 @@ def autoupdate(runner, tags_only):
     input_config = load_config(runner.config_file_path)
 
     for repo_config in input_config['repos']:
-        if is_local_repo(repo_config):
+        if is_local_repo(repo_config) or is_meta_repo(repo_config):
             output_repos.append(repo_config)
             continue
         output.write('Updating {}...'.format(repo_config['repo']))

pre_commit/meta_hooks/check_files_matches_any.py

@@ -0,0 +1,40 @@
+import argparse
+
+import pre_commit.constants as C
+from pre_commit import git
+from pre_commit.commands.run import _filter_by_include_exclude
+from pre_commit.commands.run import _filter_by_types
+from pre_commit.runner import Runner
+
+
+def check_all_hooks_match_files(config_file):
+    runner = Runner.create(config_file)
+    files = git.get_all_files()
+    retv = 0
+
+    for repo in runner.repositories:
+        for hook_id, hook in repo.hooks:
+            include, exclude = hook['files'], hook['exclude']
+            filtered = _filter_by_include_exclude(files, include, exclude)
+            types, exclude_types = hook['types'], hook['exclude_types']
+            filtered = _filter_by_types(filtered, types, exclude_types)
+            if not filtered:
+                print('{} does not apply to this repository'.format(hook_id))
+                retv = 1
+
+    return retv
+
+
+def main(argv=None):
+    parser = argparse.ArgumentParser()
+    parser.add_argument('filenames', nargs='*', default=[C.CONFIG_FILE])
+    args = parser.parse_args(argv)
+
+    retv = 0
+    for filename in args.filenames:
+        retv |= check_all_hooks_match_files(filename)
+    return retv
+
+
+if __name__ == '__main__':
+    exit(main())

pre_commit/meta_hooks/check_useless_excludes.py

@@ -0,0 +1,64 @@
+from __future__ import print_function
+
+import argparse
+import re
+
+import pre_commit.constants as C
+from pre_commit import git
+from pre_commit.clientlib import load_config
+from pre_commit.clientlib import MANIFEST_HOOK_DICT
+from pre_commit.schema import apply_defaults
+
+
+def exclude_matches_any(filenames, include, exclude):
+    if exclude == '^$':
+        return True
+    include_re, exclude_re = re.compile(include), re.compile(exclude)
+    for filename in filenames:
+        if include_re.search(filename) and exclude_re.search(filename):
+            return True
+    return False
+
+
+def check_useless_excludes(config_file):
+    config = load_config(config_file)
+    files = git.get_all_files()
+    retv = 0
+
+    exclude = config['exclude']
+    if not exclude_matches_any(files, '', exclude):
+        print(
+            'The global exclude pattern {!r} does not match any files'
+            .format(exclude),
+        )
+        retv = 1
+
+    for repo in config['repos']:
+        for hook in repo['hooks']:
+            # Not actually a manifest dict, but this more accurately reflects
+            # the defaults applied during runtime
+            hook = apply_defaults(hook, MANIFEST_HOOK_DICT)
+            include, exclude = hook['files'], hook['exclude']
+            if not exclude_matches_any(files, include, exclude):
+                print(
+                    'The exclude pattern {!r} for {} does not match any files'
+                    .format(exclude, hook['id']),
+                )
+                retv = 1
+
+    return retv
+
+
+def main(argv=None):
+    parser = argparse.ArgumentParser()
+    parser.add_argument('filenames', nargs='*', default=[C.CONFIG_FILE])
+    args = parser.parse_args(argv)
+
+    retv = 0
+    for filename in args.filenames:
+        retv |= check_useless_excludes(filename)
+    return retv
+
+
+if __name__ == '__main__':
+    exit(main())

pre_commit/repository.py

@@ -4,7 +4,9 @@ import io
 import json
 import logging
 import os
+import pipes
 import shutil
+import sys
 from collections import defaultdict
 
 import pkg_resources
@@ -14,6 +16,7 @@ import pre_commit.constants as C
 from pre_commit import five
 from pre_commit import git
 from pre_commit.clientlib import is_local_repo
+from pre_commit.clientlib import is_meta_repo
 from pre_commit.clientlib import load_manifest
 from pre_commit.clientlib import MANIFEST_HOOK_DICT
 from pre_commit.languages.all import languages
@@ -125,6 +128,12 @@ def _hook(*hook_dicts):
     return ret
 
 
+def _hook_from_manifest_dct(dct):
+    dct = validate(apply_defaults(dct, MANIFEST_HOOK_DICT), MANIFEST_HOOK_DICT)
+    dct = _hook(dct)
+    return dct
+
+
 class Repository(object):
     def __init__(self, repo_config, store):
         self.repo_config = repo_config
@@ -135,6 +144,8 @@ class Repository(object):
     def create(cls, config, store):
         if is_local_repo(config):
             return LocalRepository(config, store)
+        elif is_meta_repo(config):
+            return MetaRepository(config, store)
         else:
             return cls(config, store)
 
@@ -221,14 +232,8 @@ class LocalRepository(Repository):
 
     @cached_property
     def hooks(self):
-        def _from_manifest_dct(dct):
-            dct = validate(dct, MANIFEST_HOOK_DICT)
-            dct = apply_defaults(dct, MANIFEST_HOOK_DICT)
-            dct = _hook(dct)
-            return dct
-
         return tuple(
-            (hook['id'], _from_manifest_dct(hook))
+            (hook['id'], _hook_from_manifest_dct(hook))
             for hook in self.repo_config['hooks']
         )
 
@@ -246,6 +251,60 @@ class LocalRepository(Repository):
         return tuple(ret)
 
 
+class MetaRepository(LocalRepository):
+    @cached_property
+    def manifest_hooks(self):
+        # The hooks are imported here to prevent circular imports.
+        from pre_commit.meta_hooks import check_files_matches_any
+        from pre_commit.meta_hooks import check_useless_excludes
+
+        def _make_entry(mod):
+            """the hook `entry` is passed through `shlex.split()` by the
+            command runner, so to prevent issues with spaces and backslashes
+            (on Windows) it must be quoted here.
+            """
+            return '{} -m {}'.format(pipes.quote(sys.executable), mod.__name__)
+
+        meta_hooks = [
+            {
+                'id': 'check-files-matches-any',
+                'name': 'Check hooks match any files',
+                'files': '.pre-commit-config.yaml',
+                'language': 'system',
+                'entry': _make_entry(check_files_matches_any),
+            },
+            {
+                'id': 'check-useless-excludes',
+                'name': 'Check for useless excludes',
+                'files': '.pre-commit-config.yaml',
+                'language': 'system',
+                'entry': _make_entry(check_useless_excludes),
+            },
+        ]
+
+        return {
+            hook['id']: _hook_from_manifest_dct(hook)
+            for hook in meta_hooks
+        }
+
+    @cached_property
+    def hooks(self):
+        for hook in self.repo_config['hooks']:
+            if hook['id'] not in self.manifest_hooks:
+                logger.error(
+                    '`{}` is not a valid meta hook.  '
+                    'Typo? Perhaps it is introduced in a newer version?  '
+                    'Often `pip install --upgrade pre-commit` fixes this.'
+                    .format(hook['id']),
+                )
+                exit(1)
+
+        return tuple(
+            (hook['id'], _hook(self.manifest_hooks[hook['id']], hook))
+            for hook in self.repo_config['hooks']
+        )
+
+
 class _UniqueList(list):
     def __init__(self):
         self._set = set()

pre_commit/schema.py

@@ -101,6 +101,9 @@ def _check_conditional(self, dct):
         if isinstance(self.condition_value, Not):
             op = 'is'
             cond_val = self.condition_value.val
+        elif isinstance(self.condition_value, NotIn):
+            op = 'is any of'
+            cond_val = self.condition_value.values
         else:
             op = 'is not'
             cond_val = self.condition_value
@@ -198,14 +201,19 @@ class Array(collections.namedtuple('Array', ('of',))):
         return [remove_defaults(val, self.of) for val in v]
 
 
-class Not(object):
-    def __init__(self, val):
-        self.val = val
-
+class Not(collections.namedtuple('Not', ('val',))):
     def __eq__(self, other):
         return other is not MISSING and other != self.val
 
 
+class NotIn(collections.namedtuple('NotIn', ('values',))):
+    def __new__(cls, *values):
+        return super(NotIn, cls).__new__(cls, values=values)
+
+    def __eq__(self, other):
+        return other is not MISSING and other not in self.values
+
+
 def check_any(_):
     pass
 

tests/commands/autoupdate_test.py

@@ -295,6 +295,24 @@ def test_autoupdate_local_hooks_with_out_of_date_repo(
     assert new_config_writen['repos'][0] == local_config
 
 
+def test_autoupdate_meta_hooks(tmpdir, capsys):
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(
+        'repos:\n'
+        '-   repo: meta\n'
+        '    hooks:\n'
+        '    -   id: check-useless-excludes\n',
+    )
+    ret = autoupdate(Runner(tmpdir.strpath, C.CONFIG_FILE), tags_only=True)
+    assert ret == 0
+    assert cfg.read() == (
+        'repos:\n'
+        '-   repo: meta\n'
+        '    hooks:\n'
+        '    -   id: check-useless-excludes\n'
+    )
+
+
 def test_updates_old_format_to_new_format(tmpdir, capsys):
     cfg = tmpdir.join(C.CONFIG_FILE)
     cfg.write(

tests/commands/run_test.py

@@ -645,6 +645,31 @@ def test_local_hook_fails(
     )
 
 
+def test_meta_hook_passes(
+        cap_out, repo_with_passing_hook, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                )),
+            ),
+        ),
+    ))
+    add_config_to_repo(repo_with_passing_hook, config)
+
+    _test_run(
+        cap_out,
+        repo_with_passing_hook,
+        opts={},
+        expected_outputs=[b'Check for useless excludes'],
+        expected_ret=0,
+        stage=False,
+    )
+
+
 @pytest.yield_fixture
 def modified_config_repo(repo_with_passing_hook):
     with modify_config(repo_with_passing_hook, commit=False) as config:

tests/meta_hooks/hook_matches_any_test.py

@@ -0,0 +1,130 @@
+from collections import OrderedDict
+
+from pre_commit.meta_hooks import check_files_matches_any
+from pre_commit.util import cwd
+from testing.fixtures import add_config_to_repo
+from testing.fixtures import git_dir
+
+
+def test_hook_excludes_everything(
+        capsys, tempdir_factory, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('exclude', '.pre-commit-config.yaml'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_files_matches_any.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    assert 'check-useless-excludes does not apply to this repository' in out
+
+
+def test_hook_includes_nothing(
+        capsys, tempdir_factory, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('files', 'foo'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_files_matches_any.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    assert 'check-useless-excludes does not apply to this repository' in out
+
+
+def test_hook_types_not_matched(
+        capsys, tempdir_factory, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('types', ['python']),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_files_matches_any.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    assert 'check-useless-excludes does not apply to this repository' in out
+
+
+def test_hook_types_excludes_everything(
+        capsys, tempdir_factory, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('exclude_types', ['yaml']),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_files_matches_any.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    assert 'check-useless-excludes does not apply to this repository' in out
+
+
+def test_valid_includes(
+        capsys, tempdir_factory, mock_out_store_directory,
+):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_files_matches_any.main(()) == 0
+
+    out, _ = capsys.readouterr()
+    assert out == ''

tests/meta_hooks/useless_excludes_test.py

@@ -0,0 +1,107 @@
+from collections import OrderedDict
+
+from pre_commit.meta_hooks import check_useless_excludes
+from pre_commit.util import cwd
+from testing.fixtures import add_config_to_repo
+from testing.fixtures import git_dir
+
+
+def test_useless_exclude_global(capsys, tempdir_factory):
+    config = OrderedDict((
+        ('exclude', 'foo'),
+        (
+            'repos', [
+                OrderedDict((
+                    ('repo', 'meta'),
+                    (
+                        'hooks', (
+                            OrderedDict((
+                                ('id', 'check-useless-excludes'),
+                            )),
+                        ),
+                    ),
+                )),
+            ],
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_useless_excludes.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    assert "The global exclude pattern 'foo' does not match any files" in out
+
+
+def test_useless_exclude_for_hook(capsys, tempdir_factory):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('exclude', 'foo'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_useless_excludes.main(()) == 1
+
+    out, _ = capsys.readouterr()
+    expected = (
+        "The exclude pattern 'foo' for check-useless-excludes "
+        "does not match any files"
+    )
+    assert expected in out
+
+
+def test_no_excludes(capsys, tempdir_factory):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_useless_excludes.main(()) == 0
+
+    out, _ = capsys.readouterr()
+    assert out == ''
+
+
+def test_valid_exclude(capsys, tempdir_factory):
+    config = OrderedDict((
+        ('repo', 'meta'),
+        (
+            'hooks', (
+                OrderedDict((
+                    ('id', 'check-useless-excludes'),
+                    ('exclude', '.pre-commit-config.yaml'),
+                )),
+            ),
+        ),
+    ))
+
+    repo = git_dir(tempdir_factory)
+    add_config_to_repo(repo, config)
+
+    with cwd(repo):
+        assert check_useless_excludes.main(()) == 0
+
+    out, _ = capsys.readouterr()
+    assert out == ''

tests/repository_test.py

@@ -721,6 +721,18 @@ def test_hook_id_not_present(tempdir_factory, store, fake_log_handler):
     )
 
 
+def test_meta_hook_not_present(store, fake_log_handler):
+    config = {'repo': 'meta', 'hooks': [{'id': 'i-dont-exist'}]}
+    repo = Repository.create(config, store)
+    with pytest.raises(SystemExit):
+        repo.require_installed()
+    assert fake_log_handler.handle.call_args[0][0].msg == (
+        '`i-dont-exist` is not a valid meta hook.  '
+        'Typo? Perhaps it is introduced in a newer version?  '
+        'Often `pip install --upgrade pre-commit` fixes this.'
+    )
+
+
 def test_too_new_version(tempdir_factory, store, fake_log_handler):
     path = make_repo(tempdir_factory, 'script_hooks_repo')
     with modify_manifest(path) as manifest:

tests/schema_test.py

@@ -19,6 +19,7 @@ from pre_commit.schema import load_from_filename
 from pre_commit.schema import Map
 from pre_commit.schema import MISSING
 from pre_commit.schema import Not
+from pre_commit.schema import NotIn
 from pre_commit.schema import Optional
 from pre_commit.schema import OptionalNoDefault
 from pre_commit.schema import remove_defaults
@@ -107,6 +108,16 @@ def test_not(val, expected):
     assert (compared == val) is expected
 
 
+@pytest.mark.parametrize(
+    ('values', 'expected'),
+    (('bar', True), ('foo', False), (MISSING, False)),
+)
+def test_not_in(values, expected):
+    compared = NotIn('baz', 'foo')
+    assert (values == compared) is expected
+    assert (compared == values) is expected
+
+
 trivial_array_schema = Array(Map('foo', 'id'))
 
 
@@ -196,6 +207,13 @@ map_conditional_absent_not = Map(
         condition_key='key', condition_value=Not(True), ensure_absent=True,
     ),
 )
+map_conditional_absent_not_in = Map(
+    'foo', 'key',
+    Conditional(
+        'key2', check_bool,
+        condition_key='key', condition_value=NotIn(1, 2), ensure_absent=True,
+    ),
+)
 
 
 @pytest.mark.parametrize('schema', (map_conditional, map_conditional_not))
@@ -248,6 +266,19 @@ def test_ensure_absent_conditional_not():
     )
 
 
+def test_ensure_absent_conditional_not_in():
+    with pytest.raises(ValidationError) as excinfo:
+        validate({'key': 1, 'key2': True}, map_conditional_absent_not_in)
+    _assert_exception_trace(
+        excinfo.value,
+        (
+            'At foo(key=1)',
+            'Expected key2 to be absent when key is any of (1, 2), '
+            'found key2: True',
+        ),
+    )
+
+
 def test_no_error_conditional_absent():
     validate({}, map_conditional_absent)
     validate({}, map_conditional_absent_not)