Simplify mixed-line-ending hook

pre_commit_hooks/mixed_line_ending.py

@@ -1,212 +1,83 @@
+from __future__ import absolute_import
+from __future__ import print_function
+from __future__ import unicode_literals
+
 import argparse
-import re
+import collections
-import sys
-
-from enum import Enum
 
 
-class LineEnding(Enum):
+CRLF = b'\r\n'
-    CR = b'\r', 'cr', re.compile(b'\r(?!\n)', re.DOTALL)
+LF = b'\n'
-    CRLF = b'\r\n', 'crlf', re.compile(b'\r\n', re.DOTALL)
+CR = b'\r'
-    LF = b'\n', 'lf', re.compile(b'(?<!\r)\n', re.DOTALL)
+# Prefer LF to CRLF to CR, but detect CRLF before LF
-
+ALL_ENDINGS = (CR, CRLF, LF)
-    def __init__(self, string, opt_name, regex):
+FIX_TO_LINE_ENDING = {'cr': CR, 'crlf': CRLF, 'lf': LF}
-        self.string = string
-        self.str_print = repr(string)
-        self.opt_name = opt_name
-        self.regex = regex
 
 
-class MixedLineEndingOption(Enum):
+def _fix(filename, contents, ending):
-    AUTO = 'auto', None
+    new_contents = b''.join(
-    NO = 'no', None
+        line.rstrip(b'\r\n') + ending for line in contents.splitlines(True)
-    CRLF = LineEnding.CRLF.opt_name, LineEnding.CRLF
+    )
-    LF = LineEnding.LF.opt_name, LineEnding.LF
+    with open(filename, 'wb') as f:
-
+        f.write(new_contents)
-    def __init__(self, opt_name, line_ending_enum):
-        self.opt_name = opt_name
-        self.line_ending_enum = line_ending_enum
 
 
-class MixedLineDetection(Enum):
+def fix_filename(filename, fix):
-    NOT_MIXED = 1, False, None
+    with open(filename, 'rb') as f:
-    UNKNOWN = 2, False, None
+        contents = f.read()
-    MIXED_MOSTLY_CRLF = 3, True, LineEnding.CRLF
-    MIXED_MOSTLY_LF = 4, True, LineEnding.LF
-    MIXED_MOSTLY_CR = 5, True, LineEnding.CR
 
-    def __init__(self, index, mle_found, line_ending_enum):
+    counts = collections.defaultdict(int)
-        # TODO hack to prevent enum overriding
-        self.index = index
-        self.mle_found = mle_found
-        self.line_ending_enum = line_ending_enum
 
+    for line in contents.splitlines(True):
+        for ending in ALL_ENDINGS:
+            if line.endswith(ending):
+                counts[ending] += 1
+                break
 
-ANY_LINE_ENDING_PATTERN = re.compile(
+    # Some amount of mixed line endings
-    b'(' + LineEnding.CRLF.regex.pattern +
+    mixed = sum(bool(x) for x in counts.values()) > 1
-    b'|' + LineEnding.LF.regex.pattern +
-    b'|' + LineEnding.CR.regex.pattern + b')',
-)
 
+    if fix == 'no' or (fix == 'auto' and not mixed):
+        return mixed
 
-def mixed_line_ending(argv=None):
+    if fix == 'auto':
-    options = _parse_arguments(argv)
+        max_ending = LF
+        max_lines = 0
+        # ordering is important here such that lf > crlf > cr
+        for ending_type in ALL_ENDINGS:
+            # also important, using >= to find a max that prefers the last
+            if counts[ending_type] >= max_lines:
+                max_ending = ending_type
+                max_lines = counts[ending_type]
 
-    filenames = options['filenames']
+        _fix(filename, contents, max_ending)
-    fix_option = options['fix']
+        return 1
-
-    if fix_option == MixedLineEndingOption.NO:
-        return _process_no_fix(filenames)
-    elif fix_option == MixedLineEndingOption.AUTO:
-        return _process_fix_auto(filenames)
-    # when a line ending character is forced with --fix option
     else:
