From b1a9209f9f9a4b4b8d6bdf5d2c0660d70c6b3312 Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Fri, 1 Jan 2021 23:57:24 +0100 Subject: [PATCH] extended warning if globs are used instead of regex to top level --- pre_commit/clientlib.py | 19 ++++++++++++++++--- tests/clientlib_test.py | 19 ++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 5dfaf7a3..8f35057d 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -132,7 +132,7 @@ class WarnMutableRev(cfgv.ConditionalOptional): ) -class OptionalSensibleRegex(cfgv.OptionalNoDefault): +class OptionalSensibleRegexAtHook(cfgv.OptionalNoDefault): def check(self, dct: Dict[str, Any]) -> None: super().check(dct) @@ -144,6 +144,17 @@ class OptionalSensibleRegex(cfgv.OptionalNoDefault): ) +class OptionalSensibleRegexAtTop(cfgv.OptionalNoDefault): + def check(self, dct: Dict[str, Any]) -> None: + super().check(dct) + + if '/*' in dct.get(self.key, ''): + logger.warning( + f'The top-level {self.key!r} field is a regex, not a glob -- ' + f"matching '/*' probably isn't what you want here", + ) + + class MigrateShaToRev: key = 'rev' @@ -259,8 +270,8 @@ CONFIG_HOOK_DICT = cfgv.Map( for item in MANIFEST_HOOK_DICT.items if item.key != 'id' ), - OptionalSensibleRegex('files', cfgv.check_string), - OptionalSensibleRegex('exclude', cfgv.check_string), + OptionalSensibleRegexAtHook('files', cfgv.check_string), + OptionalSensibleRegexAtHook('exclude', cfgv.check_string), ) CONFIG_REPO_DICT = cfgv.Map( 'Repository', 'repo', @@ -329,6 +340,8 @@ CONFIG_SCHEMA = cfgv.Map( ), warn_unknown_keys_root, ), + OptionalSensibleRegexAtTop('files', cfgv.check_string), + OptionalSensibleRegexAtTop('exclude', cfgv.check_string), # do not warn about configuration for pre-commit.ci cfgv.OptionalNoDefault('ci', cfgv.check_type(dict)), diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index d08ecdf0..6bdb0d62 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -244,7 +244,7 @@ def test_warn_mutable_rev_conditional(): cfgv.validate(config_obj, CONFIG_REPO_DICT) -def test_validate_optional_sensible_regex(caplog): +def test_validate_optional_sensible_regex_at_hook_level(caplog): config_obj = { 'id': 'flake8', 'files': 'dir/*.py', @@ -261,6 +261,23 @@ def test_validate_optional_sensible_regex(caplog): ] +def test_validate_optional_sensible_regex_at_top_level(caplog): + config_obj = { + 'files': 'dir/*.py', + 'repos': [], + } + cfgv.validate(config_obj, CONFIG_SCHEMA) + + assert caplog.record_tuples == [ + ( + 'pre_commit', + logging.WARNING, + "The top-level 'files' field is a regex, not a glob -- matching " + "'/*' probably isn't what you want here", + ), + ] + + @pytest.mark.parametrize('fn', (validate_config_main, validate_manifest_main)) def test_mains_not_ok(tmpdir, fn): not_yaml = tmpdir.join('f.notyaml')