forbid-new-submodules: fix triggering failure when only a submodule i…

…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609

.pre-commit-hooks.yaml

@@ -154,6 +154,7 @@
     language: python
     entry: forbid-new-submodules
     description: Prevent addition of new git submodules
+    types: [directory]
 -   id: mixed-line-ending
     name: Mixed line ending
     description: Replaces or checks mixed line ending

pre_commit_hooks/forbid_new_submodules.py

@@ -1,14 +1,32 @@
+import argparse
+import os
 from typing import Optional
 from typing import Sequence
 
 from pre_commit_hooks.util import cmd_output
 
 
 def main(argv: Optional[Sequence[str]] = None) -> int:
-    # `argv` is ignored, pre-commit will send us a list of files that we
-    # don't care about
+    parser = argparse.ArgumentParser()
+    parser.add_argument('filenames', nargs='*')
+    args = parser.parse_args(argv)
+
+    if not args.filenames:
+        return 0
+    if (
+        'PRE_COMMIT_FROM_REF' in os.environ and
+        'PRE_COMMIT_TO_REF' in os.environ
+    ):
+        diff_args: Sequence[str] = (
+            '--merge-base',
+            os.environ['PRE_COMMIT_FROM_REF'],
+            os.environ['PRE_COMMIT_TO_REF'],
+        )
+    else:
+        diff_args = ('--staged',)
     added_diff = cmd_output(
-        'git', 'diff', '--staged', '--diff-filter=A', '--raw',
+        'git', 'diff', '--diff-filter=A', '--raw', *diff_args, '--',
+        *args.filenames,
     )
     retv = 0
     for line in added_diff.splitlines():

tests/forbid_new_submodules_test.py

@@ -1,4 +1,6 @@
+import os
 import subprocess
+import unittest.mock
 
 import pytest
 
@@ -31,12 +33,32 @@ def git_dir_with_git_dir(tmpdir):
 )
 def test_main_new_submodule(git_dir_with_git_dir, capsys, cmd):
     subprocess.check_call(cmd)
-    assert main() == 1
+    assert main(()) == 0
+    assert main(('random_non-related_file',)) == 0
+    assert main(('foo',)) == 1
+    out, _ = capsys.readouterr()
+    assert out.startswith('foo: new submodule introduced\n')
+
+
+def test_main_new_submodule_committed(git_dir_with_git_dir, capsys):
+    REV_PARSE_HEAD = ('git', 'rev-parse', 'HEAD')
+    FROM = subprocess.check_output(REV_PARSE_HEAD).decode().strip()
+    subprocess.check_call(('git', 'submodule', 'add', './foo'))
+    subprocess.check_call(('git', 'commit', '-m', 'new submodule'))
+    TO = subprocess.check_output(REV_PARSE_HEAD).decode().strip()
+    with unittest.mock.patch.dict(
+        os.environ,
+        {'PRE_COMMIT_FROM_REF': FROM, 'PRE_COMMIT_TO_REF': TO},
+    ):
+        assert main(()) == 0
+        assert main(('random_non-related_file',)) == 0
+        assert main(('foo',)) == 1
     out, _ = capsys.readouterr()
     assert out.startswith('foo: new submodule introduced\n')
 
 
 def test_main_no_new_submodule(git_dir_with_git_dir):
     open('test.py', 'a+').close()
     subprocess.check_call(('git', 'add', 'test.py'))
-    assert main() == 0
+    assert main(()) == 0
+    assert main(('test.py',)) == 0