diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 36340642..9cd63760 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index a80c6b40..ca0ed5e2 100644 --- a/pre_commit/commands/autoupdate.py +++ b/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'])) diff --git a/pre_commit/meta_hooks/check_files_matches_any.py b/pre_commit/meta_hooks/check_files_matches_any.py index d253939e..8c9a92d8 100644 --- a/pre_commit/meta_hooks/check_files_matches_any.py +++ b/pre_commit/meta_hooks/check_files_matches_any.py @@ -1,16 +1,16 @@ 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.git import get_all_files from pre_commit.runner import Runner def check_all_hooks_match_files(config_file): runner = Runner.create(config_file) - files = get_all_files() - files_matched = True + files = git.get_all_files() + retv = 0 for repo in runner.repositories: for hook_id, hook in repo.hooks: @@ -20,9 +20,9 @@ def check_all_hooks_match_files(config_file): filtered = _filter_by_types(filtered, types, exclude_types) if not filtered: print('{} does not apply to this repository'.format(hook_id)) - files_matched = False + retv = 1 - return files_matched + return retv def main(argv=None): @@ -32,7 +32,7 @@ def main(argv=None): retv = 0 for filename in args.filenames: - retv |= not check_all_hooks_match_files(filename) + retv |= check_all_hooks_match_files(filename) return retv diff --git a/pre_commit/meta_hooks/check_useless_excludes.py b/pre_commit/meta_hooks/check_useless_excludes.py index 89448d78..189633a8 100644 --- a/pre_commit/meta_hooks/check_useless_excludes.py +++ b/pre_commit/meta_hooks/check_useless_excludes.py @@ -4,11 +4,15 @@ 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.git import get_all_files +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): @@ -18,28 +22,31 @@ def exclude_matches_any(filenames, include, exclude): def check_useless_excludes(config_file): config = load_config(config_file) - files = get_all_files() - useless_excludes = False + files = git.get_all_files() + retv = 0 - exclude = config.get('exclude') - if exclude != '^$' and not exclude_matches_any(files, '', exclude): + exclude = config['exclude'] + if not exclude_matches_any(files, '', exclude): print( 'The global exclude pattern {!r} does not match any files' .format(exclude), ) - useless_excludes = True + retv = 1 for repo in config['repos']: for hook in repo['hooks']: - include, exclude = hook.get('files', ''), hook.get('exclude') - if exclude and not exclude_matches_any(files, include, exclude): + # 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']), ) - useless_excludes = True + retv = 1 - return useless_excludes + return retv def main(argv=None): diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 8adcbec0..2eb62ecb 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -128,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 @@ -226,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'] ) @@ -258,33 +258,32 @@ class MetaRepository(LocalRepository): from pre_commit.meta_hooks import check_files_matches_any from pre_commit.meta_hooks import check_useless_excludes - # Note: 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. + 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-useless-excludes', - 'name': 'Check for useless excludes', - 'files': '.pre-commit-config.yaml', - 'language': 'system', - 'entry': pipes.quote(sys.executable), - 'args': ['-m', check_useless_excludes.__name__], - }, { 'id': 'check-files-matches-any', 'name': 'Check hooks match any files', 'files': '.pre-commit-config.yaml', 'language': 'system', - 'entry': pipes.quote(sys.executable), - 'args': ['-m', check_files_matches_any.__name__], + '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']: apply_defaults( - validate(hook, MANIFEST_HOOK_DICT), - MANIFEST_HOOK_DICT, - ) + hook['id']: _hook_from_manifest_dct(hook) for hook in meta_hooks } diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index 9ae70c64..7119c6be 100644 --- a/tests/commands/autoupdate_test.py +++ b/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( diff --git a/tests/meta_hooks/hook_matches_any_test.py b/tests/meta_hooks/hook_matches_any_test.py index 92c6fc45..005cdf68 100644 --- a/tests/meta_hooks/hook_matches_any_test.py +++ b/tests/meta_hooks/hook_matches_any_test.py @@ -25,7 +25,7 @@ def test_hook_excludes_everything( add_config_to_repo(repo, config) with cwd(repo): - assert check_files_matches_any.main(argv=[]) == 1 + assert check_files_matches_any.main(()) == 1 out, _ = capsys.readouterr() assert 'check-useless-excludes does not apply to this repository' in out @@ -50,7 +50,7 @@ def test_hook_includes_nothing( add_config_to_repo(repo, config) with cwd(repo): - assert check_files_matches_any.main(argv=[]) == 1 + assert check_files_matches_any.main(()) == 1 out, _ = capsys.readouterr() assert 'check-useless-excludes does not apply to this repository' in out @@ -75,7 +75,7 @@ def test_hook_types_not_matched( add_config_to_repo(repo, config) with cwd(repo): - assert check_files_matches_any.main(argv=[]) == 1 + assert check_files_matches_any.main(()) == 1 out, _ = capsys.readouterr() assert 'check-useless-excludes does not apply to this repository' in out @@ -100,7 +100,7 @@ def test_hook_types_excludes_everything( add_config_to_repo(repo, config) with cwd(repo): - assert check_files_matches_any.main(argv=[]) == 1 + assert check_files_matches_any.main(()) == 1 out, _ = capsys.readouterr() assert 'check-useless-excludes does not apply to this repository' in out @@ -124,7 +124,7 @@ def test_valid_includes( add_config_to_repo(repo, config) with cwd(repo): - assert check_files_matches_any.main(argv=[]) == 0 + assert check_files_matches_any.main(()) == 0 out, _ = capsys.readouterr() assert out == '' diff --git a/tests/meta_hooks/useless_excludes_test.py b/tests/meta_hooks/useless_excludes_test.py index 8b6ba7b0..08b87aa8 100644 --- a/tests/meta_hooks/useless_excludes_test.py +++ b/tests/meta_hooks/useless_excludes_test.py @@ -29,7 +29,7 @@ def test_useless_exclude_global(capsys, tempdir_factory): add_config_to_repo(repo, config) with cwd(repo): - assert check_useless_excludes.main(argv=[]) == 1 + assert check_useless_excludes.main(()) == 1 out, _ = capsys.readouterr() assert "The global exclude pattern 'foo' does not match any files" in out @@ -52,7 +52,7 @@ def test_useless_exclude_for_hook(capsys, tempdir_factory): add_config_to_repo(repo, config) with cwd(repo): - assert check_useless_excludes.main(argv=[]) == 1 + assert check_useless_excludes.main(()) == 1 out, _ = capsys.readouterr() expected = ( @@ -78,7 +78,7 @@ def test_no_excludes(capsys, tempdir_factory): add_config_to_repo(repo, config) with cwd(repo): - assert check_useless_excludes.main(argv=[]) == 0 + assert check_useless_excludes.main(()) == 0 out, _ = capsys.readouterr() assert out == '' @@ -101,7 +101,7 @@ def test_valid_exclude(capsys, tempdir_factory): add_config_to_repo(repo, config) with cwd(repo): - assert check_useless_excludes.main(argv=[]) == 0 + assert check_useless_excludes.main(()) == 0 out, _ = capsys.readouterr() assert out == ''