Skip to content

Commit 6af6015

Browse files
asottileasottile
committed
Refactor filename collection for hooks
1 parent 3cc5aa0 commit 6af6015

4 files changed

Lines changed: 80 additions & 106 deletions

File tree

‎pre_commit/commands/run.py‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import logging
55
import os
6+
import re
67
import subprocess
78
import sys
89

@@ -36,7 +37,19 @@ def _hook_msg_start(hook, verbose):
3637
)
3738

3839

39-
def filter_filenames_by_types(filenames, types, exclude_types):
40+
def _filter_by_include_exclude(filenames, include, exclude):
41+
include_re, exclude_re = re.compile(include), re.compile(exclude)
42+
return {
43+
filename for filename in filenames
44+
if (
45+
include_re.search(filename) and
46+
not exclude_re.search(filename) and
47+
os.path.lexists(filename)
48+
)
49+
}
50+
51+
52+
def _filter_by_types(filenames, types, exclude_types):
4053
types, exclude_types = frozenset(types), frozenset(exclude_types)
4154
ret = []
4255
for filename in filenames:
@@ -46,34 +59,15 @@ def filter_filenames_by_types(filenames, types, exclude_types):
4659
return tuple(ret)
4760

4861

49-
def get_filenames(args, include_expr, exclude_expr):
50-
if args.origin and args.source:
51-
getter = git.get_files_matching(
52-
lambda: git.get_changed_files(args.origin, args.source),
53-
)
54-
elif args.hook_stage == 'commit-msg':
55-
def getter(*_):
56-
return (args.commit_msg_filename,)
57-
elif args.files:
58-
getter = git.get_files_matching(lambda: args.files)
59-
elif args.all_files:
60-
getter = git.get_all_files_matching
61-
elif git.is_in_merge_conflict():
62-
getter = git.get_conflicted_files_matching
63-
else:
64-
getter = git.get_staged_files_matching
65-
return getter(include_expr, exclude_expr)
66-
67-
6862
SKIPPED = 'Skipped'
6963
NO_FILES = '(no files to check)'
7064

7165

72-
def _run_single_hook(hook, repo, args, skips, cols):
73-
filenames = get_filenames(args, hook['files'], hook['exclude'])
74-
filenames = filter_filenames_by_types(
75-
filenames, hook['types'], hook['exclude_types'],
76-
)
66+
def _run_single_hook(filenames, hook, repo, args, skips, cols):
67+
include, exclude = hook['files'], hook['exclude']
68+
filenames = _filter_by_include_exclude(filenames, include, exclude)
69+
types, exclude_types = hook['types'], hook['exclude_types']
70+
filenames = _filter_by_types(filenames, types, exclude_types)
7771
if hook['id'] in skips:
7872
output.write(get_hook_message(
7973
_hook_msg_start(hook, args.verbose),
@@ -169,13 +163,29 @@ def _compute_cols(hooks, verbose):
169163
return max(cols, 80)
170164

171165

166+
def _all_filenames(args):
167+
if args.origin and args.source:
168+
return git.get_changed_files(args.origin, args.source)
169+
elif args.hook_stage == 'commit-msg':
170+
return (args.commit_msg_filename,)
171+
elif args.files:
172+
return args.files
173+
elif args.all_files:
174+
return git.get_all_files()
175+
elif git.is_in_merge_conflict():
176+
return git.get_conflicted_files()
177+
else:
178+
return git.get_staged_files()
179+
180+
172181
def _run_hooks(config, repo_hooks, args, environ):
173182
"""Actually run the hooks."""
174183
skips = _get_skips(environ)
175184
cols = _compute_cols([hook for _, hook in repo_hooks], args.verbose)
185+
filenames = _all_filenames(args)
176186
retval = 0
177187
for repo, hook in repo_hooks:
178-
retval |= _run_single_hook(hook, repo, args, skips, cols)
188+
retval |= _run_single_hook(filenames, hook, repo, args, skips, cols)
179189
if retval and config['fail_fast']:
180190
break
181191
if (

‎pre_commit/git.py‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
from __future__ import unicode_literals
22

3-
import functools
43
import logging
54
import os.path
6-
import re
75
import sys
86

97
from pre_commit.errors import FatalError
108
from pre_commit.util import CalledProcessError
119
from pre_commit.util import cmd_output
12-
from pre_commit.util import memoize_by_cwd
1310

1411

1512
logger = logging.getLogger('pre_commit')
@@ -63,7 +60,6 @@ def parse_merge_msg_for_conflicts(merge_msg):
6360
]
6461

6562

66-
@memoize_by_cwd
6763
def get_conflicted_files():
6864
logger.info('Checking merge-conflict files only.')
6965
# Need to get the conflicted files from the MERGE_MSG because they could
@@ -82,7 +78,6 @@ def get_conflicted_files():
8278
return set(merge_conflict_filenames) | set(merge_diff_filenames)
8379

8480

85-
@memoize_by_cwd
8681
def get_staged_files():
8782
return zsplit(cmd_output(
8883
'git', 'diff', '--staged', '--name-only', '--no-ext-diff', '-z',
@@ -91,7 +86,6 @@ def get_staged_files():
9186
)[1])
9287

9388

94-
@memoize_by_cwd
9589
def get_all_files():
9690
return zsplit(cmd_output('git', 'ls-files', '-z')[1])
9791

@@ -103,29 +97,6 @@ def get_changed_files(new, old):
10397
)[1])
10498

