From fd9d9d276b0bf727c48bd720248d88ff915686d8 Mon Sep 17 00:00:00 2001 From: Yoav Caspi Date: Sat, 11 May 2019 20:43:12 +0300 Subject: [PATCH 1/4] Add warning to additional keys in config --- pre_commit/clientlib.py | 17 +++++++++++++++++ tests/clientlib_test.py | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 2f16650a..a16a73ac 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import argparse import functools +import logging import pipes import sys @@ -15,6 +16,8 @@ from pre_commit.error_handler import FatalError from pre_commit.languages.all import all_languages from pre_commit.util import parse_version +logger = logging.getLogger('pre_commit') + def check_type_tag(tag): if tag not in ALL_TAGS: @@ -144,6 +147,16 @@ def _entry(modname): ) +def warn_on_unknown_keys_at_top_level(extra, orig_keys): + logger.warning( + 'Your pre-commit-config contain these extra keys: {}. ' + 'while the only valid keys are: {}.'.format( + ', '.join(extra), + ', '.join(sorted(orig_keys)), + ), + ), + + _meta = ( ( 'check-hooks-apply', ( @@ -222,6 +235,10 @@ CONFIG_REPO_DICT = cfgv.Map( ), MigrateShaToRev(), + cfgv.WarnAdditionalKeys( + {'repo', 'rev', 'hooks'}, + warn_on_unknown_keys_at_top_level, + ), ) DEFAULT_LANGUAGE_VERSION = cfgv.Map( 'DefaultLanguageVersion', None, diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 2cdc1528..069dca36 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals +import logging + import cfgv import pytest @@ -116,6 +118,27 @@ def test_validate_config_old_list_format_ok(tmpdir): assert not validate_config_main((f.strpath,)) +def test_validate_warn_on_unknown_keys_at_top_level(tmpdir, caplog): + f = tmpdir.join('cfg.yaml') + f.write( + '- repo: https://gitlab.com/pycqa/flake8\n' + ' rev: 3.7.7\n' + ' hooks:\n' + ' - id: flake8\n' + ' args: [--some-args]\n', + ) + ret_val = validate_config_main((f.strpath,)) + assert not ret_val + assert caplog.record_tuples == [ + ( + 'pre_commit', + logging.WARNING, + 'Your pre-commit-config contain these extra keys: args. ' + 'while the only valid keys are: hooks, repo, rev.', + ), + ] + + @pytest.mark.parametrize('fn', (validate_config_main, validate_manifest_main)) def test_mains_not_ok(tmpdir, fn): not_yaml = tmpdir.join('f.notyaml') From 217d31ec1cfe2ed67d360016d8b3f13e1ee16c1f Mon Sep 17 00:00:00 2001 From: Yoav Caspi Date: Sat, 11 May 2019 22:57:52 +0300 Subject: [PATCH 2/4] Add a check and test to the real top level and improve the warning message --- pre_commit/clientlib.py | 14 ++++++++------ tests/clientlib_test.py | 27 ++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index a16a73ac..3285a48b 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -149,12 +149,10 @@ def _entry(modname): def warn_on_unknown_keys_at_top_level(extra, orig_keys): logger.warning( - 'Your pre-commit-config contain these extra keys: {}. ' - 'while the only valid keys are: {}.'.format( - ', '.join(extra), - ', '.join(sorted(orig_keys)), + 'Unexpected config key(s): {}'.format( + ', '.join(sorted(extra)), ), - ), + ) _meta = ( @@ -236,7 +234,7 @@ CONFIG_REPO_DICT = cfgv.Map( MigrateShaToRev(), cfgv.WarnAdditionalKeys( - {'repo', 'rev', 'hooks'}, + ('repo', 'rev', 'hooks'), warn_on_unknown_keys_at_top_level, ), ) @@ -264,6 +262,10 @@ CONFIG_SCHEMA = cfgv.Map( cfgv.check_and(cfgv.check_string, check_min_version), '0', ), + cfgv.WarnAdditionalKeys( + ('repos',), + warn_on_unknown_keys_at_top_level, + ), ) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 069dca36..cace0f32 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -118,7 +118,7 @@ def test_validate_config_old_list_format_ok(tmpdir): assert not validate_config_main((f.strpath,)) -def test_validate_warn_on_unknown_keys_at_top_level(tmpdir, caplog): +def test_validate_warn_on_unknown_keys_at_repo_level(tmpdir, caplog): f = tmpdir.join('cfg.yaml') f.write( '- repo: https://gitlab.com/pycqa/flake8\n' @@ -133,8 +133,29 @@ def test_validate_warn_on_unknown_keys_at_top_level(tmpdir, caplog): ( 'pre_commit', logging.WARNING, - 'Your pre-commit-config contain these extra keys: args. ' - 'while the only valid keys are: hooks, repo, rev.', + 'Unexpected config key(s): args', + ), + ] + + +def test_validate_warn_on_unknown_keys_at_top_level(tmpdir, caplog): + f = tmpdir.join('cfg.yaml') + f.write( + 'repos:\n' + '- repo: https://gitlab.com/pycqa/flake8\n' + ' rev: 3.7.7\n' + ' hooks:\n' + ' - id: flake8\n' + 'foo:\n' + ' id: 1.0.0\n', + ) + ret_val = validate_config_main((f.strpath,)) + assert not ret_val + assert caplog.record_tuples == [ + ( + 'pre_commit', + logging.WARNING, + 'Unexpected config key(s): foo', ), ] From ba7760b705f9f0d31215aceab5c3c7bde1ee6447 Mon Sep 17 00:00:00 2001 From: Yoav Caspi Date: Sun, 12 May 2019 15:09:15 +0300 Subject: [PATCH 3/4] Add a test to validate that cfgv.WarnAdditionalKeys working as expected in the relevant config schemas --- pre_commit/clientlib.py | 11 ++++++++++- tests/clientlib_test.py | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 3285a48b..3ceefb1b 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -109,6 +109,8 @@ META = 'meta' class MigrateShaToRev(object): + key = 'rev' + @staticmethod def _cond(key): return cfgv.Conditional( @@ -263,7 +265,14 @@ CONFIG_SCHEMA = cfgv.Map( '0', ), cfgv.WarnAdditionalKeys( - ('repos',), + ( + 'repos', + 'default_language_version', + 'default_stages', + 'exclude', + 'fail_fast', + 'minimum_pre_commit_version', + ), warn_on_unknown_keys_at_top_level, ), ) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index cace0f32..13b42a59 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -305,3 +305,12 @@ def test_minimum_pre_commit_version_failing(): def test_minimum_pre_commit_version_passing(): cfg = {'repos': [], 'minimum_pre_commit_version': '0'} cfgv.validate(cfg, CONFIG_SCHEMA) + + +@pytest.mark.parametrize('schema', (CONFIG_SCHEMA, CONFIG_REPO_DICT)) +def test_warn_additional(schema): + allowed_keys = {item.key for item in schema.items if hasattr(item, 'key')} + warn_additional, = [ + x for x in schema.items if isinstance(x, cfgv.WarnAdditionalKeys) + ] + assert allowed_keys == set(warn_additional.keys) From 7a998a091e9160ce612cb21f1bd35fd0573ef05b Mon Sep 17 00:00:00 2001 From: Yoav Caspi Date: Sun, 12 May 2019 23:29:42 +0300 Subject: [PATCH 4/4] improve function name --- pre_commit/clientlib.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 3ceefb1b..c16a3ace 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -149,7 +149,7 @@ def _entry(modname): ) -def warn_on_unknown_keys_at_top_level(extra, orig_keys): +def warn_unknown_keys(extra, orig_keys): logger.warning( 'Unexpected config key(s): {}'.format( ', '.join(sorted(extra)), @@ -237,7 +237,7 @@ CONFIG_REPO_DICT = cfgv.Map( MigrateShaToRev(), cfgv.WarnAdditionalKeys( ('repo', 'rev', 'hooks'), - warn_on_unknown_keys_at_top_level, + warn_unknown_keys, ), ) DEFAULT_LANGUAGE_VERSION = cfgv.Map( @@ -273,7 +273,7 @@ CONFIG_SCHEMA = cfgv.Map( 'fail_fast', 'minimum_pre_commit_version', ), - warn_on_unknown_keys_at_top_level, + warn_unknown_keys, ), )