Remove defaults from pre-commit config schema. Resolves pre-commit#227

asottile · asottile · commit 039a7a587892 · 2015-05-18T13:59:42.000-07:00
diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py
@@ -33,7 +33,7 @@ class InvalidConfigError(FatalError):
                     'properties': {
                         'id': {'type': 'string'},
                         'files': {'type': 'string'},
-                        'exclude': {'type': 'string', 'default': '^$'},
+                        'exclude': {'type': 'string'},
                         'language_version': {'type': 'string'},
                         'args': {
                             'type': 'array',
@@ -71,7 +71,7 @@ def validate_config_extra(config):
             )
         for hook in repo['hooks']:
             try_regex(repo, hook['id'], hook.get('files', ''), 'files')
-            try_regex(repo, hook['id'], hook['exclude'], 'exclude')
+            try_regex(repo, hook['id'], hook.get('exclude', ''), 'exclude')
 
 
 load_config = get_validator(
diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py
@@ -20,6 +20,7 @@ class InvalidManifestError(ValueError):
             'name': {'type': 'string'},
             'description': {'type': 'string', 'default': ''},
             'entry': {'type': 'string'},
+            'exclude': {'type': 'string', 'default': '^$'},
             'language': {'type': 'string'},
             'language_version': {'type': 'string', 'default': 'default'},
             'files': {'type': 'string'},
@@ -52,8 +53,14 @@ def validate_files(hook_config):
     if not is_regex_valid(hook_config['files']):
         raise InvalidManifestError(
             'Invalid files regex at {0}: {1}'.format(
-                hook_config['id'],
-                hook_config['files'],
+                hook_config['id'], hook_config['files'],
+            )
+        )
+
+    if not is_regex_valid(hook_config.get('exclude', '')):
+        raise InvalidManifestError(
+            'Invalid exclude regex at {0}: {1}'.format(
+                hook_config['id'], hook_config['exclude'],
             )
         )
 
diff --git a/tests/clientlib/validate_config_test.py b/tests/clientlib/validate_config_test.py
@@ -174,3 +174,23 @@ def test_config_with_local_hooks_definition_passes(config_obj):
     jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA)
     config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA)
     validate_config_extra(config)
+
+
+def test_does_not_contain_defaults():
+    """Due to the way our merging works, if this schema has any defaults they
+    will clobber potentially useful values in the backing manifest. #227
+    """
+    to_process = [(CONFIG_JSON_SCHEMA, ())]
+    while to_process:
+        schema, route = to_process.pop()
+        # Check this value
+        if isinstance(schema, dict):
+            if 'default' in schema:
+                raise AssertionError(
+                    'Unexpected default in schema at {0}'.format(
+                        ' => '.join(route),
+                    )
+                )
+
+            for key, value in schema.items():
+                to_process.append((value, route + (key,)))
diff --git a/tests/clientlib/validate_manifest_test.py b/tests/clientlib/validate_manifest_test.py
@@ -46,6 +46,9 @@ def test_additional_manifest_check_passing(obj):
         [{'id': 'a', 'language': 'not a language', 'files': ''}],
         [{'id': 'a', 'language': 'python3', 'files': ''}],
         [{'id': 'a', 'language': 'python', 'files': 'invalid regex('}],
+        [{'id': 'a', 'language': 'not a language', 'files': ''}],
+        [{'id': 'a', 'language': 'python3', 'files': ''}],
+        [{'id': 'a', 'language': 'python', 'files': '', 'exclude': '('}],
     ),
 )
 def test_additional_manifest_failing(obj):
diff --git a/tests/manifest_test.py b/tests/manifest_test.py
@@ -22,6 +22,7 @@ def test_manifest_contents(manifest):
         'args': [],
         'description': '',
         'entry': 'bin/hook.sh',
+        'exclude': '^$',
         'expected_return_value': 0,
         'files': '',
         'id': 'bash_hook',
@@ -36,6 +37,7 @@ def test_hooks(manifest):
         'args': [],
         'description': '',
         'entry': 'bin/hook.sh',
+        'exclude': '^$',
         'expected_return_value': 0,
         'files': '',
         'id': 'bash_hook',
