Add --require-env to check-executables-have-shebangs

Author: chuckatkins

README.md

@@ -47,6 +47,10 @@ Check for files with names that would conflict on a case-insensitive filesystem
 
 #### `check-executables-have-shebangs`
 Checks that non-binary executables have a proper shebang.
+  - `--require-env` - Require shebangs to invoke an interpreter through
+    `/usr/bin/env` (e.g. `#!/usr/bin/env python`).
+  - `--fix` - Rewrite existing shebangs to use `/usr/bin/env`
+    (requires `--require-env`).
 
 #### `check-illegal-windows-names`
 Check for files that cannot be created on Windows.

pre_commit_hooks/check_executables_have_shebangs.py

@@ -14,18 +14,22 @@
 EXECUTABLE_VALUES = frozenset(('1', '3', '5', '7'))
 
 
-def check_executables(paths: list[str]) -> int:
+def check_executables(
+        paths: list[str], *, require_env: bool = False, fix: bool = False,
+) -> int:
     fs_tracks_executable_bit = cmd_output(
         'git', 'config', 'core.fileMode', retcode=None,
     ).strip()
     if fs_tracks_executable_bit == 'false':  # pragma: win32 cover
-        return _check_git_filemode(paths)
+        return _check_git_filemode(
+            paths, require_env=require_env, fix=fix,
+        )
     else:  # pragma: win32 no cover
         retv = 0
         for path in paths:
-            if not has_shebang(path):
-                _message(path)
-                retv = 1
+            retv |= _check_executable(
+                path, require_env=require_env, fix=fix,
+            )
 
         return retv
 
@@ -43,42 +47,128 @@ def git_ls_files(paths: Sequence[str]) -> Generator[GitLsFile]:
         yield GitLsFile(mode, filename)
 
 
-def _check_git_filemode(paths: Sequence[str]) -> int:
+def _check_git_filemode(
+        paths: Sequence[str], *, require_env: bool = False, fix: bool = False,
+) -> int:
     seen: set[str] = set()
     for ls_file in git_ls_files(paths):
         is_executable = any(b in EXECUTABLE_VALUES for b in ls_file.mode[-3:])
-        if is_executable and not has_shebang(ls_file.filename):
-            _message(ls_file.filename)
+        if is_executable and _check_executable(
+                ls_file.filename, require_env=require_env, fix=fix,
+        ):
             seen.add(ls_file.filename)
 
     return int(bool(seen))
 
 
-def has_shebang(path: str) -> int:
+def has_shebang(path: str, *, require_env: bool = False) -> bool:
     with open(path, 'rb') as f:
         first_bytes = f.read(2)
+        if first_bytes != b'#!':
+            return False
+        elif not require_env:
+            return True
+        else:
+            cmd = f.readline().split()
+
+    return (
+        len(cmd) >= 2 and
+        cmd[0] == b'/usr/bin/env'
+    )
 
-    return first_bytes == b'#!'
 
+def _fix_shebang(path: str) -> bool:
+    with open(path, 'rb+') as f:
+        first_line = f.readline()
+        if not first_line.startswith(b'#!'):
+            return False
+
+        line = first_line.rstrip(b'\r\n')
+        newline = first_line[len(line):]
+        command = line[2:].strip()
+        cmd = command.split(maxsplit=1)
+        if not cmd:
+            return False
+
+        executable = cmd[0].rsplit(b'/', 1)[-1]
+        if not executable or (executable == b'env' and len(cmd) == 1):
+            return False
+
+        if executable == b'env':
+            new_first_line = b'#!/usr/bin/env ' + cmd[1] + newline
+        elif len(cmd) == 2:
+            new_first_line = (
+                b'#!/usr/bin/env -S ' + executable + b' ' + cmd[1] + newline
+            )
+        else:
+            new_first_line = b'#!/usr/bin/env ' + executable + newline
+
+        rest = f.read()
+        f.seek(0)
+        f.write(new_first_line)
+        f.write(rest)
+        f.truncate()
+
+    return True
+
+
+def _check_executable(
+        path: str, *, require_env: bool, fix: bool,
+) -> int:
+    if has_shebang(path, require_env=require_env):
+        return 0
+    elif fix and _fix_shebang(path):
+        print(f'Fixing {path}')
+    else:
+        _message(path, require_env=require_env)
+
+    return 1
+
+
+def _message(path: str, *, require_env: bool = False) -> None:
+    if require_env:
+        problem = 'does not have a /usr/bin/env shebang'
+        suggestion = (
+            'use a /usr/bin/env shebang (e.g. `#!/usr/bin/env python`)'
+        )
+    else:
+        problem = 'has no (or invalid) shebang'
+        suggestion = 'double-check its shebang'
 