10599

106-
def get_files_matching(all_file_list_strategy):
107-
@functools.wraps(all_file_list_strategy)
108-
@memoize_by_cwd
109-
def wrapper(include_expr, exclude_expr):
110-
include_regex = re.compile(include_expr)
111-
exclude_regex = re.compile(exclude_expr)
112-
return {
113-
filename
114-
for filename in all_file_list_strategy()
115-
if (
116-
include_regex.search(filename) and
117-
not exclude_regex.search(filename) and
118-
os.path.lexists(filename)
119-
)
120-
}
121-
return wrapper
122-
123-
124-
get_staged_files_matching = get_files_matching(get_staged_files)
125-
get_all_files_matching = get_files_matching(get_all_files)
126-
get_conflicted_files_matching = get_files_matching(get_conflicted_files)
127-
128-
129100
def check_for_cygwin_mismatch():
130101
"""See https://github.com/pre-commit/pre-commit/issues/354"""
131102
if sys.platform in ('cygwin', 'win32'): # pragma: no cover (windows)

‎tests/commands/run_test.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import pre_commit.constants as C
1313
from pre_commit.commands.install_uninstall import install
1414
from pre_commit.commands.run import _compute_cols
15+
from pre_commit.commands.run import _filter_by_include_exclude
1516
from pre_commit.commands.run import _get_skips
1617
from pre_commit.commands.run import _has_unmerged_paths
1718
from pre_commit.commands.run import run
@@ -25,6 +26,7 @@
2526
from testing.fixtures import modify_config
2627
from testing.fixtures import read_config
2728
from testing.util import cmd_output_mocked_pre_commit_home
29+
from testing.util import xfailif_no_symlink
2830

2931

