Make hooks specify files. Optionally allow config to override manifest.

asottile · asottile · commit 96174deac671 · 2014-06-05T18:39:34.000-07:00
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,5 +1,5 @@
 -   repo: git@github.com:pre-commit/pre-commit-hooks
-    sha: 59f23b7c556ce1cf66eb6dc574e10d2b4274be4b
+    sha: 7c003425b35fff516c0ee88f4040c8c208d474bd
     hooks:
     -   id: trailing-whitespace
         files: \.(js|rb|md|py|sh|txt|yaml|yml)$
diff --git a/example_hooks.yaml b/example_hooks.yaml
@@ -15,12 +15,13 @@
 -   id: my_hook
     name: My Simple Hook
     description: This is my simple hook that does blah
-    entry: my-simple-hook.py
+    entry: my-simple-hook
     language: python
-    expected_return_value: 0
+    files: \.py$
 -   id: my_grep_based_hook
     name: My Bash Based Hook
     description: This is a hook that uses grep to validate some stuff
     entry: ./my_grep_based_hook.sh
     language: script
+    files: \.(py|sh)$
     expected_return_value: 1
diff --git a/example_pre-commit-config.yaml b/example_pre-commit-config.yaml
@@ -2,8 +2,5 @@
     sha: cd74dc150c142c3be70b24eaf0b02cae9d235f37
     hooks:
         -   id: pyflakes
-            files: '\.py$'
         -   id: jslint
-            files: '\.js$'
         -   id: trim_trailing_whitespace
-            files: '\.py$'
diff --git a/hooks.yaml b/hooks.yaml
@@ -1,10 +1,12 @@
--   id: validate_manifest
-    name: Validate Pre-Commit Manifest
-    description: This validator validates a pre-commit hooks manifest file
-    entry: validate-manifest
-    language: python
 -   id: validate_config
     name: Validate Pre-Commit Config
     description: This validator validates a pre-commit hooks config file
     entry: validate-config
     language: python
+    files: ^\.pre-commit-config.yaml$
+-   id: validate_manifest
+    name: Validate Pre-Commit Manifest
+    description: This validator validates a pre-commit hooks manifest file
+    entry: validate-manifest
+    language: python
+    files: ^hooks.yaml$
diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py
@@ -32,7 +32,7 @@ class InvalidConfigError(ValueError):
                             'items': {'type': 'string'},
                         },
                     },
-                    'required': ['id', 'files'],
+                    'required': ['id'],
                 }
             }
         },
@@ -55,7 +55,7 @@ def try_regex(repo, hook, value, field_name):
 def validate_config_extra(config):
     for repo in config:
         for hook in repo['hooks']:
-            try_regex(repo, hook['id'], hook['files'], 'files')
+            try_regex(repo, hook['id'], hook.get('files', ''), 'files')
             try_regex(repo, hook['id'], hook['exclude'], 'exclude')
 
 
diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py
@@ -21,9 +21,10 @@ class InvalidManifestError(ValueError):
             'entry': {'type': 'string'},
             'language': {'type': 'string'},
             'language_version': {'type': 'string', 'default': 'default'},
+            'files': {'type': 'string'},
             'expected_return_value': {'type': 'number', 'default': 0},
         },
-        'required': ['id', 'name', 'entry', 'language'],
+        'required': ['id', 'name', 'entry', 'language', 'files'],
     },
 }
 
diff --git a/pre_commit/repository.py b/pre_commit/repository.py
@@ -38,7 +38,7 @@ def languages(self):
     def hooks(self):
         # TODO: merging in manifest dicts is a smell imo
         return OrderedDict(
-            (hook['id'], dict(hook, **self.manifest.hooks[hook['id']]))
+            (hook['id'], dict(self.manifest.hooks[hook['id']], **hook))
             for hook in self.repo_config['hooks']
         )
 
