Merge pull request #2524 from chrisRedwine/fix-local-regex-warnings

extend warning if globs are used instead of regex to local hooks

pre_commit/clientlib.py

@@ -298,6 +298,14 @@ CONFIG_HOOK_DICT = cfgv.Map(
     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',
 
@@ -308,7 +316,7 @@ CONFIG_REPO_DICT = cfgv.Map(
         'repo', cfgv.NotIn(LOCAL, META),
     ),
     cfgv.ConditionalRecurse(
-        'hooks', cfgv.Array(MANIFEST_HOOK_DICT),
+        'hooks', cfgv.Array(LOCAL_HOOK_DICT),
         'repo', LOCAL,
     ),
     cfgv.ConditionalRecurse(

tests/clientlib_test.py

@@ -15,6 +15,8 @@ from pre_commit.clientlib import DEFAULT_LANGUAGE_VERSION
 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
@@ -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(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'),
     (
@@ -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'),
     (