mirror of
https://github.com/pre-commit/pre-commit.git
synced 2026-01-13 12:30:08 -06:00
Refactor meta hooks
This commit is contained in:
Paul Hooijenga
@@ -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())
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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']
|
||||
)
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
0
tests/meta_hooks/__init__.py
Normal file
0
tests/meta_hooks/__init__.py
Normal file
130
tests/meta_hooks/hook_matches_any_test.py
Normal file
130
tests/meta_hooks/hook_matches_any_test.py
Normal file
@@ -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 == ''
|
||||
107
tests/meta_hooks/useless_excludes_test.py
Normal file
107
tests/meta_hooks/useless_excludes_test.py
Normal file
@@ -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 == ''
|
||||
@@ -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.'
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user