Merge pull request #484 from mxr/flake8-comprehensions
Fix C407
diff --git a/pre_commit_hooks/check_executables_have_shebangs.py b/pre_commit_hooks/check_executables_have_shebangs.py
index c34c7b7..1c50ea0 100644
--- a/pre_commit_hooks/check_executables_have_shebangs.py
+++ b/pre_commit_hooks/check_executables_have_shebangs.py
@@ -2,26 +2,60 @@
import argparse
import shlex
import sys
+from typing import List
from typing import Optional
from typing import Sequence
+from typing import Set
+
+from pre_commit_hooks.util import cmd_output
+
+EXECUTABLE_VALUES = frozenset(('1', '3', '5', '7'))
-def check_has_shebang(path: str) -> int:
+def check_executables(paths: List[str]) -> int:
+ if sys.platform == 'win32': # pragma: win32 cover
+ return _check_git_filemode(paths)
+ else: # pragma: win32 no cover
+ retv = 0
+ for path in paths:
+ if not _check_has_shebang(path):
+ _message(path)
+ retv = 1
+
+ return retv
+
+
+def _check_git_filemode(paths: Sequence[str]) -> int:
+ outs = cmd_output('git', 'ls-files', '--stage', '--', *paths)
+ seen: Set[str] = set()
+ for out in outs.splitlines():
+ metadata, path = out.split('\t')
+ tagmode = metadata.split(' ', 1)[0]
+
+ is_executable = any(b in EXECUTABLE_VALUES for b in tagmode[-3:])
+ has_shebang = _check_has_shebang(path)
+ if is_executable and not has_shebang:
+ _message(path)
+ seen.add(path)
+
+ return int(bool(seen))
+
+
+def _check_has_shebang(path: str) -> int:
with open(path, 'rb') as f:
first_bytes = f.read(2)
- if first_bytes != b'#!':
- quoted = shlex.quote(path)
- print(
- f'{path}: marked executable but has no (or invalid) shebang!\n'
- f" If it isn't supposed to be executable, try: "
- f'`chmod -x {quoted}`\n'
- f' If it is supposed to be executable, double-check its shebang.',
- file=sys.stderr,
- )
- return 1
- else:
- return 0
+ return first_bytes == b'#!'
+
+
+def _message(path: str) -> None:
+ print(
+ f'{path}: marked executable but has no (or invalid) shebang!\n'
+ f" If it isn't supposed to be executable, try: "
+ f'`chmod -x {shlex.quote(path)}`\n'
+ f' If it is supposed to be executable, double-check its shebang.',
+ file=sys.stderr,
+ )
def main(argv: Optional[Sequence[str]] = None) -> int:
@@ -29,12 +63,7 @@
parser.add_argument('filenames', nargs='*')
args = parser.parse_args(argv)
- retv = 0
-
- for filename in args.filenames:
- retv |= check_has_shebang(filename)
-
- return retv
+ return check_executables(args.filenames)
if __name__ == '__main__':
diff --git a/pre_commit_hooks/requirements_txt_fixer.py b/pre_commit_hooks/requirements_txt_fixer.py
index 6190692..78103a1 100644
--- a/pre_commit_hooks/requirements_txt_fixer.py
+++ b/pre_commit_hooks/requirements_txt_fixer.py
@@ -1,4 +1,5 @@
import argparse
+import re
from typing import IO
from typing import List
from typing import Optional
@@ -10,6 +11,9 @@
class Requirement:
+ UNTIL_COMPARISON = re.compile(b'={2,3}|!=|~=|>=?|<=?')
+ UNTIL_SEP = re.compile(rb'[^;\s]+')
+
def __init__(self) -> None:
self.value: Optional[bytes] = None
self.comments: List[bytes] = []
@@ -17,11 +21,20 @@
@property
def name(self) -> bytes:
assert self.value is not None, self.value
+ name = self.value.lower()
for egg in (b'#egg=', b'&egg='):
if egg in self.value:
- return self.value.lower().partition(egg)[-1]
+ return name.partition(egg)[-1]
- return self.value.lower().partition(b'==')[0]
+ m = self.UNTIL_SEP.match(name)
+ assert m is not None
+
+ name = m.group()
+ m = self.UNTIL_COMPARISON.search(name)
+ if not m:
+ return name
+
+ return name[:m.start()]
def __lt__(self, requirement: 'Requirement') -> int:
# \n means top of file comment, so always return True,
diff --git a/tests/check_executables_have_shebangs_test.py b/tests/check_executables_have_shebangs_test.py
index 15f0c79..3b77612 100644
--- a/tests/check_executables_have_shebangs_test.py
+++ b/tests/check_executables_have_shebangs_test.py
@@ -1,8 +1,19 @@
+import os
+import sys
+
import pytest
+from pre_commit_hooks import check_executables_have_shebangs
from pre_commit_hooks.check_executables_have_shebangs import main
+from pre_commit_hooks.util import cmd_output
+
+skip_win32 = pytest.mark.skipif(
+ sys.platform == 'win32',
+ reason="non-git checks aren't relevant on windows",
+)
+@skip_win32 # pragma: win32 no cover
@pytest.mark.parametrize(
'content', (
b'#!/bin/bash\nhello world\n',
@@ -17,6 +28,7 @@
assert main((path.strpath,)) == 0
+@skip_win32 # pragma: win32 no cover
@pytest.mark.parametrize(
'content', (
b'',
@@ -24,7 +36,6 @@
b'\n#!python\n',
b'python\n',
'☃'.encode(),
-
),
)
def test_bad_shebang(content, tmpdir, capsys):
@@ -33,3 +44,67 @@
assert main((path.strpath,)) == 1
_, stderr = capsys.readouterr()
assert stderr.startswith(f'{path}: marked executable but')
+
+
+def test_check_git_filemode_passing(tmpdir):
+ with tmpdir.as_cwd():
+ cmd_output('git', 'init', '.')
+
+ f = tmpdir.join('f')
+ f.write('#!/usr/bin/env bash')
+ f_path = str(f)
+ cmd_output('chmod', '+x', f_path)
+ cmd_output('git', 'add', f_path)
+ cmd_output('git', 'update-index', '--chmod=+x', f_path)
+
+ g = tmpdir.join('g').ensure()
+ g_path = str(g)
+ cmd_output('git', 'add', g_path)
+
+ # this is potentially a problem, but not something the script intends
+ # to check for -- we're only making sure that things that are
+ # executable have shebangs
+ h = tmpdir.join('h')
+ h.write('#!/usr/bin/env bash')
+ h_path = str(h)
+ cmd_output('git', 'add', h_path)
+
+ files = (f_path, g_path, h_path)
+ assert check_executables_have_shebangs._check_git_filemode(files) == 0
+
+
+def test_check_git_filemode_failing(tmpdir):
+ with tmpdir.as_cwd():
+ cmd_output('git', 'init', '.')
+
+ f = tmpdir.join('f').ensure()
+ f_path = str(f)
+ cmd_output('chmod', '+x', f_path)
+ 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) == 1
+
+
+@pytest.mark.parametrize(
+ ('content', 'mode', 'expected'),
+ (
+ pytest.param('#!python', '+x', 0, id='shebang with executable'),
+ pytest.param('#!python', '-x', 0, id='shebang without executable'),
+ pytest.param('', '+x', 1, id='no shebang with executable'),
+ pytest.param('', '-x', 0, id='no shebang without executable'),
+ ),
+)
+def test_git_executable_shebang(temp_git_dir, content, mode, expected):
+ with temp_git_dir.as_cwd():
+ path = temp_git_dir.join('path')
+ path.write(content)
+ cmd_output('git', 'add', str(path))
+ cmd_output('chmod', mode, str(path))
+ cmd_output('git', 'update-index', f'--chmod={mode}', str(path))
+
+ # simulate how identify choses that something is executable
+ filenames = [path for path in [str(path)] if os.access(path, os.X_OK)]
+
+ assert main(filenames) == expected
diff --git a/tests/requirements_txt_fixer_test.py b/tests/requirements_txt_fixer_test.py
index 17a9a41..fae5a72 100644
--- a/tests/requirements_txt_fixer_test.py
+++ b/tests/requirements_txt_fixer_test.py
@@ -33,9 +33,28 @@
(b'\nfoo\nbar\n', FAIL, b'bar\n\nfoo\n'),
(b'\nbar\nfoo\n', PASS, b'\nbar\nfoo\n'),
(
- b'pyramid==1\npyramid-foo==2\n',
- PASS,
- b'pyramid==1\npyramid-foo==2\n',
+ b'pyramid-foo==1\npyramid>=2\n',
+ FAIL,
+ b'pyramid>=2\npyramid-foo==1\n',
+ ),
+ (
+ b'a==1\n'
+ b'c>=1\n'
+ b'bbbb!=1\n'
+ b'c-a>=1;python_version>="3.6"\n'
+ b'e>=2\n'
+ b'd>2\n'
+ b'g<2\n'
+ b'f<=2\n',
+ FAIL,
+ b'a==1\n'
+ b'bbbb!=1\n'
+ b'c>=1\n'
+ b'c-a>=1;python_version>="3.6"\n'
+ b'd>2\n'
+ b'e>=2\n'
+ b'f<=2\n'
+ b'g<2\n',
),
(b'ocflib\nDjango\nPyMySQL\n', FAIL, b'Django\nocflib\nPyMySQL\n'),
(
