diff --git a/pre_commit/clientlib/validate_base.py b/pre_commit/clientlib/validate_base.py index 75a4cf96..dcf7f115 100644 --- a/pre_commit/clientlib/validate_base.py +++ b/pre_commit/clientlib/validate_base.py @@ -4,6 +4,8 @@ import jsonschema.exceptions import os.path import yaml +from pre_commit.jsonschema_extensions import apply_defaults + def get_validator( json_schema, @@ -39,6 +41,8 @@ def get_validator( 'File {0} is not a valid file'.format(filename), e, ) + obj = apply_defaults(obj, json_schema) + additional_validation_strategy(obj) return obj diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py index 0f783a6e..307b5633 100644 --- a/pre_commit/clientlib/validate_config.py +++ b/pre_commit/clientlib/validate_config.py @@ -28,9 +28,10 @@ CONFIG_JSON_SCHEMA = { 'properties': { 'id': {'type': 'string'}, 'files': {'type': 'string'}, + 'exclude': {'type': 'string', 'default': '^$'}, 'args': { 'type': 'array', - 'minItems': 1, + 'default': [], 'items': {'type': 'string'}, }, }, @@ -43,17 +44,22 @@ CONFIG_JSON_SCHEMA = { } +def try_regex(repo, hook, value, field_name): + try: + re.compile(value) + except re.error: + raise InvalidConfigError( + 'Invalid {0} regex at {1}, {2}: {3}'.format( + field_name, repo, hook, value, + ) + ) + + def validate_config_extra(config): for repo in config: for hook in repo['hooks']: - try: - re.compile(hook['files']) - except re.error: - raise InvalidConfigError( - 'Invalid file regex at {0}, {1}: {2}'.format( - repo['repo'], hook['id'], hook['files'], - ) - ) + try_regex(repo, hook['id'], hook['files'], 'files') + try_regex(repo, hook['id'], hook['exclude'], 'exclude') load_config = get_validator( diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py index 43200832..3f89cfae 100644 --- a/pre_commit/clientlib/validate_manifest.py +++ b/pre_commit/clientlib/validate_manifest.py @@ -20,10 +20,10 @@ MANIFEST_JSON_SCHEMA = { 'properties': { 'id': {'type': 'string'}, 'name': {'type': 'string'}, - 'description': {'type': 'string'}, + 'description': {'type': 'string', 'default': ''}, 'entry': {'type': 'string'}, 'language': {'type': 'string'}, - 'expected_return_value': {'type': 'number'}, + 'expected_return_value': {'type': 'number', 'default': 0}, }, 'required': ['id', 'name', 'entry', 'language'], }, @@ -32,11 +32,9 @@ MANIFEST_JSON_SCHEMA = { def additional_manifest_check(obj): for hook_config in obj: - language = hook_config.get('language') + language = hook_config['language'] - if language is not None and not any( - language.startswith(lang) for lang in all_languages - ): + if not any(language.startswith(lang) for lang in all_languages): raise InvalidManifestError( 'Expected language {0} for {1} to start with one of {2!r}'.format( hook_config['id'], diff --git a/pre_commit/git.py b/pre_commit/git.py index 7a50ead2..692dbc23 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -40,12 +40,16 @@ def get_all_files(): def get_files_matching(all_file_list_strategy): @functools.wraps(all_file_list_strategy) @memoize_by_cwd - def wrapper(expr): - regex = re.compile(expr) + def wrapper(include_expr, exclude_expr): + include_regex = re.compile(include_expr) + exclude_regex = re.compile(exclude_expr) return set(filter(os.path.exists, ( filename for filename in all_file_list_strategy() - if regex.search(filename) + if ( + include_regex.search(filename) and + not exclude_regex.search(filename) + ) ))) return wrapper diff --git a/pre_commit/jsonschema_extensions.py b/pre_commit/jsonschema_extensions.py new file mode 100644 index 00000000..938303c9 --- /dev/null +++ b/pre_commit/jsonschema_extensions.py @@ -0,0 +1,32 @@ + +import copy +import jsonschema +import jsonschema.validators + + +# From https://github.com/Julian/jsonschema/blob/master/docs/faq.rst +def extend_with_default(validator_class): + validate_properties = validator_class.VALIDATORS["properties"] + + def set_defaults(validator, properties, instance, schema): + for error in validate_properties( + validator, properties, instance, schema, + ): + yield error + + for property, subschema in properties.iteritems(): + if "default" in subschema: + instance.setdefault(property, subschema["default"]) + + return jsonschema.validators.extend( + validator_class, {"properties" : set_defaults}, + ) + + +DefaultingValidator = extend_with_default(jsonschema.Draft4Validator) + + +def apply_defaults(obj, schema): + obj = copy.deepcopy(obj) + DefaultingValidator(schema).validate(obj) + return obj diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index 4c05f3e7..fab8e5ae 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -2,7 +2,7 @@ def run_hook(env, hook, file_args): return env.run( - ' '.join(['xargs', hook['entry']] + hook.get('args', [])), + ' '.join(['xargs', hook['entry']] + hook['args']), stdin='\n'.join(list(file_args) + ['']), retcode=None, ) diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py index 783743ae..e622a7c8 100644 --- a/pre_commit/languages/script.py +++ b/pre_commit/languages/script.py @@ -6,7 +6,7 @@ def install_environment(repo_cmd_runner): def run_hook(repo_cmd_runner, hook, file_args): return repo_cmd_runner.run( - ['xargs', '{{prefix}}{0}'.format(hook['entry'])] + hook.get('args', []), + ['xargs', '{{prefix}}{0}'.format(hook['entry'])] + hook['args'], # TODO: this is duplicated in pre_commit/languages/helpers.py stdin='\n'.join(list(file_args) + ['']), retcode=None, diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py index 8439c091..39dcf89c 100644 --- a/pre_commit/languages/system.py +++ b/pre_commit/languages/system.py @@ -6,7 +6,7 @@ def install_environment(repo_cmd_runner): def run_hook(repo_cmd_runner, hook, file_args): return repo_cmd_runner.run( - ['xargs', hook['entry']] + hook.get('args', []), + ['xargs', hook['entry']] + hook['args'], # TODO: this is duplicated in pre_commit/languages/helpers.py stdin='\n'.join(list(file_args) + ['']), retcode=None, diff --git a/pre_commit/repository.py b/pre_commit/repository.py index f8f4d1c2..10ff2b6c 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -27,9 +27,7 @@ class Repository(object): @cached_property def languages(self): - return set(filter(None, ( - hook.get('language') for hook in self.hooks.values() - ))) + return set(hook['language'] for hook in self.hooks.values()) @cached_property def hooks(self): diff --git a/pre_commit/run.py b/pre_commit/run.py index 63247658..14974313 100644 --- a/pre_commit/run.py +++ b/pre_commit/run.py @@ -35,10 +35,10 @@ def _run_single_hook(runner, repository, hook_id, all_files=False): retcode, stdout, stderr = repository.run_hook( runner.cmd_runner, hook_id, - get_filenames(hook['files']), + get_filenames(hook['files'], hook['exclude']), ) - if retcode != repository.hooks[hook_id].get('expected_return_value', 0): + if retcode != repository.hooks[hook_id]['expected_return_value']: output = '\n'.join([stdout, stderr]).strip() retcode = 1 color = RED diff --git a/tests/clientlib/validate_config_test.py b/tests/clientlib/validate_config_test.py index bf80793f..f3ccc4df 100644 --- a/tests/clientlib/validate_config_test.py +++ b/tests/clientlib/validate_config_test.py @@ -7,6 +7,7 @@ from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import InvalidConfigError from pre_commit.clientlib.validate_config import run from pre_commit.clientlib.validate_config import validate_config_extra +from pre_commit.jsonschema_extensions import apply_defaults def test_returns_0_for_valid_config(): @@ -35,29 +36,40 @@ def is_valid_according_to_schema(obj, schema): [{ 'repo': 'git@github.com:pre-commit/pre-commit-hooks', 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '*.py', - } - ] + 'hooks': [{'id': 'pyflakes', 'files': '\.py$'}], }], True, ), ( [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '*.py', - 'args': ['foo', 'bar', 'baz'], - } - ] + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\.py$', + 'args': ['foo', 'bar', 'baz'], + }, + ], }], True, ), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\.py$', + # Exclude pattern must be a string + 'exclude': 0, + 'args': ['foo', 'bar', 'baz'], + }, + ], + }], + False, + ), )) def test_is_valid_according_to_schema(manifest_obj, expected): ret = is_valid_according_to_schema(manifest_obj, CONFIG_JSON_SCHEMA) @@ -67,12 +79,50 @@ def test_is_valid_according_to_schema(manifest_obj, expected): def test_config_with_failing_regexes_fails(): with pytest.raises(InvalidConfigError): # Note the regex '(' is invalid (unbalanced parens) - validate_config_extra( - [{'repo': 'foo', 'hooks': [{'id': 'hook_id', 'files': '('}]}] + config = apply_defaults( + [{ + 'repo': 'foo', + 'sha': 'foo', + 'hooks': [{'id': 'hook_id', 'files': '('}], + }], + CONFIG_JSON_SCHEMA, ) + validate_config_extra(config) def test_config_with_ok_regexes_passes(): - validate_config_extra( - [{'repo': 'foo', 'hooks': [{'id': 'hook_id', 'files': '\.py$'}]}] + config = apply_defaults( + [{ + 'repo': 'foo', + 'sha': 'foo', + 'hooks': [{'id': 'hook_id', 'files': '\.py$'}], + }], + CONFIG_JSON_SCHEMA, ) + validate_config_extra(config) + + +def test_config_with_invalid_exclude_regex_fails(): + with pytest.raises(InvalidConfigError): + # Note the regex '(' is invalid (unbalanced parens) + config = apply_defaults( + [{ + 'repo': 'foo', + 'sha': 'foo', + 'hooks': [{'id': 'hook_id', 'files': '', 'exclude': '('}], + }], + CONFIG_JSON_SCHEMA, + ) + validate_config_extra(config) + + +def test_config_with_ok_exclude_regex_passes(): + config = apply_defaults( + [{ + 'repo': 'foo', + 'sha': 'foo', + 'hooks': [{'id': 'hook_id', 'files': '', 'exclude': '^vendor/'}], + }], + CONFIG_JSON_SCHEMA, + ) + validate_config_extra(config) diff --git a/tests/clientlib/validate_manifest_test.py b/tests/clientlib/validate_manifest_test.py index 7d64db28..a1c10580 100644 --- a/tests/clientlib/validate_manifest_test.py +++ b/tests/clientlib/validate_manifest_test.py @@ -27,11 +27,10 @@ def test_additional_manifest_check_raises_for_bad_language(): @pytest.mark.parametrize(('obj'), ( - [{}], [{'language': 'python'}], [{'language': 'ruby'}], )) -def test_additional_manifest_check_is_ok_with_missing_language(obj): +def test_additional_manifest_check_languages(obj): additional_manifest_check(obj) diff --git a/tests/commands_test.py b/tests/commands_test.py index 6dba08d0..21740f2c 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -1,5 +1,4 @@ -import jsonschema import os import os.path import pkg_resources @@ -18,6 +17,7 @@ from pre_commit.commands import install from pre_commit.commands import RepositoryCannotBeUpdatedError from pre_commit.commands import uninstall from pre_commit.commands import _update_repository +from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.ordereddict import OrderedDict from pre_commit.runner import Runner from pre_commit.yaml_extensions import ordered_dump @@ -60,8 +60,9 @@ def up_to_date_repo(python_hooks_repo): ('sha', git.get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) + wrapped_config = apply_defaults([config], CONFIG_JSON_SCHEMA) + validate_config_extra(wrapped_config) + config = wrapped_config[0] with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: file_obj.write( @@ -96,8 +97,9 @@ def out_of_date_repo(python_hooks_repo): ('sha', git.get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) + config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) + validate_config_extra(config_wrapped) + config = config_wrapped[0] local['git']['commit', '--allow-empty', '-m', 'foo']() head_sha = git.get_head_sha(python_hooks_repo) @@ -135,8 +137,9 @@ def hook_disappearing_repo(python_hooks_repo): ('sha', git.get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) + config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) + validate_config_extra(config_wrapped) + config = config_wrapped[0] shutil.copy(get_resource_path('manifest_without_foo.yaml'), C.MANIFEST_FILE) local['git']['add', '.']() local['git']['commit', '-m', 'Remove foo']() diff --git a/tests/conftest.py b/tests/conftest.py index 70e3a9a1..feecc6da 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,5 @@ from __future__ import absolute_import -import jsonschema import pytest import time from plumbum import local @@ -8,6 +7,7 @@ from plumbum import local from pre_commit import git from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra +from pre_commit.jsonschema_extensions import apply_defaults from testing.util import copy_tree_to_path from testing.util import get_resource_path @@ -69,9 +69,9 @@ def _make_config(path, hook_id, file_regex): 'sha': git.get_head_sha(path), 'hooks': [{'id': hook_id, 'files': file_regex}], } - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) - return config + config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) + validate_config_extra(config_wrapped) + return config_wrapped[0] @pytest.yield_fixture diff --git a/tests/git_test.py b/tests/git_test.py index 17d60311..a6ecc574 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -29,7 +29,7 @@ def get_files_matching_func(): def test_get_files_matching_base(get_files_matching_func): - ret = get_files_matching_func('') + ret = get_files_matching_func('', '^$') assert ret == set([ 'pre_commit/run.py', 'pre_commit/git.py', @@ -38,7 +38,7 @@ def test_get_files_matching_base(get_files_matching_func): def test_get_files_matching_total_match(get_files_matching_func): - ret = get_files_matching_func('^.*\.py$') + ret = get_files_matching_func('^.*\.py$', '^$') assert ret == set([ 'pre_commit/run.py', 'pre_commit/git.py', @@ -46,10 +46,15 @@ def test_get_files_matching_total_match(get_files_matching_func): def test_does_search_instead_of_match(get_files_matching_func): - ret = get_files_matching_func('\.yaml$') + ret = get_files_matching_func('\.yaml$', '^$') assert ret == set(['hooks.yaml']) def test_does_not_include_deleted_fileS(get_files_matching_func): - ret = get_files_matching_func('exist.py') + ret = get_files_matching_func('exist.py', '^$') assert ret == set() + + +def test_exclude_removes_files(get_files_matching_func): + ret = get_files_matching_func('', '\.py$') + assert ret == set(['hooks.yaml']) diff --git a/tests/jsonschema_extensions_test.py b/tests/jsonschema_extensions_test.py new file mode 100644 index 00000000..4ea2686c --- /dev/null +++ b/tests/jsonschema_extensions_test.py @@ -0,0 +1,51 @@ + +from pre_commit.jsonschema_extensions import apply_defaults + + +def test_apply_defaults_copies_object(): + input = {} + ret = apply_defaults(input, {}) + assert ret is not input + + +def test_apply_default_does_not_touch_schema_without_defaults(): + ret = apply_defaults( + {'foo': 'bar'}, + {'type': 'object', 'properties': {'foo': {}, 'baz': {}}}, + ) + assert ret == {'foo': 'bar'} + + +def test_apply_defaults_applies_defaults(): + ret = apply_defaults( + {'foo': 'bar'}, + { + 'type': 'object', + 'properties': { + 'foo': {'default': 'biz'}, + 'baz': {'default': 'herp'}, + } + } + ) + assert ret == {'foo': 'bar', 'baz': 'herp'} + + +def test_apply_defaults_deep(): + ret = apply_defaults( + {'foo': {'bar': {}}}, + { + 'type': 'object', + 'properties': { + 'foo': { + 'type': 'object', + 'properties': { + 'bar': { + 'type': 'object', + 'properties': {'baz': {'default': 'herp'}}, + }, + }, + }, + }, + }, + ) + assert ret == {'foo': {'bar': {'baz': 'herp'}}} diff --git a/tests/repository_test.py b/tests/repository_test.py index 0b4011e7..2276b224 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -1,11 +1,11 @@ import os -import jsonschema import pytest import pre_commit.constants as C from pre_commit import git from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra +from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.repository import Repository @@ -111,9 +111,9 @@ def mock_repo_config(): 'files': '\.py$', }], } - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) - return config + config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) + validate_config_extra(config_wrapped) + return config_wrapped[0] def test_repo_url(mock_repo_config):