From f9e63868db9b7ed3399f28fd1eb5bcd02a4f5e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20P=C3=A9rez?= Date: Sun, 6 Oct 2024 00:57:18 -0400 Subject: [PATCH] Disallow --unique alongside --ignore-cases in file-contents-sorter Closes #794: using these flags at the same time results in undefined behavior - the final ordering is not guaranteed as --unique breaks sorting due to passing the contents through a set. NOTE: I added a newline before and after the mutex group to be consistent with other usages of mutex groups (e.g., check_builtin_literals.py) in this repo. --- pre_commit_hooks/file_contents_sorter.py | 7 ++++-- tests/file_contents_sorter_test.py | 32 +++++++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/pre_commit_hooks/file_contents_sorter.py b/pre_commit_hooks/file_contents_sorter.py index 02bdbccf..0e2d665c 100644 --- a/pre_commit_hooks/file_contents_sorter.py +++ b/pre_commit_hooks/file_contents_sorter.py @@ -54,18 +54,21 @@ def sort_file_contents( def main(argv: Sequence[str] | None = None) -> int: parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs='+', help='Files to sort') - parser.add_argument( + + mutex = parser.add_mutually_exclusive_group(required=False) + mutex.add_argument( '--ignore-case', action='store_const', const=bytes.lower, default=None, help='fold lower case to upper case characters', ) - parser.add_argument( + mutex.add_argument( '--unique', action='store_true', help='ensure each line is unique', ) + args = parser.parse_args(argv) retv = PASS diff --git a/tests/file_contents_sorter_test.py b/tests/file_contents_sorter_test.py index 49b3b793..dfaed7b6 100644 --- a/tests/file_contents_sorter_test.py +++ b/tests/file_contents_sorter_test.py @@ -67,18 +67,6 @@ FAIL, b'Fie\nFoe\nfee\nfum\n', ), - ( - b'fee\nFie\nFoe\nfum\n', - ['--unique', '--ignore-case'], - PASS, - b'fee\nFie\nFoe\nfum\n', - ), - ( - b'fee\nfee\nFie\nFoe\nfum\n', - ['--unique', '--ignore-case'], - FAIL, - b'fee\nFie\nFoe\nfum\n', - ), ), ) def test_integration(input_s, argv, expected_retval, output, tmpdir): @@ -89,3 +77,23 @@ def test_integration(input_s, argv, expected_retval, output, tmpdir): assert path.read_binary() == output assert output_retval == expected_retval + +@pytest.mark.parametrize( + ("input_s", "argv"), + ( + ( + b"fee\nFie\nFoe\nfum\n", + ["--unique", "--ignore-case"], + ), + ( + b"fee\nfee\nFie\nFoe\nfum\n", + ["--unique", "--ignore-case"], + ), + ), +) +def test_integration_invalid_args(input_s, argv, tmpdir): + path = tmpdir.join("file.txt") + path.write_binary(input_s) + + with pytest.raises(SystemExit): + main([str(path)] + argv)