-        return _process_fix_force(filenames, fix_option.line_ending_enum)
+        target_ending = FIX_TO_LINE_ENDING[fix]
+        # find if there are lines with *other* endings
+        del counts[target_ending]
+        other_endings = bool(sum(counts.values()))
+        if other_endings:
+            _fix(filename, contents, target_ending)
+        return other_endings
 
 
-def _parse_arguments(argv=None):
+def main(argv=None):
     parser = argparse.ArgumentParser()
     parser.add_argument(
-        '-f',
+        '-f', '--fix',
-        '--fix',
+        choices=('auto', 'no') + tuple(FIX_TO_LINE_ENDING),
-        choices=[m.opt_name for m in MixedLineEndingOption],
+        default='auto',
-        default=MixedLineEndingOption.AUTO.opt_name,
         help='Replace line ending with the specified. Default is "auto"',
     )
     parser.add_argument('filenames', nargs='*', help='Filenames to fix')
     args = parser.parse_args(argv)
 
-    fix, = (
+    retv = 0
-        member for name, member
+    for filename in args.filenames:
-        in MixedLineEndingOption.__members__.items()
+        retv |= fix_filename(filename, args.fix)
-        if member.opt_name == args.fix
+    return retv
-    )
-
-    options = {
-        'fix': fix, 'filenames': args.filenames,
-    }
-
-    return options
-
-
-def _detect_line_ending(filename):
-    with open(filename, 'rb') as f:
-        buf = f.read()
-
-        le_counts = {}
-
-    for le_enum in LineEnding:
-        le_counts[le_enum] = len(le_enum.regex.findall(buf))
-
-    mixed = False
-    le_found_previously = False
-    most_le = None
-    max_le_count = 0
-
-    for le, le_count in le_counts.items():
-        le_found_cur = le_count > 0
-
-        mixed |= le_found_previously and le_found_cur
-        le_found_previously |= le_found_cur
-
-        if le_count == max_le_count:
-            most_le = None
-        elif le_count > max_le_count:
-            max_le_count = le_count
-            most_le = le
-
-    if not mixed:
-        return MixedLineDetection.NOT_MIXED
-
-    for mld in MixedLineDetection:
-        if (
-                mld.line_ending_enum is not None and
-                mld.line_ending_enum == most_le
-        ):
-            return mld
-
-    return MixedLineDetection.UNKNOWN
-
-
-def _process_no_fix(filenames):
-    print('Checking if the files have mixed line ending.')
-
-    mle_filenames = []
-    for filename in filenames:
-        detect_result = _detect_line_ending(filename)
-
-        if detect_result.mle_found:
-            mle_filenames.append(filename)
-
-    mle_found = len(mle_filenames) > 0
-
-    if mle_found:
-        print(
-            'The following files have mixed line endings:\n\t%s',
-            '\n\t'.join(mle_filenames),
-        )
-
-    return 1 if mle_found else 0
-
-
-def _process_fix_auto(filenames):
-    mle_found = False
-
-    for filename in filenames:
-        detect_result = _detect_line_ending(filename)
-
-        if detect_result == MixedLineDetection.NOT_MIXED:
-            print('The file %s has no mixed line ending', filename)
-        elif detect_result == MixedLineDetection.UNKNOWN:
-            print(
-                'Could not define most frequent line ending in '
-                'file %s. File skiped.', filename,
-            )
-
-            mle_found = True
-        else:
-            le_enum = detect_result.line_ending_enum
-
-            print(
-                'The file %s has mixed line ending with a '
-                'majority of %s. Converting...', filename, le_enum.str_print,
-            )
-
-            _convert_line_ending(filename, le_enum.string)
-            mle_found = True
-
-            print(
-                'The file %s has been converted to %s line ending.',
-                filename, le_enum.str_print,
-            )
-
-    return 1 if mle_found else 0
-
-
-def _process_fix_force(filenames, line_ending_enum):
-    for filename in filenames:
-        _convert_line_ending(filename, line_ending_enum.string)
-
-        print(
-            'The file %s has been forced to %s line ending.',
-            filename, line_ending_enum.str_print,
-        )
-
-    return 1
-
-
-def _convert_line_ending(filename, line_ending):
-    with open(filename, 'rb+') as f:
-        bufin = f.read()
-
-        # convert line ending
-        bufout = ANY_LINE_ENDING_PATTERN.sub(line_ending, bufin)
-
-        # write the result in the file replacing the existing content
-        f.seek(0)
-        f.write(bufout)
-        f.truncate()
 
 
 if __name__ == '__main__':
