Skip to content

Commit

Permalink
extend warning if globs are used instead of regex to local hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisRedwine committed Sep 25, 2022
1 parent b1de333 commit 8315892
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
10 changes: 9 additions & 1 deletion pre_commit/clientlib.py
Expand Up @@ -298,6 +298,14 @@ def check(self, dct: dict[str, Any]) -> None:
OptionalSensibleRegexAtHook('files', cfgv.check_string),
OptionalSensibleRegexAtHook('exclude', cfgv.check_string),
)
LOCAL_HOOK_DICT = cfgv.Map(
'Hook', 'id',

*MANIFEST_HOOK_DICT.items,

OptionalSensibleRegexAtHook('files', cfgv.check_string),
OptionalSensibleRegexAtHook('exclude', cfgv.check_string),
)
CONFIG_REPO_DICT = cfgv.Map(
'Repository', 'repo',

Expand All @@ -308,7 +316,7 @@ def check(self, dct: dict[str, Any]) -> None:
'repo', cfgv.NotIn(LOCAL, META),
),
cfgv.ConditionalRecurse(
'hooks', cfgv.Array(MANIFEST_HOOK_DICT),
'hooks', cfgv.Array(LOCAL_HOOK_DICT),
'repo', LOCAL,
),
cfgv.ConditionalRecurse(
Expand Down
39 changes: 39 additions & 0 deletions tests/clientlib_test.py
Expand Up @@ -15,6 +15,8 @@
from pre_commit.clientlib import MANIFEST_SCHEMA
from pre_commit.clientlib import META_HOOK_DICT
from pre_commit.clientlib import MigrateShaToRev
from pre_commit.clientlib import OptionalSensibleRegexAtHook
from pre_commit.clientlib import OptionalSensibleRegexAtTop
from pre_commit.clientlib import validate_config_main
from pre_commit.clientlib import validate_manifest_main
from testing.fixtures import sample_local_config
Expand Down Expand Up @@ -261,6 +263,27 @@ def test_warn_mutable_rev_conditional():
cfgv.validate(config_obj, CONFIG_REPO_DICT)


@pytest.mark.parametrize(
'validator_cls',
(
OptionalSensibleRegexAtHook,
OptionalSensibleRegexAtTop,
),
)
def test_sensible_regex_validators_dont_pass_none(caplog, validator_cls):
key = 'files'
with pytest.raises(cfgv.ValidationError) as excinfo:
validator = validator_cls(key, cfgv.check_string)
validator.check({key: None})

assert str(excinfo.value) == (
'\n'
f'==> At key: {key}'
'\n'
'=====> Expected string got NoneType'
)


@pytest.mark.parametrize(
('regex', 'warning'),
(
Expand Down Expand Up @@ -296,6 +319,22 @@ def test_validate_optional_sensible_regex_at_hook(caplog, regex, warning):
assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)]


def test_validate_optional_sensible_regex_at_local_hook(caplog):
config_obj = sample_local_config()
config_obj['hooks'][0]['files'] = r'dir/*.py'

cfgv.validate(config_obj, CONFIG_REPO_DICT)

assert caplog.record_tuples == [
(
'pre_commit',
logging.WARNING,
"The 'files' field in hook 'do_not_commit' is a regex, not a glob "
"-- matching '/*' probably isn't what you want here",
),
]


@pytest.mark.parametrize(
('regex', 'warning'),
(
Expand Down

0 comments on commit 8315892

Please sign in to comment.