3032
@pytest.yield_fixture
@@ -744,3 +746,45 @@ def test_fail_fast(
744746
ret, printed = _do_run(cap_out, repo_with_failing_hook, _get_opts())
745747
# it should have only run one hook
746748
assert printed.count(b'Failing hook') == 1
749+
750+
751+
@pytest.fixture
752+
def some_filenames():
753+
return (
754+
'.pre-commit-hooks.yaml',
755+
'pre_commit/main.py',
756+
'pre_commit/git.py',
757+
'im_a_file_that_doesnt_exist.py',
758+
)
759+
760+
761+
def test_include_exclude_base_case(some_filenames):
762+
ret = _filter_by_include_exclude(some_filenames, '', '^$')
763+
assert ret == {
764+
'.pre-commit-hooks.yaml',
765+
'pre_commit/main.py',
766+
'pre_commit/git.py',
767+
}
768+
769+
770+
@xfailif_no_symlink
771+
def test_matches_broken_symlink(tmpdir): # pramga: no cover (non-windows)
772+
with tmpdir.as_cwd():
773+
os.symlink('does-not-exist', 'link')
774+
ret = _filter_by_include_exclude({'link'}, '', '^$')
775+
assert ret == {'link'}
776+
777+
778+
def test_include_exclude_total_match(some_filenames):
779+
ret = _filter_by_include_exclude(some_filenames, r'^.*\.py$', '^$')
780+
assert ret == {'pre_commit/main.py', 'pre_commit/git.py'}
781+
782+
783+
def test_include_exclude_does_search_instead_of_match(some_filenames):
784+
ret = _filter_by_include_exclude(some_filenames, r'\.yaml$', '^$')
785+
assert ret == {'.pre-commit-hooks.yaml'}
786+
787+
788+
def test_include_exclude_exclude_removes_files(some_filenames):
789+
ret = _filter_by_include_exclude(some_filenames, '', r'\.py$')
790+
assert ret == {'.pre-commit-hooks.yaml'}

‎tests/git_test.py‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from pre_commit.util import cmd_output
1212
from pre_commit.util import cwd
1313
from testing.fixtures import git_dir
14-
from testing.util import xfailif_no_symlink
1514

1615

1716
def test_get_root_at_root(tempdir_factory):
@@ -66,56 +65,6 @@ def test_cherry_pick_conflict(in_merge_conflict):
6665
assert git.is_in_merge_conflict() is False
6766

6867

69-
@pytest.fixture
70-
def get_files_matching_func():
71-
def get_filenames():
72-
return (
73-
'.pre-commit-hooks.yaml',
74-
'pre_commit/main.py',
75-
'pre_commit/git.py',
76-
'im_a_file_that_doesnt_exist.py',
77-
)
78-
79-
return git.get_files_matching(get_filenames)
80-
81-
82-
def test_get_files_matching_base(get_files_matching_func):
83-
ret = get_files_matching_func('', '^$')
84-
assert ret == {
85-
'.pre-commit-hooks.yaml',
86-
'pre_commit/main.py',
87-
'pre_commit/git.py',
88-
}
89-
90-
91-
@xfailif_no_symlink
92-
def test_matches_broken_symlink(tmpdir): # pragma: no cover (non-windwos)
93-
with tmpdir.as_cwd():
94-
os.symlink('does-not-exist', 'link')
95-
func = git.get_files_matching(lambda: ('link',))
96-
assert func('', '^$') == {'link'}
97-
98-
99-
def test_get_files_matching_total_match(get_files_matching_func):
100-
ret = get_files_matching_func('^.*\\.py$', '^$')
101-
assert ret == {'pre_commit/main.py', 'pre_commit/git.py'}
102-
103-
104-
def test_does_search_instead_of_match(get_files_matching_func):
105-
ret = get_files_matching_func('\\.yaml$', '^$')
106-
assert ret == {'.pre-commit-hooks.yaml'}
107-
108-
109-
def test_does_not_include_deleted_fileS(get_files_matching_func):
110-
ret = get_files_matching_func('exist.py', '^$')
111-
assert ret == set()
112-
113-
114-
def test_exclude_removes_files(get_files_matching_func):
115-
ret = get_files_matching_func('', '\\.py$')
116-
assert ret == {'.pre-commit-hooks.yaml'}
117-
118-
11968
def resolve_conflict():
12069
with open('conflict_file', 'w') as conflicted_file:
12170
conflicted_file.write('herp\nderp\n')

0 commit comments

Comments
 (0)