diff --git a/testing/resources/consumer_repo/.pre-commit-config.yaml b/testing/resources/consumer_repo/.pre-commit-config.yaml
@@ -2,19 +2,12 @@
     sha: bec87f6c87284ea15dbcf7801810404c8036bab4
     hooks:
     -   id: pyflakes
-        files: \.py$
     -   id: debug-statements
-        files: \.py$
     -   id: trailing-whitespace
-        files: \.(py|sh|yaml)$
     -   id: name-tests-test
-        files: tests/.+\.py$
     -   id: end-of-file-fixer
-        files: \.(py|sh|yaml)$
 -   repo: git@github.com:pre-commit/pre-commit
     sha: c62c1a3b513ab9e057e85a5e950bd7c438371076
     hooks:
     -   id: validate_manifest
-        files: ^hooks.yaml$
     -   id: validate_config
-        files: ^\.pre-commit-config.yaml$
diff --git a/testing/resources/failing_hook_repo/hooks.yaml b/testing/resources/failing_hook_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Failing hook
     entry: bin/hook.sh
     language: script
+    files: .
diff --git a/testing/resources/manifest_without_foo.yaml b/testing/resources/manifest_without_foo.yaml
@@ -2,3 +2,4 @@
     name: Bar
     entry: bar
     language: python
+    files: \.py$
diff --git a/testing/resources/node_0_11_8_hooks_repo/hooks.yaml b/testing/resources/node_0_11_8_hooks_repo/hooks.yaml
@@ -3,3 +3,4 @@
     entry: node-11-8-hook
     language: node
     language_version: 0.11.8
+    files: \.js$
diff --git a/testing/resources/node_hooks_repo/hooks.yaml b/testing/resources/node_hooks_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Foo
     entry: foo
     language: node
+    files: \.js$
diff --git a/testing/resources/prints_cwd_repo/hooks.yaml b/testing/resources/prints_cwd_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Prints Cwd
     entry: pwd
     language: system
+    files: \.sh$
diff --git a/testing/resources/python3_hooks_repo/hooks.yaml b/testing/resources/python3_hooks_repo/hooks.yaml
@@ -3,3 +3,4 @@
     entry: python3-hook
     language: python
     language_version: python3.3
+    files: \.py$
diff --git a/testing/resources/python_hooks_repo/hooks.yaml b/testing/resources/python_hooks_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Foo
     entry: foo
     language: python
+    files: \.py$
diff --git a/testing/resources/ruby_1_9_3_p547_hooks_repo/hooks.yaml b/testing/resources/ruby_1_9_3_p547_hooks_repo/hooks.yaml
@@ -3,3 +3,4 @@
     entry: ruby_hook
     language: ruby
     language_version: 1.9.3-p547
+    files: \.rb$
diff --git a/testing/resources/ruby_hooks_repo/hooks.yaml b/testing/resources/ruby_hooks_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Ruby Hook
     entry: ruby_hook
     language: ruby
+    files: \.rb$
diff --git a/testing/resources/script_hooks_repo/hooks.yaml b/testing/resources/script_hooks_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: Bash hook
     entry: bin/hook.sh
     language: script
+    files: ''
diff --git a/testing/resources/system_hook_with_spaces_repo/hooks.yaml b/testing/resources/system_hook_with_spaces_repo/hooks.yaml
@@ -2,3 +2,4 @@
     name: System hook with spaces
     entry: /usr/bin/python -c 'import sys; print("Hello World")'
     language: system
+    files: \.sh$
diff --git a/testing/resources/valid_yaml_but_invalid_config.yaml b/testing/resources/valid_yaml_but_invalid_config.yaml
@@ -3,4 +3,3 @@
         -   id: pyflakes
         -   id: jslint
         -   id: trim_trailing_whitespace
-            files: '*.py'
diff --git a/tests/clientlib/validate_manifest_test.py b/tests/clientlib/validate_manifest_test.py
@@ -46,20 +46,34 @@ def test_additional_manifest_check_languages_failing(obj):
         additional_manifest_check(obj)
 
 
