From 00a1aac45aae57d2a619e7b1672ca90107d84787 Mon Sep 17 00:00:00 2001 From: Radek SPRTA Date: Mon, 20 Sep 2021 21:10:14 +0200 Subject: [PATCH] Check for regex slashes in OptionalSensibleRegexAt* --- pre_commit/clientlib.py | 34 +++++------------- tests/clientlib_test.py | 78 ++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 73 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index e1a8d2a30..f0a92810a 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -143,6 +143,12 @@ def check(self, dct: Dict[str, Any]) -> None: f"regex, not a glob -- matching '/*' probably isn't what you " f'want here', ) + if r'[\/]' in dct.get(self.key, ''): + logger.warning( + fr'pre-commit normalizes the slashes in {self.key!r} field in ' + fr"hook {dct.get('id')!r} to forward slashes, so you can use " + fr'/ instead of [\/]', + ) class OptionalSensibleRegexAtTop(cfgv.OptionalNoDefault): @@ -154,29 +160,11 @@ def check(self, dct: Dict[str, Any]) -> None: f'The top-level {self.key!r} field is a regex, not a glob -- ' f"matching '/*' probably isn't what you want here", ) - - -class OptionalSensibleSlashesAtHook(cfgv.OptionalNoDefault): - def check(self, dct: Dict[str, Any]) -> None: - super().check(dct) - - if r'[\/]' in dct.get(self.key, ''): - logger.warning( - f'Pre-commit normalizes the slashes in {self.key!r} field in ' - f"hook {dct.get('id')!r} to forward slashes, so you can use " - f'/ instead of [\\/]', - ) - - -class OptionalSensibleSlashesAtTop(cfgv.OptionalNoDefault): - def check(self, dct: Dict[str, Any]) -> None: - super().check(dct) - if r'[\/]' in dct.get(self.key, ''): logger.warning( - f'Pre-commit normalizes the slashes in the top-level ' - f'{self.key!r} field to forward slashes, so you can use / ' - f'instead of [\\/]', + fr'pre-commit normalizes the slashes in the top-level ' + fr'{self.key!r} field to forward slashes, so you can use / ' + fr'instead of [\/]', ) @@ -297,8 +285,6 @@ def warn_unknown_keys_repo( ), OptionalSensibleRegexAtHook('files', cfgv.check_string), OptionalSensibleRegexAtHook('exclude', cfgv.check_string), - OptionalSensibleSlashesAtHook('files', cfgv.check_string), - OptionalSensibleSlashesAtHook('exclude', cfgv.check_string), ) CONFIG_REPO_DICT = cfgv.Map( 'Repository', 'repo', @@ -369,8 +355,6 @@ def warn_unknown_keys_repo( ), OptionalSensibleRegexAtTop('files', cfgv.check_string), OptionalSensibleRegexAtTop('exclude', cfgv.check_string), - OptionalSensibleSlashesAtTop('files', cfgv.check_string), - OptionalSensibleSlashesAtTop('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 dd87ebaa5..f354c3cb3 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -247,72 +247,54 @@ def test_warn_mutable_rev_conditional(): cfgv.validate(config_obj, CONFIG_REPO_DICT) -def test_validate_optional_sensible_regex_at_hook_level(caplog): - config_obj = { - 'id': 'flake8', - 'files': 'dir/*.py', - } - cfgv.validate(config_obj, CONFIG_HOOK_DICT) - - assert caplog.record_tuples == [ +@pytest.mark.parametrize( + ('regex', 'warning'), + [ ( - 'pre_commit', - logging.WARNING, + r'dir/*.py', "The 'files' field in hook 'flake8' is a regex, not a glob -- " "matching '/*' probably isn't what you want here", ), - ] - - -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", + r'dir[\/].*\.py', + r"pre-commit normalizes the slashes in 'files' field in hook " + r"'flake8' to forward slashes, so you can use / instead of [\/]", ), - ] - - -def test_validate_optional_sensible_slashes_at_hook_level(caplog): + ], +) +def test_validate_optional_sensible_regex_at_hook_level(caplog, regex, warning): config_obj = { 'id': 'flake8', - 'files': r'dir[\/].*\.py', + 'files': regex, } cfgv.validate(config_obj, CONFIG_HOOK_DICT) - assert caplog.record_tuples == [ - ( - 'pre_commit', - logging.WARNING, - "Pre-commit normalizes the slashes in 'files' field in hook " - "'flake8' to forward slashes, so you can use / instead of [\\/]", - ), - ] + assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)] -def test_validate_optional_sensible_slashes_at_top_level(caplog): +@pytest.mark.parametrize( + ('regex', 'warning'), + [ + ( + r'dir/*.py', + "The top-level 'files' field is a regex, not a glob -- " + "matching '/*' probably isn't what you want here", + ), + ( + r'dir[\/].*\.py', + r"pre-commit normalizes the slashes in the top-level 'files' field " + r'to forward slashes, so you can use / instead of [\/]', + ), + ], +) +def test_validate_optional_sensible_regex_at_top_level(caplog, regex, warning): config_obj = { - 'files': r'dir[\/].*\.py', + 'files': regex, 'repos': [], } cfgv.validate(config_obj, CONFIG_SCHEMA) - assert caplog.record_tuples == [ - ( - 'pre_commit', - logging.WARNING, - "Pre-commit normalizes the slashes in the top-level 'files' field " - 'to forward slashes, so you can use / instead of [\\/]', - ), - ] + assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)] @pytest.mark.parametrize('fn', (validate_config_main, validate_manifest_main))