-def _message(path: str) -> None:
     print(
-        f'{path}: marked executable but has no (or invalid) shebang!\n'
+        f'{path}: marked executable but {problem}!\n'
         f"  If it isn't supposed to be executable, try: "
         f'`chmod -x {shlex.quote(path)}`\n'
         f'  If on Windows, you may also need to: '
         f'`git add --chmod=-x {shlex.quote(path)}`\n'
-        f'  If it is supposed to be executable, double-check its shebang.',
+        f'  If it is supposed to be executable, {suggestion}.',
         file=sys.stderr,
     )
 
 
 def main(argv: Sequence[str] | None = None) -> int:
     parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        '--require-env', action='store_true',
+        help='Require shebangs to invoke an interpreter through /usr/bin/env',
+    )
+    parser.add_argument(
+        '--fix', action='store_true',
+        help=(
+            'Rewrite existing shebangs to use /usr/bin/env '
+            '(requires --require-env)'
+        ),
+    )
     parser.add_argument('filenames', nargs='*')
     args = parser.parse_args(argv)
+    if args.fix and not args.require_env:
+        parser.error('--fix requires --require-env')
 
-    return check_executables(args.filenames)
+    return check_executables(
+        args.filenames,
+        require_env=args.require_env,
+        fix=args.fix,
+    )
 
 
 if __name__ == '__main__':

tests/check_executables_have_shebangs_test.py

@@ -30,6 +30,64 @@ def test_has_shebang(content, tmpdir):
     assert main((str(path),)) == 0
 
 
+@skip_win32  # pragma: win32 no cover
+@pytest.mark.parametrize(
+    ('content', 'expected'), (
+        (b'#!/usr/bin/env python\n', 0),
+        (b'#!/usr/bin/env -S python -O\n', 0),
+        (b'#!/bin/env bash\n', 1),
+        (b'#!/bin/bash\n', 1),
+        (b'#!/usr/bin/env\n', 1),
+    ),
+)
+def test_require_env_shebang(content, expected, tmpdir):
+    path = tmpdir.join('path')
+    path.write(content, 'wb')
+    assert main(('--require-env', str(path))) == expected
+
+
+@skip_win32  # pragma: win32 no cover
+@pytest.mark.parametrize(
+    ('content', 'expected'), (
+        (b'#!/bin/bash\necho hi\n', b'#!/usr/bin/env bash\necho hi\n'),
+        (
+            b'#!/opt/bin/python -O\nprint("hi")\n',
+            b'#!/usr/bin/env -S python -O\nprint("hi")\n',
+        ),
+        (b'#!/bin/env bash\necho hi\n', b'#!/usr/bin/env bash\necho hi\n'),
+    ),
+)
+def test_require_env_fix(content, expected, tmpdir, capsys):
+    path = tmpdir.join('path')
+    path.write(content, 'wb')
+
+    assert main(('--require-env', '--fix', str(path))) == 1
+    stdout, stderr = capsys.readouterr()
+    assert stdout == f'Fixing {path}\n'
+    assert stderr == ''
+    assert path.read('rb') == expected
+
+    assert main(('--require-env', '--fix', str(path))) == 0
+
+
+@skip_win32  # pragma: win32 no cover
+@pytest.mark.parametrize(
+    'content', (b'echo hi\n', b'#!\n', b'#!/\n', b'#!/bin/env\n'),
+)
+def test_require_env_fix_cannot_infer_interpreter(content, tmpdir):
+    path = tmpdir.join('path')
+    path.write(content, 'wb')
+
+    assert main(('--require-env', '--fix', str(path))) == 1
+    assert path.read('rb') == content
+
+
+def test_fix_requires_require_env():
+    with pytest.raises(SystemExit) as excinfo:
+        main(('--fix',))
+    assert excinfo.value.code == 2
+
+
 @skip_win32  # pragma: win32 no cover
 @pytest.mark.parametrize(
     'content', (
@@ -104,6 +162,23 @@ def test_check_git_filemode_failing(tmpdir):
         assert check_executables_have_shebangs._check_git_filemode(files) == 1
 
 
+def test_check_git_filemode_require_env(tmpdir):
+    with tmpdir.as_cwd():
+        cmd_output('git', 'init', '.')
+
+        f = tmpdir.join('f')
+        f.write('#!/bin/bash')
+        f_path = str(f)
+        cmd_output('git', 'add', f_path)
+        cmd_output('git', 'update-index', '--chmod=+x', f_path)
+
+        files = (f_path,)
+        assert check_executables_have_shebangs._check_git_filemode(files) == 0
+        assert check_executables_have_shebangs._check_git_filemode(
+            files, require_env=True,
+        ) == 1
+
+
 @pytest.mark.parametrize(
     ('content', 'mode', 'expected'),
     (