From cef9c4af03e954e5fe3e8746a49ac59e4781747d Mon Sep 17 00:00:00 2001 From: Radek SPRTA Date: Fri, 17 Sep 2021 21:16:11 +0200 Subject: [PATCH] Add warning for regular expression with [\/] (#2043) --- pre_commit/clientlib.py | 24 ++++++++++++++++ tests/clientlib_test.py | 64 +++++++++++++++++++++++++++++------------ 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index bc7274a7d..7d87ee043 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -143,6 +143,18 @@ 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 slashes in the {self.key!r} field ' + fr'in hook {dct.get("id")!r} to forward slashes, so you ' + fr'can use / instead of [\/]', + ) + if r'[/\\]' in dct.get(self.key, ''): + logger.warning( + fr'pre-commit normalizes slashes in the {self.key!r} field ' + fr'in hook {dct.get("id")!r} to forward slashes, so you ' + fr'can use / instead of [/\\]', + ) class OptionalSensibleRegexAtTop(cfgv.OptionalNoDefault): @@ -154,6 +166,18 @@ 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", ) + if r'[\/]' in dct.get(self.key, ''): + logger.warning( + 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 [\/]', + ) + if r'[/\\]' in dct.get(self.key, ''): + logger.warning( + 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 [/\\]', + ) class MigrateShaToRev: diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index da794e6e7..5427b1dae 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -247,38 +247,64 @@ def test_warn_mutable_rev_conditional(): cfgv.validate(config_obj, CONFIG_REPO_DICT) -def test_validate_optional_sensible_regex_at_hook_level(caplog): +@pytest.mark.parametrize( + ('regex', 'warning'), + ( + ( + r'dir/*.py', + "The 'files' field in hook 'flake8' is a regex, not a glob -- " + "matching '/*' probably isn't what you want here", + ), + ( + r'dir[\/].*\.py', + r"pre-commit normalizes slashes in the 'files' field in hook " + r"'flake8' to forward slashes, so you can use / instead of [\/]", + ), + ( + r'dir[/\\].*\.py', + r"pre-commit normalizes slashes in the 'files' field in hook " + r"'flake8' to forward slashes, so you can use / instead of [/\\]", + ), + ), +) +def test_validate_optional_sensible_regex_at_hook(caplog, regex, warning): config_obj = { 'id': 'flake8', - 'files': 'dir/*.py', + 'files': regex, } cfgv.validate(config_obj, CONFIG_HOOK_DICT) - assert caplog.record_tuples == [ + assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)] + + +@pytest.mark.parametrize( + ('regex', 'warning'), + ( ( - 'pre_commit', - logging.WARNING, - "The 'files' field in hook 'flake8' is a regex, not a glob -- " + r'dir/*.py', + "The top-level 'files' field is a regex, not a glob -- " "matching '/*' probably isn't what you want here", ), - ] - - -def test_validate_optional_sensible_regex_at_top_level(caplog): + ( + r'dir[\/].*\.py', + r"pre-commit normalizes the slashes in the top-level 'files' " + r'field to forward slashes, so you can use / instead of [\/]', + ), + ( + r'dir[/\\].*\.py', + r"pre-commit normalizes the slashes in the top-level 'files' " + r'field to forward slashes, so you can use / instead of [/\\]', + ), + ), +) +def test_validate_optional_sensible_regex_at_top_level(caplog, regex, warning): config_obj = { - 'files': 'dir/*.py', + 'files': regex, '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", - ), - ] + assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)] @pytest.mark.parametrize('fn', (validate_config_main, validate_manifest_main))