From a17514d55eb220b21085a72e2a066c21490826de Mon Sep 17 00:00:00 2001 From: Enji Cooper Date: Wed, 18 Dec 2019 17:25:07 -0800 Subject: [PATCH] Add `--check` support to EOF and Trailing Whitespace fixers This change adds an advisory mode via `--check` that only warns of formatting issues with files, but does not address them. This support is desirable because--while I don't mind the automagic changes when done in a mechanical way--some individuals who I described the current behavior of these fixers to were a bit uneasy about the magic that went along with them. Adding `--check` so others can opt out (similar to `black --check`) is a compromise on this front. Signed-off-by: Enji Cooper --- pre_commit_hooks/end_of_file_fixer.py | 29 ++++++++++++------ pre_commit_hooks/trailing_whitespace_fixer.py | 30 ++++++++++++------- tests/end_of_file_fixer_test.py | 12 ++++++++ tests/trailing_whitespace_fixer_test.py | 14 +++++++++ 4 files changed, 65 insertions(+), 20 deletions(-) diff --git a/pre_commit_hooks/end_of_file_fixer.py b/pre_commit_hooks/end_of_file_fixer.py index 4e77c94..8b0e19b 100644 --- a/pre_commit_hooks/end_of_file_fixer.py +++ b/pre_commit_hooks/end_of_file_fixer.py @@ -9,7 +9,7 @@ from typing import Optional from typing import Sequence -def fix_file(file_obj): # type: (IO[bytes]) -> int +def fix_file(file_obj, apply_fixes=True): # type: (IO[bytes], bool) -> int # Test for newline at end of file # Empty files will throw IOError here try: @@ -20,8 +20,9 @@ def fix_file(file_obj): # type: (IO[bytes]) -> int # last_character will be '' for an empty file if last_character not in {b'\n', b'\r'} and last_character != b'': # Needs this seek for windows, otherwise IOError - file_obj.seek(0, os.SEEK_END) - file_obj.write(b'\n') + if apply_fixes: + file_obj.seek(0, os.SEEK_END) + file_obj.write(b'\n') return 1 while last_character in {b'\n', b'\r'}: @@ -29,8 +30,9 @@ def fix_file(file_obj): # type: (IO[bytes]) -> int if file_obj.tell() == 1: # If we've reached the beginning of the file and it is all # linebreaks then we can make this file empty - file_obj.seek(0) - file_obj.truncate() + if apply_fixes: + file_obj.seek(0) + file_obj.truncate() return 1 # Go back two bytes and read a character @@ -45,8 +47,9 @@ def fix_file(file_obj): # type: (IO[bytes]) -> int if remaining == sequence: return 0 elif remaining.startswith(sequence): - file_obj.seek(position + len(sequence)) - file_obj.truncate() + if apply_fixes: + file_obj.seek(position + len(sequence)) + file_obj.truncate() return 1 return 0 @@ -54,6 +57,11 @@ def fix_file(file_obj): # type: (IO[bytes]) -> int def main(argv=None): # type: (Optional[Sequence[str]]) -> int parser = argparse.ArgumentParser() + parser.add_argument( + '--check', action='store_true', + help="Don't write the files back. Returns a non-zero code if changes " + 'would be applied. Returns zero if no changes are required.', + ) parser.add_argument('filenames', nargs='*', help='Filenames to fix') args = parser.parse_args(argv) @@ -62,9 +70,12 @@ def main(argv=None): # type: (Optional[Sequence[str]]) -> int for filename in args.filenames: # Read as binary so we can read byte-by-byte with open(filename, 'rb+') as file_obj: - ret_for_file = fix_file(file_obj) + ret_for_file = fix_file(file_obj, apply_fixes=not args.check) if ret_for_file: - print('Fixing {}'.format(filename)) + if args.check: + print('Would fix {}'.format(filename)) + else: + print('Fixing {}'.format(filename)) retv |= ret_for_file return retv diff --git a/pre_commit_hooks/trailing_whitespace_fixer.py b/pre_commit_hooks/trailing_whitespace_fixer.py index a21b54f..ac43539 100644 --- a/pre_commit_hooks/trailing_whitespace_fixer.py +++ b/pre_commit_hooks/trailing_whitespace_fixer.py @@ -7,21 +7,20 @@ from typing import Optional from typing import Sequence -def _fix_file(filename, is_markdown, chars): - # type: (str, bool, Optional[bytes]) -> bool +def _fix_file(filename, is_markdown, chars=None, apply_fixes=True): + # type: (str, bool, Optional[bytes], bool) -> bool with open(filename, mode='rb') as file_processed: lines = file_processed.readlines() newlines = [_process_line(line, is_markdown, chars) for line in lines] if newlines != lines: - with open(filename, mode='wb') as file_processed: - for line in newlines: - file_processed.write(line) - return True - else: - return False + if apply_fixes: + with open(filename, mode='wb') as file_processed: + for line in newlines: + file_processed.write(line) + return newlines != lines -def _process_line(line, is_markdown, chars): +def _process_line(line, is_markdown, chars=None): # type: (bytes, bool, Optional[bytes]) -> bytes if line[-2:] == b'\r\n': eol = b'\r\n' @@ -39,6 +38,12 @@ def _process_line(line, is_markdown, chars): def main(argv=None): # type: (Optional[Sequence[str]]) -> int parser = argparse.ArgumentParser() + parser.add_argument( + '--check', + action='store_true', + help="Don't write the files back. Returns a non-zero code if changes " + 'would be applied. Returns zero if no changes are required.', + ) parser.add_argument( '--no-markdown-linebreak-ext', action='store_true', @@ -89,8 +94,11 @@ def main(argv=None): # type: (Optional[Sequence[str]]) -> int for filename in args.filenames: _, extension = os.path.splitext(filename.lower()) md = all_markdown or extension in md_exts - if _fix_file(filename, md, chars): - print('Fixing {}'.format(filename)) + if _fix_file(filename, md, chars, apply_fixes=not args.check): + if args.check: + print('Would fix {}'.format(filename)) + else: + print('Fixing {}'.format(filename)) return_code = 1 return return_code diff --git a/tests/end_of_file_fixer_test.py b/tests/end_of_file_fixer_test.py index 7f644e7..c2993e7 100644 --- a/tests/end_of_file_fixer_test.py +++ b/tests/end_of_file_fixer_test.py @@ -40,3 +40,15 @@ def test_integration(input_s, expected_retval, output, tmpdir): assert file_output == output assert ret == expected_retval + + +@pytest.mark.parametrize(('input_s', 'expected_retval', 'output'), TESTS) +def test_integration_check(input_s, expected_retval, output, tmpdir): + path = tmpdir.join('file.txt') + path.write_binary(input_s) + + ret = main(['--check', path.strpath]) + file_output = path.read_binary() + + assert file_output == input_s + assert ret == expected_retval diff --git a/tests/trailing_whitespace_fixer_test.py b/tests/trailing_whitespace_fixer_test.py index 97f9aef..d84b689 100644 --- a/tests/trailing_whitespace_fixer_test.py +++ b/tests/trailing_whitespace_fixer_test.py @@ -20,6 +20,20 @@ def test_fixes_trailing_whitespace(input_s, expected, tmpdir): assert path.read() == expected +@pytest.mark.parametrize( + ('input_s'), + ( + ('foo \nbar \n'), + ('bar\t\nbaz\t\n'), + ), +) +def test_check(input_s, tmpdir): + path = tmpdir.join('file.md') + path.write(input_s) + assert main(('--check', path.strpath,)) == 1 + assert path.read() == input_s + + def test_ok_no_newline_end_of_file(tmpdir): filename = tmpdir.join('f') filename.write_binary(b'foo\nbar')