diff --git a/pre_commit/meta_hooks/check_files_matches_any.py b/pre_commit/meta_hooks/check_files_matches_any.py index 88b4806f..d253939e 100644 --- a/pre_commit/meta_hooks/check_files_matches_any.py +++ b/pre_commit/meta_hooks/check_files_matches_any.py @@ -1,36 +1,40 @@ -import re -import sys +import argparse import pre_commit.constants as C -from pre_commit.clientlib import load_config +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 files_matches_any(filenames, include): - include_re = re.compile(include) - for filename in filenames: - if include_re.search(filename): - return True - return False - - -def check_files_matches_any(config_file=None): - config = load_config(config_file or C.CONFIG_FILE) +def check_all_hooks_match_files(config_file): + runner = Runner.create(config_file) files = get_all_files() - files_not_matched = False + files_matched = True - for repo in config['repos']: - for hook in repo['hooks']: - include = hook.get('files', '') - if include and not files_matches_any(files, include): - print( - 'The files pattern for {} does not match any files' - .format(hook['id']) - ) - files_not_matched = True + 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)) + files_matched = False - return files_not_matched + return files_matched + + +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 |= not check_all_hooks_match_files(filename) + return retv if __name__ == '__main__': - sys.exit(check_files_matches_any()) + exit(main()) diff --git a/pre_commit/meta_hooks/check_useless_excludes.py b/pre_commit/meta_hooks/check_useless_excludes.py index 8e891bc1..89448d78 100644 --- a/pre_commit/meta_hooks/check_useless_excludes.py +++ b/pre_commit/meta_hooks/check_useless_excludes.py @@ -1,5 +1,7 @@ +from __future__ import print_function + +import argparse import re -import sys import pre_commit.constants as C from pre_commit.clientlib import load_config @@ -14,14 +16,17 @@ def exclude_matches_any(filenames, include, exclude): return False -def check_useless_excludes(config_file=None): - config = load_config(config_file or C.CONFIG_FILE) +def check_useless_excludes(config_file): + config = load_config(config_file) files = get_all_files() useless_excludes = False exclude = config.get('exclude') if exclude != '^$' and not exclude_matches_any(files, '', exclude): - print('The global exclude pattern does not match any files') + print( + 'The global exclude pattern {!r} does not match any files' + .format(exclude), + ) useless_excludes = True for repo in config['repos']: @@ -29,13 +34,24 @@ def check_useless_excludes(config_file=None): include, exclude = hook.get('files', ''), hook.get('exclude') if exclude and not exclude_matches_any(files, include, exclude): print( - 'The exclude pattern for {} does not match any files' - .format(hook['id']) + 'The exclude pattern {!r} for {} does not match any files' + .format(exclude, hook['id']), ) useless_excludes = True return useless_excludes +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__': - sys.exit(check_useless_excludes()) + exit(main()) diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 0afb5004..8adcbec0 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -252,50 +252,56 @@ class LocalRepository(Repository): class MetaRepository(LocalRepository): - # 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. - meta_hooks = { - 'check-useless-excludes': { - 'name': 'Check for useless excludes', - 'files': '.pre-commit-config.yaml', - 'language': 'system', - 'entry': pipes.quote(sys.executable), - 'args': ['-m', 'pre_commit.meta_hooks.check_useless_excludes'], - }, - 'check-files-matches-any': { - 'name': 'Check hooks match any files', - 'files': '.pre-commit-config.yaml', - 'language': 'system', - 'entry': pipes.quote(sys.executable), - 'args': ['-m', 'pre_commit.meta_hooks.check_files_matches_any'], - }, - } + @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 + + # 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. + 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__], + }, + ] + + return { + hook['id']: apply_defaults( + validate(hook, MANIFEST_HOOK_DICT), + MANIFEST_HOOK_DICT, + ) + for hook in meta_hooks + } @cached_property def hooks(self): for hook in self.repo_config['hooks']: - if hook['id'] not in self.meta_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 `pre-commit autoupdate` fixes this.'.format( - hook['id'], - ), + 'Often `pip install --upgrade pre-commit` fixes this.' + .format(hook['id']), ) exit(1) return tuple( - ( - hook['id'], - apply_defaults( - validate( - dict(self.meta_hooks[hook['id']], **hook), - MANIFEST_HOOK_DICT, - ), - MANIFEST_HOOK_DICT, - ), - ) + (hook['id'], _hook(self.manifest_hooks[hook['id']], hook)) for hook in self.repo_config['hooks'] ) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 24771d22..336222d6 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -670,104 +670,6 @@ def test_meta_hook_passes( ) -def test_useless_exclude_global( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): - config = OrderedDict(( - ('exclude', 'foo'), - ( - 'repos', [ - 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={'all_files': True}, - expected_outputs=[ - b'Check for useless excludes', - b'The global exclude pattern does not match any files', - ], - expected_ret=1, - stage=False, - ) - - -def test_useless_exclude_for_hook( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): - config = OrderedDict(( - ('repo', 'meta'), - ( - 'hooks', ( - OrderedDict(( - ('id', 'check-useless-excludes'), - ('exclude', 'foo'), - )), - ), - ), - )) - add_config_to_repo(repo_with_passing_hook, config) - - _test_run( - cap_out, - repo_with_passing_hook, - opts={'all_files': True}, - expected_outputs=[ - b'Check for useless excludes', - b'The exclude pattern for check-useless-excludes ' - b'does not match any files', - ], - expected_ret=1, - stage=False, - ) - - -def test_files_match_any( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): - config = OrderedDict(( - ('repo', 'meta'), - ( - 'hooks', ( - OrderedDict(( - ('id', 'check-files-matches-any'), - )), - OrderedDict(( - ('id', 'check-useless-excludes'), - ('files', 'foo'), - )), - ), - ), - )) - add_config_to_repo(repo_with_passing_hook, config) - - _test_run( - cap_out, - repo_with_passing_hook, - opts={'all_files': True}, - expected_outputs=[ - b'Check hooks match any files', - b'The files pattern for check-useless-excludes ' - b'does not match any files', - ], - expected_ret=1, - stage=False, - ) - - @pytest.yield_fixture def modified_config_repo(repo_with_passing_hook): with modify_config(repo_with_passing_hook, commit=False) as config: diff --git a/tests/meta_hooks/__init__.py b/tests/meta_hooks/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/meta_hooks/hook_matches_any_test.py b/tests/meta_hooks/hook_matches_any_test.py new file mode 100644 index 00000000..92c6fc45 --- /dev/null +++ b/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(argv=[]) == 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(argv=[]) == 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(argv=[]) == 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(argv=[]) == 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(argv=[]) == 0 + + out, _ = capsys.readouterr() + assert out == '' diff --git a/tests/meta_hooks/useless_excludes_test.py b/tests/meta_hooks/useless_excludes_test.py new file mode 100644 index 00000000..8b6ba7b0 --- /dev/null +++ b/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(argv=[]) == 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(argv=[]) == 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(argv=[]) == 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(argv=[]) == 0 + + out, _ = capsys.readouterr() + assert out == '' diff --git a/tests/repository_test.py b/tests/repository_test.py index 80489406..f7c027cd 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -729,7 +729,7 @@ def test_meta_hook_not_present(store, fake_log_handler): 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 `pre-commit autoupdate` fixes this.' + 'Often `pip install --upgrade pre-commit` fixes this.' )