diff --git a/CHANGES.rst b/CHANGES.rst index 36294bf96..8e3a21615 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,9 +6,13 @@ These features will be included in the next release: Added ----- +- ``--check`` returns 1 from the process but leaves files untouched if any file would + require reformatting + Fixed ----- + 1.0.0_ - 2020-07-15 =================== diff --git a/README.rst b/README.rst index 8ad9180a1..0cdb2ea3b 100644 --- a/README.rst +++ b/README.rst @@ -112,6 +112,9 @@ The following `command line arguments`_ can also be used to modify the defaults: .. code-block:: shell + --check Don't write the files back, just return the status. + Return code 0 means nothing would change. Return code + 1 means some files would be reformatted. -c PATH, --config PATH Ask `black` and `isort` to read configuration from PATH. -S, --skip-string-normalization @@ -123,6 +126,8 @@ The following `command line arguments`_ can also be used to modify the defaults: *New in version 1.0.0:* isort_ is configured with ``-c`` and ``-l``, too. +*New in version 1.1.0:* The ``--check`` command line option. + .. _Black documentation about pyproject.toml: https://black.readthedocs.io/en/stable/pyproject_toml.html .. _isort documentation about config files: https://timothycrosley.github.io/isort/docs/configuration/config_files/ .. _command line arguments: https://black.readthedocs.io/en/stable/installation_and_usage.html#command-line-options diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 1f0954d52..d7bccd24d 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -4,7 +4,7 @@ import sys from difflib import unified_diff from pathlib import Path -from typing import Iterable, List +from typing import Generator, Iterable, List, Tuple from darker.black_diff import BlackArgs, run_black from darker.chooser import choose_lines @@ -19,8 +19,8 @@ def format_edited_parts( - srcs: Iterable[Path], isort: bool, black_args: BlackArgs, print_diff: bool, -) -> None: + srcs: Iterable[Path], enable_isort: bool, black_args: BlackArgs +) -> Generator[Tuple[Path, str, str, List[str]], None, None]: """Black (and optional isort) formatting for chunks with edits since the last commit 1. run isort on each edited file @@ -39,9 +39,10 @@ def format_edited_parts( 10. write the reformatted source back to the original file :param srcs: Directories and files to re-format - :param isort: ``True`` to also run ``isort`` first on each changed file + :param enable_isort: ``True`` to also run ``isort`` first on each changed file :param black_args: Command-line arguments to send to ``black.FileMode`` - :param print_diff: ``True`` to output diffs instead of modifying source files + :return: A generator which yields details about changes for each file which should + be reformatted, and skips unchanged files. """ git_root = get_common_root(srcs) @@ -53,7 +54,7 @@ def format_edited_parts( worktree_content = src.read_text() # 1. run isort - if isort: + if enable_isort: edited_content = apply_isort( worktree_content, src, @@ -70,7 +71,11 @@ def format_edited_parts( edited_linenums = edited_linenums_differ.head_vs_lines( path_in_repo, edited_lines, context_lines ) - if isort and not edited_linenums and edited_content == worktree_content: + if ( + enable_isort + and not edited_linenums + and edited_content == worktree_content + ): logger.debug("No changes in %s after isort", src) break @@ -117,30 +122,43 @@ def format_edited_parts( continue else: # 10. A re-formatted Python file which produces an identical AST was - # created successfully - write an updated file - # or print the diff - if print_diff: - difflines = list( - unified_diff( - worktree_content.splitlines(), - chosen_lines, - src.as_posix(), - src.as_posix(), - ) - ) - if len(difflines) > 2: - h1, h2, *rest = difflines - print(h1, end="") - print(h2, end="") - print("\n".join(rest)) - else: - logger.info("Writing %s bytes into %s", len(result_str), src) - src.write_text(result_str) + # created successfully - write an updated file or print the diff + # if there were any changes to the original + if result_str != worktree_content: + # `result_str` is just `chosen_lines` concatenated with newlines. + # We need both forms when showing diffs or modifying files. + # Pass them both on to avoid back-and-forth conversion. + yield src, worktree_content, result_str, chosen_lines break -def main(argv: List[str] = None) -> None: - """Parse the command line and apply black formatting for each source file""" +def modify_file(path: Path, new_content: str) -> None: + """Write new content to a file and inform the user by logging""" + logger.info("Writing %s bytes into %s", len(new_content), path) + path.write_text(new_content) + + +def print_diff(path: Path, old_content: str, new_lines: List[str]) -> None: + """Print ``black --diff`` style output for the changes""" + difflines = list( + unified_diff( + old_content.splitlines(), new_lines, path.as_posix(), path.as_posix(), + ) + ) + header1, header2, *rest = difflines + print(header1, end="") + print(header2, end="") + print("\n".join(rest)) + + +def main(argv: List[str] = None) -> int: + """Parse the command line and apply black formatting for each source file + + :param argv: The command line arguments to the ``darker`` command + :return: 1 if the ``--check`` argument was provided and at least one file was (or + should be) reformatted; 0 otherwise. + + """ if argv is None: argv = sys.argv[1:] args = parse_command_line(argv) @@ -166,8 +184,21 @@ def main(argv: List[str] = None) -> None: black_args["skip_string_normalization"] = args.skip_string_normalization paths = {Path(p) for p in args.src} - format_edited_parts(paths, args.isort, black_args, args.diff) + some_files_changed = False + # `new_content` is just `new_lines` concatenated with newlines. + # We need both forms when showing diffs or modifying files. + # Pass them both on to avoid back-and-forth conversion. + for path, old_content, new_content, new_lines in format_edited_parts( + paths, args.isort, black_args + ): + some_files_changed = True + if args.diff: + print_diff(path, old_content, new_lines) + if not args.check and not args.diff: + modify_file(path, new_content) + return 1 if args.check and some_files_changed else 0 if __name__ == "__main__": - main() + RETVAL = main() + sys.exit(RETVAL) diff --git a/src/darker/command_line.py b/src/darker/command_line.py index cf574de50..f94358f7c 100644 --- a/src/darker/command_line.py +++ b/src/darker/command_line.py @@ -38,6 +38,15 @@ def parse_command_line(argv: List[str]) -> Namespace: action="store_true", help="Don't write the files back, just output a diff for each file on stdout", ) + parser.add_argument( + "--check", + action="store_true", + help=( + "Don't write the files back, just return the status. Return code 0 means" + " nothing would change. Return code 1 means some files would be" + " reformatted." + ), + ) parser.add_argument( "-i", "--isort", action="store_true", help="".join(isort_help), ) diff --git a/src/darker/tests/test_command_line.py b/src/darker/tests/test_command_line.py index 8fb2e0026..0883a4196 100644 --- a/src/darker/tests/test_command_line.py +++ b/src/darker/tests/test_command_line.py @@ -1,7 +1,7 @@ import re from pathlib import Path from textwrap import dedent -from unittest.mock import call, patch +from unittest.mock import DEFAULT, Mock, call, patch import pytest @@ -84,26 +84,47 @@ def test_black_options(monkeypatch, tmpdir, git_repo, options, expect): @pytest.mark.parametrize( 'options, expect', [ - (['a.py'], ({Path('a.py')}, False, {}, False)), - (['--isort', 'a.py'], ({Path('a.py')}, True, {}, False)), + (['a.py'], ({Path('a.py')}, False, {})), + (['--isort', 'a.py'], ({Path('a.py')}, True, {})), ( ['--config', 'my.cfg', 'a.py'], - ({Path('a.py')}, False, {'config': 'my.cfg'}, False), + ({Path('a.py')}, False, {'config': 'my.cfg'}), ), ( ['--line-length', '90', 'a.py'], - ({Path('a.py')}, False, {'line_length': 90}, False), + ({Path('a.py')}, False, {'line_length': 90}), ), ( ['--skip-string-normalization', 'a.py'], - ({Path('a.py')}, False, {'skip_string_normalization': True}, False), + ({Path('a.py')}, False, {'skip_string_normalization': True}), ), - (['--diff', 'a.py'], ({Path('a.py')}, False, {}, True)), + (['--diff', 'a.py'], ({Path('a.py')}, False, {})), ], ) def test_options(options, expect): with patch('darker.__main__.format_edited_parts') as format_edited_parts: - main(options) + retval = main(options) format_edited_parts.assert_called_once_with(*expect) + assert retval == 0 + + +@pytest.mark.parametrize( + 'check, changes, expect_retval', + [(False, False, 0), (False, True, 0), (True, False, 0), (True, True, 1)], +) +def test_main_retval(check, changes, expect_retval): + """main() return value is correct based on --check and the need to reformat files""" + format_edited_parts = Mock() + format_edited_parts.return_value = ( + [(Path('/dummy.py'), 'old\n', 'new\n', ['new'])] if changes else [] + ) + check_arg_maybe = ['--check'] if check else [] + with patch.multiple( + 'darker.__main__', format_edited_parts=format_edited_parts, modify_file=DEFAULT + ): + + retval = main(check_arg_maybe + ['a.py']) + + assert retval == expect_retval diff --git a/src/darker/tests/test_main.py b/src/darker/tests/test_main.py index d81edbb19..d710830c9 100644 --- a/src/darker/tests/test_main.py +++ b/src/darker/tests/test_main.py @@ -57,11 +57,12 @@ def test_isort_option_with_isort_calls_sortimports(tmpdir, run_isort, isort_args def test_format_edited_parts_empty(): with pytest.raises(ValueError): - darker.__main__.format_edited_parts([], False, {}, True) + list(darker.__main__.format_edited_parts([], False, {})) A_PY = ['import sys', 'import os', "print( '42')", ''] A_PY_BLACK = ['import sys', 'import os', '', 'print("42")', ''] +A_PY_BLACK_UNNORMALIZE = ['import sys', 'import os', '', "print('42')", ''] A_PY_BLACK_ISORT = ['import os', 'import sys', '', 'print("42")', ''] A_PY_DIFF_BLACK = [ @@ -106,37 +107,100 @@ def test_format_edited_parts_empty(): @pytest.mark.parametrize( - 'isort, black_args, print_diff, expect_stdout, expect_a_py', + 'enable_isort, black_args, expect', [ - (False, {}, True, A_PY_DIFF_BLACK, A_PY), - (True, {}, False, [''], A_PY_BLACK_ISORT,), + (False, {}, A_PY_BLACK), + (True, {}, A_PY_BLACK_ISORT), + (False, {'skip_string_normalization': True}, A_PY_BLACK_UNNORMALIZE), + ], +) +def test_format_edited_parts(git_repo, monkeypatch, enable_isort, black_args, expect): + monkeypatch.chdir(git_repo.root) + paths = git_repo.add({'a.py': '\n', 'b.py': '\n'}, commit='Initial commit') + paths['a.py'].write('\n'.join(A_PY)) + paths['b.py'].write('print(42 )\n') + + changes = list( + darker.__main__.format_edited_parts([Path('a.py')], enable_isort, black_args) + ) + + expect_changes = [(paths['a.py'], '\n'.join(A_PY), '\n'.join(expect), expect[:-1])] + assert changes == expect_changes + + +def test_format_edited_parts_all_unchanged(git_repo, monkeypatch): + """``format_edited_parts()`` yields nothing if no reformatting was needed""" + monkeypatch.chdir(git_repo.root) + paths = git_repo.add({'a.py': 'pass\n', 'b.py': 'pass\n'}, commit='Initial commit') + paths['a.py'].write('"properly"\n"formatted"\n') + paths['b.py'].write('"not"\n"checked"\n') + + result = list(darker.__main__.format_edited_parts([Path('a.py')], True, {})) + + assert result == [] + + +@pytest.mark.parametrize( + 'arguments, expect_stdout, expect_a_py, expect_retval', + [ + (['--diff'], A_PY_DIFF_BLACK, A_PY, 0), + (['--isort'], [''], A_PY_BLACK_ISORT, 0), ( - False, - {'skip_string_normalization': True}, - True, + ['--skip-string-normalization', '--diff'], A_PY_DIFF_BLACK_NO_STR_NORMALIZE, A_PY, + 0, ), - (False, {}, False, [''], A_PY_BLACK), - (True, {}, True, A_PY_DIFF_BLACK_ISORT, A_PY,), + ([], [''], A_PY_BLACK, 0), + (['--isort', '--diff'], A_PY_DIFF_BLACK_ISORT, A_PY, 0), + (['--check'], [''], A_PY, 1), + (['--check', '--diff'], A_PY_DIFF_BLACK, A_PY, 1), + (['--check', '--isort'], [''], A_PY, 1), + (['--check', '--diff', '--isort'], A_PY_DIFF_BLACK_ISORT, A_PY, 1), ], ) -def test_format_edited_parts( - git_repo, - monkeypatch, - capsys, - isort, - black_args, - print_diff, - expect_stdout, - expect_a_py, -): +def test_main( + git_repo, monkeypatch, capsys, arguments, expect_stdout, expect_a_py, expect_retval +): # pylint: disable=too-many-arguments + """Main function outputs diffs and modifies files correctly""" monkeypatch.chdir(git_repo.root) paths = git_repo.add({'a.py': '\n', 'b.py': '\n'}, commit='Initial commit') paths['a.py'].write('\n'.join(A_PY)) paths['b.py'].write('print(42 )\n') - darker.__main__.format_edited_parts([Path('a.py')], isort, black_args, print_diff) + + retval = darker.__main__.main(arguments + ['a.py']) + stdout = capsys.readouterr().out.replace(str(git_repo.root), '') assert stdout.split('\n') == expect_stdout assert paths['a.py'].readlines(cr=False) == expect_a_py assert paths['b.py'].readlines(cr=False) == ['print(42 )', ''] + assert retval == expect_retval + + +def test_output_diff(capsys): + """output_diff() prints Black-style diff output""" + darker.__main__.print_diff( + Path('a.py'), + 'unchanged\nremoved\nkept 1\n2\n3\n4\n5\n6\n7\nchanged\n', + ['inserted', 'unchanged', 'kept 1', '2', '3', '4', '5', '6', '7', 'Changed'], + ) + + assert capsys.readouterr().out.splitlines() == [ + '--- a.py', + '+++ a.py', + '@@ -1,5 +1,5 @@', + '', + '+inserted', + ' unchanged', + '-removed', + ' kept 1', + ' 2', + ' 3', + '@@ -7,4 +7,4 @@', + '', + ' 5', + ' 6', + ' 7', + '-changed', + '+Changed', + ] diff --git a/src/darker/version.py b/src/darker/version.py index e9dbd439e..9f672a370 100644 --- a/src/darker/version.py +++ b/src/darker/version.py @@ -1 +1,3 @@ -__version__ = "1.0.1.dev" +"""The version number for Darker is governed by this file""" + +__version__ = "1.1.0.dev"