Skip to content

Commit

Permalink
Short-circuit hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
mxr committed Sep 10, 2023
1 parent e2c6a82 commit c647535
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 32 deletions.
40 changes: 20 additions & 20 deletions pre_commit/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import unicodedata
from typing import Any
from typing import Collection
from typing import Generator
from typing import Iterable
from typing import MutableMapping
from typing import Sequence

Expand Down Expand Up @@ -57,20 +59,20 @@ def _full_msg(


def filter_by_include_exclude(
names: Collection[str],
names: Iterable[str],
include: str,
exclude: str,
) -> list[str]:
) -> Generator[str, None, None]:
include_re, exclude_re = re.compile(include), re.compile(exclude)
return [
return (
filename for filename in names
if include_re.search(filename)
if not exclude_re.search(filename)
]
)


class Classifier:
def __init__(self, filenames: Collection[str]) -> None:
def __init__(self, filenames: Iterable[str]) -> None:
self.filenames = [f for f in filenames if os.path.lexists(f)]

@functools.lru_cache(maxsize=None)
Expand All @@ -79,35 +81,33 @@ def _types_for_file(self, filename: str) -> set[str]:

def by_types(
self,
names: Sequence[str],
names: Iterable[str],
types: Collection[str],
types_or: Collection[str],
exclude_types: Collection[str],
) -> list[str]:
) -> Generator[str, None, None]:
types = frozenset(types)
types_or = frozenset(types_or)
exclude_types = frozenset(exclude_types)
ret = []
for filename in names:
tags = self._types_for_file(filename)
if (
tags >= types and
(not types_or or tags & types_or) and
not tags & exclude_types
):
ret.append(filename)
return ret

def filenames_for_hook(self, hook: Hook) -> tuple[str, ...]:
names = self.filenames
names = filter_by_include_exclude(names, hook.files, hook.exclude)
names = self.by_types(
names,
yield filename

def filenames_for_hook(self, hook: Hook) -> Generator[str, None, None]:
names = iter(self.filenames)
filtered = filter_by_include_exclude(names, hook.files, hook.exclude)
by_types = self.by_types(
filtered,
hook.types,
hook.types_or,
hook.exclude_types,
)
return tuple(names)
return by_types

@classmethod
def from_config(
Expand All @@ -122,8 +122,8 @@ def from_config(
# see #1173
if os.altsep == '/' and os.sep == '\\':
filenames = [f.replace(os.sep, os.altsep) for f in filenames]
filenames = filter_by_include_exclude(filenames, include, exclude)
return Classifier(filenames)
filtered = filter_by_include_exclude(filenames, include, exclude)
return Classifier(filtered)


def _get_skips(environ: MutableMapping[str, str]) -> set[str]:
Expand All @@ -148,7 +148,7 @@ def _run_single_hook(
verbose: bool,
use_color: bool,
) -> tuple[bool, bytes]:
filenames = classifier.filenames_for_hook(hook)
filenames = tuple(classifier.filenames_for_hook(hook))

if hook.id in skips or hook.alias in skips:
output.write(
Expand Down
2 changes: 1 addition & 1 deletion pre_commit/meta_hooks/check_hooks_apply.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def check_all_hooks_match_files(config_file: str) -> int:
for hook in all_hooks(config, Store()):
if hook.always_run or hook.language == 'fail':
continue
elif not classifier.filenames_for_hook(hook):
elif not any(classifier.filenames_for_hook(hook)):
print(f'{hook.id} does not apply to this repository')
retv = 1

Expand Down
9 changes: 6 additions & 3 deletions pre_commit/meta_hooks/check_useless_excludes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import argparse
import re
from typing import Iterable
from typing import Sequence

from cfgv import apply_defaults
Expand All @@ -14,7 +15,7 @@


def exclude_matches_any(
filenames: Sequence[str],
filenames: Iterable[str],
include: str,
exclude: str,
) -> bool:
Expand Down Expand Up @@ -54,9 +55,11 @@ def check_useless_excludes(config_file: str) -> int:
types = hook['types']
types_or = hook['types_or']
exclude_types = hook['exclude_types']
names = classifier.by_types(names, types, types_or, exclude_types)
by_types = classifier.by_types(
names, types, types_or, exclude_types,
)
include, exclude = hook['files'], hook['exclude']
if not exclude_matches_any(names, include, exclude):
if not exclude_matches_any(by_types, include, exclude):
print(
f'The exclude pattern {exclude!r} for {hook["id"]} does '
f'not match any files',
Expand Down
16 changes: 8 additions & 8 deletions tests/commands/run_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1127,8 +1127,8 @@ def test_classifier_empty_types_or(tmpdir):
types_or=[],
exclude_types=[],
)
assert for_symlink == ['foo']
assert for_file == ['bar']
assert tuple(for_symlink) == ('foo',)
assert tuple(for_file) == ('bar',)


@pytest.fixture
Expand All @@ -1142,33 +1142,33 @@ def some_filenames():

def test_include_exclude_base_case(some_filenames):
ret = filter_by_include_exclude(some_filenames, '', '^$')
assert ret == [
assert tuple(ret) == (
'.pre-commit-hooks.yaml',
'pre_commit/git.py',
'pre_commit/main.py',
]
)


def test_matches_broken_symlink(tmpdir):
with tmpdir.as_cwd():
os.symlink('does-not-exist', 'link')
ret = filter_by_include_exclude({'link'}, '', '^$')
assert ret == ['link']
assert tuple(ret) == ('link',)


def test_include_exclude_total_match(some_filenames):
ret = filter_by_include_exclude(some_filenames, r'^.*\.py$', '^$')
assert ret == ['pre_commit/git.py', 'pre_commit/main.py']
assert tuple(ret) == ('pre_commit/git.py', 'pre_commit/main.py')


def test_include_exclude_does_search_instead_of_match(some_filenames):
ret = filter_by_include_exclude(some_filenames, r'\.yaml$', '^$')
assert ret == ['.pre-commit-hooks.yaml']
assert tuple(ret) == ('.pre-commit-hooks.yaml',)


def test_include_exclude_exclude_removes_files(some_filenames):
ret = filter_by_include_exclude(some_filenames, '', r'\.py$')
assert ret == ['.pre-commit-hooks.yaml']
assert tuple(ret) == ('.pre-commit-hooks.yaml',)


def test_args_hook_only(cap_out, store, repo_with_passing_hook):
Expand Down

0 comments on commit c647535

Please sign in to comment.