Merge pull request #59 from pre-commit/fix_update_adding_defaults

Fix autoupdate spamming with defaults.

pre_commit/commands.py

@@ -8,7 +8,9 @@ import stat
 from plumbum import local
 
 import pre_commit.constants as C
+from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
 from pre_commit.clientlib.validate_config import load_config
+from pre_commit.jsonschema_extensions import remove_defaults
 from pre_commit.ordereddict import OrderedDict
 from pre_commit.repository import Repository
 from pre_commit.yaml_extensions import ordered_dump
@@ -75,7 +77,7 @@ def _update_repository(repo_config):
             '{0}'.format(', '.join(sorted(hooks_missing)))
         )
 
-    return new_config
+    return remove_defaults([new_config], CONFIG_JSON_SCHEMA)[0]
 
 
 def autoupdate(runner):

pre_commit/jsonschema_extensions.py

@@ -5,30 +5,55 @@ import jsonschema.validators
 
 
 # From https://github.com/Julian/jsonschema/blob/master/docs/faq.rst
-def extend_with_default(validator_class):
+def extend_validator_cls(validator_cls, modify):
-    validate_properties = validator_class.VALIDATORS["properties"]
+    validate_properties = validator_cls.VALIDATORS['properties']
 
-    def set_defaults(validator, properties, instance, schema):
+    def new_properties(validator, properties, instance, schema):
         for error in validate_properties(
             validator, properties, instance, schema,
         ):
             yield error
 
-        for property, subschema in properties.iteritems():
+        modify(properties, instance)
-            if "default" in subschema:
-                instance.setdefault(
-                    property, copy.deepcopy(subschema["default"]),
-                )
 
     return jsonschema.validators.extend(
-        validator_class, {"properties" : set_defaults},
+        validator_cls, {'properties': new_properties},
     )
 
 
-DefaultingValidator = extend_with_default(jsonschema.Draft4Validator)
+
+def default_values(properties, instance):
+    for property, subschema in properties.iteritems():
+        if 'default' in subschema:
+            instance.setdefault(
+                property, copy.deepcopy(subschema['default']),
+            )
+
+
+def remove_default_values(properties, instance):
+    for property, subschema in properties.iteritems():
+        if (
+            'default' in subschema and
+            instance.get(property) == subschema['default']
+        ):
+            del instance[property]
+
+
+_AddDefaultsValidator = extend_validator_cls(
+    jsonschema.Draft4Validator, default_values,
+)
+_RemoveDefaultsValidator = extend_validator_cls(
+    jsonschema.Draft4Validator, remove_default_values,
+)
 
 
 def apply_defaults(obj, schema):
     obj = copy.deepcopy(obj)
-    DefaultingValidator(schema).validate(obj)
+    _AddDefaultsValidator(schema).validate(obj)
+    return obj
+
+
+def remove_defaults(obj, schema):
+    obj = copy.deepcopy(obj)
+    _RemoveDefaultsValidator(schema).validate(obj)
     return obj

tests/commands_test.py

@@ -120,6 +120,12 @@ def test_out_of_date_repo(out_of_date_repo):
     assert ret['sha'] == out_of_date_repo.head_sha
 
 
+def test_removes_defaults(out_of_date_repo):
+    ret = _update_repository(out_of_date_repo.repo_config)
+    assert 'args' not in ret['hooks'][0]
+    assert 'expected_return_value' not in ret['hooks'][0]
+
+
 def test_autoupdate_out_of_date_repo(out_of_date_repo):
     before = open(C.CONFIG_FILE).read()
     runner = Runner(out_of_date_repo.python_hooks_repo)

tests/jsonschema_extensions_test.py

@@ -1,5 +1,6 @@
 
 from pre_commit.jsonschema_extensions import apply_defaults
+from pre_commit.jsonschema_extensions import remove_defaults
 
 
 def test_apply_defaults_copies_object():
@@ -56,3 +57,25 @@ def test_apply_defaults_copies():
     ret1 = apply_defaults({}, schema)
     ret2 = apply_defaults({}, schema)
     assert ret1['foo'] is not ret2['foo']
+
+
+def test_remove_defaults_copies_object():
+    input = {}
+    ret = remove_defaults(input, {})
+    assert ret is not input
+
+
+def test_remove_defaults_does_not_remove_non_default():
+    ret = remove_defaults(
+        {'foo': 'bar'},
+        {'properties': {'foo': {'default': 'baz'}}},
+    )
+    assert ret == {'foo': 'bar'}
+
+
+def test_remove_defaults_removes_default():
+    ret = remove_defaults(
+        {'foo': 'bar'},
+        {'properties': {'foo': {'default': 'bar'}}},
+    )
+    assert ret == {}