-@pytest.mark.parametrize(('manifest_obj', 'expected'), (
-    ([], False),
-    ([{'id': 'a', 'name': 'b', 'entry': 'c', 'language': 'python'}], True),
+@pytest.mark.parametrize(
+    ('manifest_obj', 'expected'),
     (
-        [{
-            'id': 'a',
-            'name': 'b',
-            'entry': 'c',
-            'language': 'python',
-            'expected_return_value': 0,
-        }],
-        True,
-    ),
-))
+        ([], False),
+        (
+            [{
+                'id': 'a',
+                'name': 'b',
+                'entry': 'c',
+                'language': 'python',
+                'files': r'\.py$'
+            }],
+            True,
+        ),
+        (
+            [{
+                'id': 'a',
+                'name': 'b',
+                'entry': 'c',
+                'language': 'python',
+                'language_version': 'python3.3',
+                'files': r'\.py$',
+                'expected_return_value': 0,
+            }],
+            True,
+        ),
+    )
+)
 def test_is_valid_according_to_schema(manifest_obj, expected):
     ret = is_valid_according_to_schema(manifest_obj, MANIFEST_JSON_SCHEMA)
     assert ret is expected
diff --git a/tests/commands_test.py b/tests/commands_test.py
@@ -58,7 +58,7 @@ def up_to_date_repo(python_hooks_repo):
     config = OrderedDict((
         ('repo', python_hooks_repo),
         ('sha', get_head_sha(python_hooks_repo)),
-        ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]),
+        ('hooks', [OrderedDict((('id', 'foo'),))]),
     ))
     wrapped_config = apply_defaults([config], CONFIG_JSON_SCHEMA)
     validate_config_extra(wrapped_config)
@@ -147,7 +147,7 @@ def hook_disappearing_repo(python_hooks_repo):
     config = OrderedDict((
         ('repo', python_hooks_repo),
         ('sha', get_head_sha(python_hooks_repo)),
-        ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]),
+        ('hooks', [OrderedDict((('id', 'foo'),))]),
     ))
     config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA)
     validate_config_extra(config_wrapped)
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -122,11 +122,11 @@ def system_hook_with_spaces_repo(dummy_git_repo):
     yield _make_repo(dummy_git_repo, 'system_hook_with_spaces_repo')
 
 
-def _make_config(path, hook_id, file_regex):
+def _make_config(path, hook_id):
     config = {
         'repo': path,
         'sha': get_head_sha(path),
-        'hooks': [{'id': hook_id, 'files': file_regex}],
+        'hooks': [{'id': hook_id}],
     }
     config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA)
     validate_config_extra(config_wrapped)
@@ -135,48 +135,48 @@ def _make_config(path, hook_id, file_regex):
 
 @pytest.yield_fixture
 def config_for_node_hooks_repo(node_hooks_repo):
-    yield _make_config(node_hooks_repo, 'foo', '\\.js$')
+    yield _make_config(node_hooks_repo, 'foo')
 
 
 @pytest.yield_fixture
 def config_for_node_0_11_8_hooks_repo(node_0_11_8_hooks_repo):
-    yield _make_config(node_0_11_8_hooks_repo, 'node-11-8-hook', '\\.js$')
+    yield _make_config(node_0_11_8_hooks_repo, 'node-11-8-hook')
 
 
 @pytest.yield_fixture
 def config_for_ruby_hooks_repo(ruby_hooks_repo):
-    yield _make_config(ruby_hooks_repo, 'ruby_hook', '\\.rb$')
+    yield _make_config(ruby_hooks_repo, 'ruby_hook')
 
 
 @pytest.yield_fixture
 def config_for_ruby_1_9_3_p547_hooks_repo(ruby_1_9_3_p547_hooks_repo):
-    yield _make_config(ruby_1_9_3_p547_hooks_repo, 'ruby_hook', '\\.rb$')
+    yield _make_config(ruby_1_9_3_p547_hooks_repo, 'ruby_hook')
 
 
 @pytest.yield_fixture
 def config_for_python_hooks_repo(python_hooks_repo):