-    sys.exit(mixed_line_ending())
+    exit(main())

setup.py

@@ -31,7 +31,6 @@ setup(
         'simplejson',
         'six',
     ],
-    extras_require={':python_version=="2.7"': ['enum34']},
     entry_points={
         'console_scripts': [
             'autopep8-wrapper = pre_commit_hooks.autopep8_wrapper:main',

tests/mixed_line_ending_test.py

@@ -1,154 +1,103 @@
+from __future__ import absolute_import
+from __future__ import unicode_literals
+
 import pytest
 
-from pre_commit_hooks.mixed_line_ending import mixed_line_ending
+from pre_commit_hooks.mixed_line_ending import main
 
-# Input, expected return value, expected output
+
-TESTS_FIX_AUTO = (
+@pytest.mark.parametrize(
-    # only 'LF'
+    ('input_s', 'output'),
-    (b'foo\nbar\nbaz\n', 0, b'foo\nbar\nbaz\n'),
+    (
-    # only 'CRLF'
-    (b'foo\r\nbar\r\nbaz\r\n', 0, b'foo\r\nbar\r\nbaz\r\n'),
-    # only 'CR'
-    (b'foo\rbar\rbaz\r', 0, b'foo\rbar\rbaz\r'),
         # mixed with majority of 'LF'
-    (b'foo\r\nbar\nbaz\n', 1, b'foo\nbar\nbaz\n'),
+        (b'foo\r\nbar\nbaz\n', b'foo\nbar\nbaz\n'),
         # mixed with majority of 'CRLF'
-    (b'foo\r\nbar\nbaz\r\n', 1, b'foo\r\nbar\r\nbaz\r\n'),
+        (b'foo\r\nbar\nbaz\r\n', b'foo\r\nbar\r\nbaz\r\n'),
         # mixed with majority of 'CR'
-    (b'foo\rbar\nbaz\r', 1, b'foo\rbar\rbaz\r'),
+        (b'foo\rbar\nbaz\r', b'foo\rbar\rbaz\r'),
         # mixed with as much 'LF' as 'CRLF'
-    (b'foo\r\nbar\nbaz', 1, b'foo\r\nbar\nbaz'),
+        (b'foo\r\nbar\n', b'foo\nbar\n'),
         # mixed with as much 'LF' as 'CR'
-    (b'foo\rbar\nbaz', 1, b'foo\rbar\nbaz'),
+        (b'foo\rbar\n', b'foo\nbar\n'),
         # mixed with as much 'CRLF' as 'CR'
-    (b'foo\r\nbar\nbaz', 1, b'foo\r\nbar\nbaz'),
+        (b'foo\r\nbar\r', b'foo\r\nbar\r\n'),
         # mixed with as much 'CRLF' as 'LF' as 'CR'
-    (b'foo\r\nbar\nbaz\r', 1, b'foo\r\nbar\nbaz\r'),
+        (b'foo\r\nbar\nbaz\r', b'foo\nbar\nbaz\n'),
+    ),
 )
-
+def test_mixed_line_ending_fixes_auto(input_s, output, tmpdir):
-
-@pytest.mark.parametrize(
-    ('input_s', 'expected_retval', 'output'),
-    TESTS_FIX_AUTO,
-)
-def test_mixed_line_ending_fix_auto(input_s, expected_retval, output, tmpdir):
     path = tmpdir.join('file.txt')
     path.write_binary(input_s)
-    ret = mixed_line_ending(('--fix=auto', path.strpath))
+    ret = main((path.strpath,))
 
-    assert ret == expected_retval
+    assert ret == 1
     assert path.read_binary() == output
 
 
-# Input, expected return value, expected output
+def test_non_mixed_no_newline_end_of_file(tmpdir):
-TESTS_NO_FIX = (
+    path = tmpdir.join('f.txt')
-    # only 'LF'
+    path.write_binary(b'foo\nbar\nbaz')
-    (b'foo\nbar\nbaz\n', 0, b'foo\nbar\nbaz\n'),
+    assert not main((path.strpath,))
-    # only 'CRLF'
+    # the hook *could* fix the end of the file, but leaves it alone
-    (b'foo\r\nbar\r\nbaz\r\n', 0, b'foo\r\nbar\r\nbaz\r\n'),
+    # this is mostly to document the current behaviour
-    # only 'CR'
+    assert path.read_binary() == b'foo\nbar\nbaz'
-    (b'foo\rbar\rbaz\r', 0, b'foo\rbar\rbaz\r'),
+
-    # mixed with majority of 'LF'
+
-    (b'foo\r\nbar\nbaz\n', 1, b'foo\r\nbar\nbaz\n'),
+def test_mixed_no_newline_end_of_file(tmpdir):
-    # mixed with majority of 'CRLF'
+    path = tmpdir.join('f.txt')
-    (b'foo\r\nbar\nbaz\r\n', 1, b'foo\r\nbar\nbaz\r\n'),
+    path.write_binary(b'foo\r\nbar\nbaz')
-    # mixed with majority of 'CR'
+    assert main((path.strpath,))
-    (b'foo\rbar\nbaz\r', 1, b'foo\rbar\nbaz\r'),
+    # the hook rewrites the end of the file, this is slightly inconsistent
-    # mixed with as much 'LF' as 'CR'
+    # with the non-mixed case but I think this is the better behaviour
-    (b'foo\rbar\nbaz', 0, b'foo\rbar\nbaz'),
+    # this is mostly to document the current behaviour
-    # mixed with as much 'CRLF' as 'CR'
+    assert path.read_binary() == b'foo\nbar\nbaz\n'
-    (b'foo\r\nbar\nbaz', 0, b'foo\r\nbar\nbaz'),
-    # mixed with as much 'CRLF' as 'LF' as 'CR'
-    (b'foo\r\nbar\nbaz\r', 0, b'foo\r\nbar\nbaz\r'),
-)
 
 
 @pytest.mark.parametrize(
-    ('input_s', 'expected_retval', 'output'),
+    ('fix_option', 'input_s'),
-    TESTS_NO_FIX,
+    (
+        # All --fix=auto with uniform line endings should be ok
+        ('--fix=auto', b'foo\r\nbar\r\nbaz\r\n'),
+        ('--fix=auto', b'foo\rbar\rbaz\r'),
+        ('--fix=auto', b'foo\nbar\nbaz\n'),
+        # --fix=crlf with crlf endings
+        ('--fix=crlf', b'foo\r\nbar\r\nbaz\r\n'),
+        # --fix=lf with lf endings
+        ('--fix=lf', b'foo\nbar\nbaz\n'),
+    ),
 )
-def test_detect_mixed_line_ending(input_s, expected_retval, output, tmpdir):
+def test_line_endings_ok(fix_option, input_s, tmpdir):
-    path = tmpdir.join('file.txt')
+    path = tmpdir.join('input.txt')
     path.write_binary(input_s)
-    ret = mixed_line_ending(('--fix=no', path.strpath))
+    ret = main((fix_option, path.strpath))
 
-    assert ret == expected_retval
+    assert ret == 0
-    assert path.read_binary() == output
+    assert path.read_binary() == input_s
 
 
-# Input, expected return value, expected output
+def test_no_fix_does_not_modify(tmpdir):
-TESTS_FIX_FORCE_LF = (
+    path = tmpdir.join('input.txt')
-    # only 'LF'
+    contents = b'foo\r\nbar\rbaz\nwomp\n'
-    (b'foo\nbar\nbaz\n', 1, b'foo\nbar\nbaz\n'),
+    path.write_binary(contents)
-    # only 'CRLF'
+    ret = main(('--fix=no', path.strpath))
-    (b'foo\r\nbar\r\nbaz\r\n', 1, b'foo\nbar\nbaz\n'),
+
-    # only 'CR'
+    assert ret == 1
-    (b'foo\rbar\rbaz\r', 1, b'foo\nbar\nbaz\n'),
+    assert path.read_binary() == contents
-    # mixed with majority of 'LF'
-    (b'foo\r\nbar\nbaz\n', 1, b'foo\nbar\nbaz\n'),
-    # mixed with majority of 'CRLF'
-    (b'foo\r\nbar\nbaz\r\n', 1, b'foo\nbar\nbaz\n'),
-    # mixed with majority of 'CR'
-    (b'foo\rbar\nbaz\r', 1, b'foo\nbar\nbaz\n'),
-    # mixed with as much 'LF' as 'CR'
-    (b'foo\rbar\nbaz', 1, b'foo\nbar\nbaz'),
-    # mixed with as much 'CRLF' as 'CR'
-    (b'foo\r\nbar\nbaz', 1, b'foo\nbar\nbaz'),
-    # mixed with as much 'CRLF' as 'LF' as 'CR'
-    (b'foo\r\nbar\nbaz\r', 1, b'foo\nbar\nbaz\n'),
-)
 
 
-@pytest.mark.parametrize(
+def test_fix_lf(tmpdir):
-    ('input_s', 'expected_retval', 'output'),
+    path = tmpdir.join('input.txt')
-    TESTS_FIX_FORCE_LF,
+    path.write_binary(b'foo\r\nbar\rbaz\n')
-)
+    ret = main(('--fix=lf', path.strpath))
-def test_mixed_line_ending_fix_force_lf(
-    input_s, expected_retval, output,
-    tmpdir,
-):
-    path = tmpdir.join('file.txt')
-    path.write_binary(input_s)
-    ret = mixed_line_ending(('--fix=lf', path.strpath))
 
-    assert ret == expected_retval
+    assert ret == 1
-    assert path.read_binary() == output
+    assert path.read_binary() == b'foo\nbar\nbaz\n'
 
 
-# Input, expected return value, expected output
+def test_fix_crlf(tmpdir):
-TESTS_FIX_FORCE_CRLF = (
+    path = tmpdir.join('input.txt')
-    # only 'LF'
+    path.write_binary(b'foo\r\nbar\rbaz\n')
-    (b'foo\nbar\nbaz\n', 1, b'foo\r\nbar\r\nbaz\r\n'),
+    ret = main(('--fix=crlf', path.strpath))
-    # only 'CRLF'
-    (b'foo\r\nbar\r\nbaz\r\n', 1, b'foo\r\nbar\r\nbaz\r\n'),
-    # only 'CR'
-    (b'foo\rbar\rbaz\r', 1, b'foo\r\nbar\r\nbaz\r\n'),
-    # mixed with majority of 'LF'
-    (b'foo\r\nbar\nbaz\n', 1, b'foo\r\nbar\r\nbaz\r\n'),
-    # mixed with majority of 'CRLF'
-    (b'foo\r\nbar\nbaz\r\n', 1, b'foo\r\nbar\r\nbaz\r\n'),
-    # mixed with majority of 'CR'
-    (b'foo\rbar\nbaz\r', 1, b'foo\r\nbar\r\nbaz\r\n'),
-    # mixed with as much 'LF' as 'CR'
-    (b'foo\rbar\nbaz', 1, b'foo\r\nbar\r\nbaz'),
-    # mixed with as much 'CRLF' as 'CR'
-    (b'foo\r\nbar\nbaz', 1, b'foo\r\nbar\r\nbaz'),
-    # mixed with as much 'CRLF' as 'LF' as 'CR'
-    (b'foo\r\nbar\nbaz\r', 1, b'foo\r\nbar\r\nbaz\r\n'),
-)
 
-
+    assert ret == 1
-@pytest.mark.parametrize(
+    assert path.read_binary() == b'foo\r\nbar\r\nbaz\r\n'
-    ('input_s', 'expected_retval', 'output'),
-    TESTS_FIX_FORCE_CRLF,
-)
-def test_mixed_line_ending_fix_force_crlf(
-    input_s, expected_retval, output,
-    tmpdir,
-):
-    path = tmpdir.join('file.txt')
-    path.write_binary(input_s)
-    ret = mixed_line_ending(('--fix=crlf', path.strpath))
-
-    assert ret == expected_retval
-    assert path.read_binary() == output