Repository: pre-commit/pre-commit-hooks
Pull request #207 (2017-06-25 21:46:29)

Fix bug with the file-contents-sorter hook when processing file that …
…does not end in a newline

Discovered that when the file-contents-sorter gets a file with contents like: `b\na` (note that there is no newline at the end) it would rearrange them to result in `ab\n` which just looks like 'ab' on one line and is no-good. 

This branch fixes that by:

1. Stripping the line endings off each line from the start
2. Do the sorting
3. Add \n to the end of each line (including the last one)

Rather than comparing the str-ified before vs after to determine if the hook made changes, it compares the list-of-stripped-lines before vs after so this hook won't add newlines to the end of files if they do not actually require sorting. 

Moar test cases. 

Also, 
```
after_string = b'\n'.join(after) + b'\n'
f.write(after_string)
```
could just be:
`f.writelines("%s\n" % l for l in after)`
Except this repo runs tests in py34. The above works in 2.7 and py3.5 added bytestr formatting like this back in. Anyway, just including this rationale in case you wonder why I didn't just use writelines(). 




Patch:
diff --git a/README.md b/README.md
index 92fb408..6e268ff 100644
--- a/README.md
+++ b/README.md
@@ -51,7 +51,7 @@ Add this to your `.pre-commit-config.yaml`
   with single quoted strings.
 - `end-of-file-fixer` - Makes sure files end in a newline and only a newline.
 - `fix-encoding-pragma` - Add `# -*- coding: utf-8 -*-` to the top of python files.
-- `file-contents-sorter` - Sort the lines in specified files (defaults to alphabetical). You must provide list of target files as input to it.
+- `file-contents-sorter` - Sort the lines in specified files (defaults to alphabetical). You must provide list of target files as input to it. Note that this hook WILL remove blank lines and does NOT respect any comments.
     - To remove the coding pragma pass `--remove` (useful in a python3-only codebase)
 - `flake8` - Run flake8 on your python files.
 - `forbid-new-submodules` - Prevent addition of new git submodules.
diff --git a/pre_commit_hooks/file_contents_sorter.py b/pre_commit_hooks/file_contents_sorter.py
index 6fa3bc0..fe7f7ee 100644
--- a/pre_commit_hooks/file_contents_sorter.py
+++ b/pre_commit_hooks/file_contents_sorter.py
@@ -18,11 +18,11 @@ FAIL = 1
 
 
 def sort_file_contents(f):
-    before = tuple(f)
-    after = sorted(before)
+    before = list(f)
+    after = sorted([line.strip(b'\n\r') for line in before if line.strip()])
 
     before_string = b''.join(before)
-    after_string = b''.join(after)
+    after_string = b'\n'.join(after) + b'\n'
 
     if before_string == after_string:
         return PASS


Test patch:
diff --git a/tests/file_contents_sorter_test.py b/tests/file_contents_sorter_test.py
index 5f4dc5b..2c85c8a 100644
--- a/tests/file_contents_sorter_test.py
+++ b/tests/file_contents_sorter_test.py
@@ -8,15 +8,19 @@ from pre_commit_hooks.file_contents_sorter import PASS
 @pytest.mark.parametrize(
     ('input_s', 'expected_retval', 'output'),
     (
-        (b'', PASS, b''),
+        (b'', FAIL, b'\n'),
         (b'lonesome\n', PASS, b'lonesome\n'),
-        (b'missing_newline', PASS, b'missing_newline'),
+        (b'missing_newline', FAIL, b'missing_newline\n'),
+        (b'newline\nmissing', FAIL, b'missing\nnewline\n'),
+        (b'missing\nnewline', FAIL, b'missing\nnewline\n'),
         (b'alpha\nbeta\n', PASS, b'alpha\nbeta\n'),
         (b'beta\nalpha\n', FAIL, b'alpha\nbeta\n'),
         (b'C\nc\n', PASS, b'C\nc\n'),
         (b'c\nC\n', FAIL, b'C\nc\n'),
         (b'mag ical \n tre vor\n', FAIL, b' tre vor\nmag ical \n'),
         (b'@\n-\n_\n#\n', FAIL, b'#\n-\n@\n_\n'),
+        (b'extra\n\n\nwhitespace\n', FAIL, b'extra\nwhitespace\n'),
+        (b'whitespace\n\n\nextra\n', FAIL, b'extra\nwhitespace\n'),
     )
 )
 def test_integration(input_s, expected_retval, output, tmpdir):