Merge pull request #36 from akaihola/check-option

`--check` option, mirroring `black --check`

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
 ===================
 

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

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)

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),
     )

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

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',
+    ]

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"