-    yield _make_config(python_hooks_repo, 'foo', '\\.py$')
+    yield _make_config(python_hooks_repo, 'foo')
 
 
 @pytest.yield_fixture
 def config_for_python3_hooks_repo(python3_hooks_repo):
-    yield _make_config(python3_hooks_repo, 'python3-hook', '\\.py$')
+    yield _make_config(python3_hooks_repo, 'python3-hook')
 
 
 @pytest.yield_fixture
 def config_for_prints_cwd_repo(prints_cwd_repo):
-    yield _make_config(prints_cwd_repo, 'prints_cwd', '^$')
+    yield _make_config(prints_cwd_repo, 'prints_cwd')
 
 
 @pytest.yield_fixture
 def config_for_script_hooks_repo(script_hooks_repo):
-    yield _make_config(script_hooks_repo, 'bash_hook', '')
+    yield _make_config(script_hooks_repo, 'bash_hook')
 
 
 @pytest.yield_fixture
 def config_for_system_hook_with_spaces(system_hook_with_spaces_repo):
     yield _make_config(
-        system_hook_with_spaces_repo, 'system-hook-with-spaces', '',
+        system_hook_with_spaces_repo, 'system-hook-with-spaces',
     )
 
 
@@ -198,7 +198,7 @@ def repo_with_passing_hook(config_for_script_hooks_repo, empty_git_dir):
 
 @pytest.yield_fixture
 def repo_with_failing_hook(failing_hook_repo, empty_git_dir):
-    _make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook', ''))
+    _make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook'))
     yield empty_git_dir
 
 
diff --git a/tests/manifest_test.py b/tests/manifest_test.py
@@ -17,6 +17,7 @@ def test_manifest_contents(manifest):
         'description': '',
         'entry': 'bin/hook.sh',
         'expected_return_value': 0,
+        'files': '',
         'id': 'bash_hook',
         'language': 'script',
         'language_version': 'default',
@@ -29,6 +30,7 @@ def test_hooks(manifest):
         'description': '',
         'entry': 'bin/hook.sh',
         'expected_return_value': 0,
+        'files': '',
         'id': 'bash_hook',
         'language': 'script',
         'language_version': 'default',
diff --git a/tests/repository_test.py b/tests/repository_test.py
@@ -32,6 +32,7 @@ def test_run_versioned_hook(config_for_python3_hooks_repo, store):
     assert ret[1] == "3.3\n['/dev/null']\nHello World\n"
 
 
+@skipif_slowtests_false
 @pytest.mark.integration
 def test_run_versioned_node_hook(config_for_node_0_11_8_hooks_repo, store):
     repo = Repository.create(config_for_node_0_11_8_hooks_repo, store)
@@ -152,3 +153,13 @@ def test_really_long_file_paths(config_for_python_hooks_repo, store):
     with local.cwd(path):
         repo = Repository.create(config_for_python_hooks_repo, store)
         repo.require_installed()
+
+
+@pytest.mark.integration
+def test_config_overrides_repo_specifics(config_for_script_hooks_repo, store):
+    repo = Repository.create(config_for_script_hooks_repo, store)
+    assert repo.hooks['bash_hook']['files'] == ''
+    # Set the file regex to something else
+    config_for_script_hooks_repo['hooks'][0]['files'] = '\\.sh$'
+    repo = Repository.create(config_for_script_hooks_repo, store)
+    assert repo.hooks['bash_hook']['files'] == '\\.sh$'

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ def languages(self):`
`38`	`38`	`def hooks(self):`
`39`	`39`	`# TODO: merging in manifest dicts is a smell imo`
`40`	`40`	`return OrderedDict(`
`41`		`- (hook['id'], dict(hook, **self.manifest.hooks[hook['id']]))`
	`41`	`+ (hook['id'], dict(self.manifest.hooks[hook['id']], **hook))`
`42`	`42`	`for hook in self.repo_config['hooks']`
`43`	`43`	`)`
`44`	`44`