From f205d5373a476e395b81fbae445294a3931b779a Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Fri, 30 Jul 2021 23:44:51 +0300 Subject: [PATCH 1/6] Parse exclusion options from Black configuration --- src/darker/black_diff.py | 38 +++++++++++++++++++---------- src/darker/tests/test_black_diff.py | 23 +++++++++++------ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/darker/black_diff.py b/src/darker/black_diff.py index 6f3c3eb95..f76fbdcba 100644 --- a/src/darker/black_diff.py +++ b/src/darker/black_diff.py @@ -37,11 +37,17 @@ import sys from functools import lru_cache from pathlib import Path -from typing import Optional, Set, cast +from typing import Optional, Pattern, Set # `FileMode as Mode` required to satisfy mypy==0.782. Strange. from black import FileMode as Mode -from black import TargetVersion, find_pyproject_toml, format_str, parse_pyproject_toml +from black import ( + TargetVersion, + find_pyproject_toml, + format_str, + parse_pyproject_toml, + re_compile_maybe_verbose, +) from darker.utils import TextDocument @@ -57,6 +63,9 @@ class BlackArgs(TypedDict, total=False): config: str + exclude: Pattern + extend_exclude: Pattern + force_exclude: Pattern line_length: int skip_string_normalization: bool skip_magic_trailing_comma: bool @@ -78,17 +87,20 @@ def read_black_config(src: Path, value: Optional[str]) -> BlackArgs: if not value: return BlackArgs() - config = parse_pyproject_toml(value) - - return cast( - BlackArgs, - { - key: value - for key, value in config.items() - if key - in ["line_length", "skip_string_normalization", "skip_magic_trailing_comma"] - }, - ) + raw_config = parse_pyproject_toml(value) + + config: BlackArgs = {} + for key in { + "line_length", + "skip_magic_trailing_comma", + "skip_string_normalization", + }: + if key in raw_config: + config[key] = raw_config[key] + for key in {"exclude", "extend_exclude", "force_exclude"}: + if key in raw_config: + config[key] = re_compile_maybe_verbose(raw_config[key]) + return config def run_black( diff --git a/src/darker/tests/test_black_diff.py b/src/darker/tests/test_black_diff.py index 379157646..37feb2994 100644 --- a/src/darker/tests/test_black_diff.py +++ b/src/darker/tests/test_black_diff.py @@ -2,6 +2,7 @@ from unittest.mock import ANY, patch import pytest +import regex as re from darker import black_diff from darker.black_diff import BlackArgs, read_black_config, run_black @@ -18,39 +19,45 @@ expect={"line_length": 99}, ), dict( - config_path="custom.toml", config_lines=["skip-string-normalization = true"], expect={"skip_string_normalization": True}, ), dict( - config_path="custom.toml", config_lines=["skip-string-normalization = false"], expect={"skip_string_normalization": False}, ), dict( - config_path="custom.toml", config_lines=["skip-magic-trailing-comma = true"], expect={"skip_magic_trailing_comma": True}, ), dict( - config_path="custom.toml", config_lines=["skip-magic-trailing-comma = false"], expect={"skip_magic_trailing_comma": False}, ), + dict(config_lines=["target-version = ['py37']"], expect={}), + dict(config_lines=[r"include = '\.pyi$'"], expect={}), + dict( + config_lines=[r"exclude = '\.pyx$'"], + expect={"exclude": re.compile("\\.pyx$")}, + ), + dict( + config_lines=["extend-exclude = '''", r"^/setup\.py", r"|^/dummy\.py", "'''"], + expect={"extend_exclude": re.compile("(?x)^/setup\\.py\n|^/dummy\\.py\n")}, + ), dict( - config_path="custom.toml", config_lines=["target-version = ['py37']"], expect={} + config_lines=["force-exclude = '''", r"^/setup\.py", r"|\.pyc$", "'''"], + expect={"force_exclude": re.compile("(?x)^/setup\\.py\n|\\.pyc$\n")}, ), - dict(config_path="custom.toml", config_lines=["include = '\\.pyi$'"], expect={}), - dict(config_path="custom.toml", config_lines=["exclude = '\\.pyx$'"], expect={}), + config_path=None, ) def test_black_config(tmpdir, config_path, config_lines, expect): tmpdir = Path(tmpdir) src = tmpdir / "src.py" toml = tmpdir / (config_path or "pyproject.toml") - toml.write_text("[tool.black]\n{}\n".format("\n".join(config_lines))) config = read_black_config(src, config_path and str(toml)) + assert config == expect From a07914fd8bb6e60cce948b432e50a663c68727bb Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 24 Jul 2021 12:36:42 +0300 Subject: [PATCH 2/6] Refactor `format_edited_parts` Separate reformatting a single file into its own function. This is in preparation for supporting Black excludes. Also will make Sourcery happier. --- src/darker/__main__.py | 170 ++++++++++++++++++++++++----------------- 1 file changed, 100 insertions(+), 70 deletions(-) diff --git a/src/darker/__main__.py b/src/darker/__main__.py index c75d5ba27..a76a518cc 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -6,7 +6,7 @@ from datetime import datetime from difflib import unified_diff from pathlib import Path -from typing import Generator, Iterable, List, Tuple +from typing import Generator, Iterable, List, Optional, Tuple from darker.black_diff import BlackArgs, run_black from darker.chooser import choose_lines @@ -50,8 +50,6 @@ def format_edited_parts( be reformatted, and skips unchanged files. """ - edited_linenums_differ = EditedLinenumsDiffer(git_root, revrange) - for path_in_repo in sorted(changed_files): src = git_root / path_in_repo rev2_content = git_get_content_at_revision( @@ -68,81 +66,113 @@ def format_edited_parts( ) else: rev2_isorted = rev2_content - max_context_lines = len(rev2_isorted.lines) - minimum_context_lines = BinarySearch(0, max_context_lines + 1) - last_successful_reformat = None - while not minimum_context_lines.found: - context_lines = minimum_context_lines.get_next() - if context_lines > 0: - logger.debug( - "Trying with %s lines of context for `git diff -U %s`", - context_lines, - src, - ) - # 2. diff the given revision and worktree for the file - # 3. extract line numbers in the edited to-file for changed lines - edited_linenums = edited_linenums_differ.revision_vs_lines( - path_in_repo, rev2_isorted, context_lines - ) - if enable_isort and not edited_linenums and rev2_isorted == rev2_content: - logger.debug("No changes in %s after isort", src) - last_successful_reformat = (src, rev2_content, rev2_isorted) - break + # 9. A re-formatted Python file which produces an identical AST was + # created successfully - write an updated file or print the diff if + # there were any changes to the original + src, rev2_content, chosen = _reformat_single_file( + git_root, + path_in_repo, + revrange, + rev2_content, + rev2_isorted, + enable_isort, + black_args, + ) + if report_unmodified or chosen != rev2_content: + yield (src, rev2_content, chosen) - # 4. run black - formatted = run_black(src, rev2_isorted, black_args) - logger.debug( - "Read %s lines from edited file %s", len(rev2_isorted.lines), src - ) - logger.debug("Black reformat resulted in %s lines", len(formatted.lines)) - # 5. get the diff between the edited and reformatted file - opcodes = diff_and_get_opcodes(rev2_isorted, formatted) +def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals + git_root: Path, + path_in_repo: Path, + revrange: RevisionRange, + rev2_content: TextDocument, + rev2_isorted: TextDocument, + enable_isort: bool, + black_args: BlackArgs, +) -> Optional[Tuple[Path, TextDocument, TextDocument]]: + """In a Python file, reformat chunks with edits since the last commit using Black + + :param git_root: The root of the Git repository the files are in + :param path_in_repo: Relative path to a Python source code file + :param revrange: The Git revisions to compare + :param rev2_content: Contents of the file at ``revrange.rev2`` + :param rev2_isorted: Contents of the file after optional import sorting + :param enable_isort: ``True`` if ``isort`` was already run for the file + :param black_args: Command-line arguments to send to ``black.FileMode`` + :return: Details about changes for the reformatted file + :raise: NotEquivalentError - # 6. convert the diff into chunks - black_chunks = list(opcodes_to_chunks(opcodes, rev2_isorted, formatted)) + """ + src = git_root / path_in_repo + edited_linenums_differ = EditedLinenumsDiffer(git_root, revrange) - # 7. choose reformatted content - chosen = TextDocument.from_lines( - choose_lines(black_chunks, edited_linenums), - encoding=rev2_content.encoding, - newline=rev2_content.newline, - mtime=datetime.utcnow().strftime(GIT_DATEFORMAT), + max_context_lines = len(rev2_isorted.lines) + minimum_context_lines = BinarySearch(0, max_context_lines + 1) + last_successful_reformat = None + while not minimum_context_lines.found: + context_lines = minimum_context_lines.get_next() + if context_lines > 0: + logger.debug( + "Trying with %s lines of context for `git diff -U %s`", + context_lines, + src, ) + # 2. diff the given revision and worktree for the file + # 3. extract line numbers in the edited to-file for changed lines + edited_linenums = edited_linenums_differ.revision_vs_lines( + path_in_repo, rev2_isorted, context_lines + ) + if enable_isort and not edited_linenums and rev2_isorted == rev2_content: + logger.debug("No changes in %s after isort", src) + last_successful_reformat = (src, rev2_content, rev2_isorted) + break + + # 4. run black + formatted = run_black(src, rev2_isorted, black_args) + logger.debug("Read %s lines from edited file %s", len(rev2_isorted.lines), src) + logger.debug("Black reformat resulted in %s lines", len(formatted.lines)) + + # 5. get the diff between the edited and reformatted file + opcodes = diff_and_get_opcodes(rev2_isorted, formatted) + + # 6. convert the diff into chunks + black_chunks = list(opcodes_to_chunks(opcodes, rev2_isorted, formatted)) + + # 7. choose reformatted content + chosen = TextDocument.from_lines( + choose_lines(black_chunks, edited_linenums), + encoding=rev2_content.encoding, + newline=rev2_content.newline, + mtime=datetime.utcnow().strftime(GIT_DATEFORMAT), + ) - # 8. verify + # 8. verify + logger.debug( + "Verifying that the %s original edited lines and %s reformatted lines " + "parse into an identical abstract syntax tree", + len(rev2_isorted.lines), + len(chosen.lines), + ) + try: + verify_ast_unchanged(rev2_isorted, chosen, black_chunks, edited_linenums) + except NotEquivalentError: + # Diff produced misaligned chunks which couldn't be reconstructed into + # a partially re-formatted Python file which produces an identical AST. + # Try again with a larger `-U` option for `git diff`, + # or give up if `context_lines` is already very large. logger.debug( - "Verifying that the %s original edited lines and %s reformatted lines " - "parse into an identical abstract syntax tree", - len(rev2_isorted.lines), - len(chosen.lines), + "AST verification of %s with %s lines of context failed", + src, + context_lines, ) - try: - verify_ast_unchanged( - rev2_isorted, chosen, black_chunks, edited_linenums - ) - except NotEquivalentError: - # Diff produced misaligned chunks which couldn't be reconstructed into - # a partially re-formatted Python file which produces an identical AST. - # Try again with a larger `-U` option for `git diff`, - # or give up if `context_lines` is already very large. - logger.debug( - "AST verification of %s with %s lines of context failed", - src, - context_lines, - ) - minimum_context_lines.respond(False) - else: - minimum_context_lines.respond(True) - last_successful_reformat = (src, rev2_content, chosen) - if not last_successful_reformat: - raise NotEquivalentError(path_in_repo) - # 9. A re-formatted Python file which produces an identical AST was - # created successfully - write an updated file or print the diff if - # there were any changes to the original - src, rev2_content, chosen = last_successful_reformat - if report_unmodified or chosen != rev2_content: - yield (src, rev2_content, chosen) + minimum_context_lines.respond(False) + else: + minimum_context_lines.respond(True) + last_successful_reformat = (src, rev2_content, chosen) + if not last_successful_reformat: + raise NotEquivalentError(path_in_repo) + return last_successful_reformat def modify_file(path: Path, new_content: TextDocument) -> None: From 1a400869bcf434da71a7d362a2d212d3db2518d0 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Fri, 30 Jul 2021 17:49:44 +0300 Subject: [PATCH 3/6] Disable Mypy checks for the `regex` package --- mypy.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mypy.ini b/mypy.ini index ba4e6785d..2dae7a443 100644 --- a/mypy.ini +++ b/mypy.ini @@ -75,3 +75,6 @@ ignore_missing_imports = True [mypy-pygments.lexers.*] ignore_missing_imports = True + +[mypy-regex] +ignore_missing_imports = True From d9795c117320f6fe2ce9a775fe613717c347b894 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Fri, 30 Jul 2021 17:52:05 +0300 Subject: [PATCH 4/6] Only read Black configuration once This used to be done for each file separately and cached. --- src/darker/__main__.py | 34 +++++++------- src/darker/black_diff.py | 71 ++++++++++++++--------------- src/darker/tests/test_black_diff.py | 12 ++--- src/darker/tests/test_main.py | 16 +++---- 4 files changed, 65 insertions(+), 68 deletions(-) diff --git a/src/darker/__main__.py b/src/darker/__main__.py index a76a518cc..8067263f2 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -6,9 +6,9 @@ from datetime import datetime from difflib import unified_diff from pathlib import Path -from typing import Generator, Iterable, List, Optional, Tuple +from typing import Generator, Iterable, List, Tuple -from darker.black_diff import BlackArgs, run_black +from darker.black_diff import BlackConfig, read_black_config, run_black from darker.chooser import choose_lines from darker.command_line import parse_command_line from darker.config import OutputMode, dump_config @@ -34,7 +34,7 @@ def format_edited_parts( changed_files: Iterable[Path], revrange: RevisionRange, enable_isort: bool, - black_args: BlackArgs, + black_config: BlackConfig, report_unmodified: bool, ) -> Generator[Tuple[Path, TextDocument, TextDocument], None, None]: """Black (and optional isort) formatting for chunks with edits since the last commit @@ -44,7 +44,7 @@ def format_edited_parts( given Git revisions :param revrange: The Git revisions to compare :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 black_config: Configuration to use for running Black :param report_unmodified: ``True`` to yield also files which weren't modified :return: A generator which yields details about changes for each file which should be reformatted, and skips unchanged files. @@ -61,8 +61,8 @@ def format_edited_parts( rev2_isorted = apply_isort( rev2_content, src, - black_args.get("config"), - black_args.get("line_length"), + black_config.get("config"), + black_config.get("line_length"), ) else: rev2_isorted = rev2_content @@ -76,7 +76,7 @@ def format_edited_parts( rev2_content, rev2_isorted, enable_isort, - black_args, + black_config, ) if report_unmodified or chosen != rev2_content: yield (src, rev2_content, chosen) @@ -89,8 +89,8 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals rev2_content: TextDocument, rev2_isorted: TextDocument, enable_isort: bool, - black_args: BlackArgs, -) -> Optional[Tuple[Path, TextDocument, TextDocument]]: + black_config: BlackConfig, +) -> Tuple[Path, TextDocument, TextDocument]: """In a Python file, reformat chunks with edits since the last commit using Black :param git_root: The root of the Git repository the files are in @@ -99,7 +99,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals :param rev2_content: Contents of the file at ``revrange.rev2`` :param rev2_isorted: Contents of the file after optional import sorting :param enable_isort: ``True`` if ``isort`` was already run for the file - :param black_args: Command-line arguments to send to ``black.FileMode`` + :param black_config: Configuration to use for running Black :return: Details about changes for the reformatted file :raise: NotEquivalentError @@ -129,7 +129,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals break # 4. run black - formatted = run_black(src, rev2_isorted, black_args) + formatted = run_black(rev2_isorted, black_config) logger.debug("Read %s lines from edited file %s", len(rev2_isorted.lines), src) logger.debug("Black reformat resulted in %s lines", len(formatted.lines)) @@ -284,15 +284,15 @@ def main(argv: List[str] = None) -> int: logger.error(f"{ISORT_INSTRUCTION} to use the `--isort` option.") exit(1) - black_args = BlackArgs() + black_config = read_black_config(tuple(args.src), args.config) if args.config: - black_args["config"] = args.config + black_config["config"] = args.config if args.line_length: - black_args["line_length"] = args.line_length + black_config["line_length"] = args.line_length if args.skip_string_normalization is not None: - black_args["skip_string_normalization"] = args.skip_string_normalization + black_config["skip_string_normalization"] = args.skip_string_normalization if args.skip_magic_trailing_comma is not None: - black_args["skip_magic_trailing_comma"] = args.skip_magic_trailing_comma + black_config["skip_magic_trailing_comma"] = args.skip_magic_trailing_comma paths = {Path(p) for p in args.src} git_root = get_common_root(paths) @@ -319,7 +319,7 @@ def main(argv: List[str] = None) -> int: changed_files, revrange, args.isort, - black_args, + black_config, report_unmodified=output_mode == OutputMode.CONTENT, ): failures_on_modified_lines = True diff --git a/src/darker/black_diff.py b/src/darker/black_diff.py index f76fbdcba..cb73ab6a0 100644 --- a/src/darker/black_diff.py +++ b/src/darker/black_diff.py @@ -16,7 +16,7 @@ First, :func:`run_black` uses Black to reformat the contents of a given file. Reformatted lines are returned e.g.:: - >>> dst = run_black(src, src_content, black_args={}) + >>> dst = run_black(src_content, black_config={}) >>> dst.lines ('for i in range(5):', ' print(i)', 'print("done")') @@ -35,9 +35,7 @@ import logging import sys -from functools import lru_cache -from pathlib import Path -from typing import Optional, Pattern, Set +from typing import Optional, Pattern, Set, Tuple # `FileMode as Mode` required to satisfy mypy==0.782. Strange. from black import FileMode as Mode @@ -56,16 +54,16 @@ else: from typing_extensions import TypedDict -__all__ = ["BlackArgs", "Mode", "run_black"] +__all__ = ["BlackConfig", "Mode", "run_black"] logger = logging.getLogger(__name__) -class BlackArgs(TypedDict, total=False): +class BlackConfig(TypedDict, total=False): config: str - exclude: Pattern - extend_exclude: Pattern - force_exclude: Pattern + exclude: Pattern[str] + extend_exclude: Pattern[str] + force_exclude: Pattern[str] line_length: int skip_string_normalization: bool skip_magic_trailing_comma: bool @@ -79,68 +77,67 @@ class BlackModeAttributes(TypedDict, total=False): magic_trailing_comma: bool -@lru_cache(maxsize=1) -def read_black_config(src: Path, value: Optional[str]) -> BlackArgs: - """Read the black configuration from pyproject.toml""" - value = value or find_pyproject_toml((str(src),)) +def read_black_config(src: Tuple[str, ...], value: Optional[str]) -> BlackConfig: + """Read the black configuration from ``pyproject.toml`` + + :param src: The source code files and directories to be processed by Darker + :param value: The path of the Black configuration file + :return: A dictionary of those Black parameters from the configuration file which + are supported by Darker + + """ + value = value or find_pyproject_toml(src) if not value: - return BlackArgs() + return BlackConfig() raw_config = parse_pyproject_toml(value) - config: BlackArgs = {} + config: BlackConfig = {} for key in { "line_length", "skip_magic_trailing_comma", "skip_string_normalization", }: if key in raw_config: - config[key] = raw_config[key] + config[key] = raw_config[key] # type: ignore for key in {"exclude", "extend_exclude", "force_exclude"}: if key in raw_config: - config[key] = re_compile_maybe_verbose(raw_config[key]) + config[key] = re_compile_maybe_verbose(raw_config[key]) # type: ignore return config -def run_black( - src: Path, src_contents: TextDocument, black_args: BlackArgs -) -> TextDocument: +def run_black(src_contents: TextDocument, black_config: BlackConfig) -> TextDocument: """Run the black formatter for the Python source code given as a string Return lines of the original file as well as the formatted content. - :param src: The originating file path for the source code :param src_contents: The source code - :param black_args: Command-line arguments to send to ``black.FileMode`` + :param black_config: Configuration to use for running Black """ - config = black_args.pop("config", None) - combined_args = read_black_config(src, config) - combined_args.update(black_args) - - effective_args = BlackModeAttributes() - if "line_length" in combined_args: - effective_args["line_length"] = combined_args["line_length"] - if "skip_magic_trailing_comma" in combined_args: - effective_args["magic_trailing_comma"] = not combined_args[ + # Collect relevant Black configuration options from ``black_config`` in order to + # pass them to Black's ``format_str()``. File exclusion options aren't needed since + # at this point we already have a single file's content to work on. + mode = BlackModeAttributes() + if "line_length" in black_config: + mode["line_length"] = black_config["line_length"] + if "skip_magic_trailing_comma" in black_config: + mode["magic_trailing_comma"] = not black_config[ "skip_magic_trailing_comma" ] - if "skip_string_normalization" in combined_args: + if "skip_string_normalization" in black_config: # The ``black`` command line argument is # ``--skip-string-normalization``, but the parameter for # ``black.Mode`` needs to be the opposite boolean of # ``skip-string-normalization``, hence the inverse boolean - effective_args["string_normalization"] = not combined_args[ + mode["string_normalization"] = not black_config[ "skip_string_normalization" ] - # Override defaults and pyproject.toml settings if they've been specified - # from the command line arguments - mode = Mode(**effective_args) contents_for_black = src_contents.string_with_newline("\n") return TextDocument.from_str( - format_str(contents_for_black, mode=mode), + format_str(contents_for_black, mode=Mode(**mode)), encoding=src_contents.encoding, override_newline=src_contents.newline, ) diff --git a/src/darker/tests/test_black_diff.py b/src/darker/tests/test_black_diff.py index 37feb2994..38cbef267 100644 --- a/src/darker/tests/test_black_diff.py +++ b/src/darker/tests/test_black_diff.py @@ -5,7 +5,7 @@ import regex as re from darker import black_diff -from darker.black_diff import BlackArgs, read_black_config, run_black +from darker.black_diff import BlackConfig, read_black_config, run_black from darker.utils import TextDocument @@ -56,14 +56,14 @@ def test_black_config(tmpdir, config_path, config_lines, expect): toml = tmpdir / (config_path or "pyproject.toml") toml.write_text("[tool.black]\n{}\n".format("\n".join(config_lines))) - config = read_black_config(src, config_path and str(toml)) + config = read_black_config((str(src),), config_path and str(toml)) assert config == expect @pytest.mark.parametrize("encoding", ["utf-8", "iso-8859-1"]) @pytest.mark.parametrize("newline", ["\n", "\r\n"]) -def test_run_black(tmpdir, encoding, newline): +def test_run_black(encoding, newline): """Running Black through its Python internal API gives correct results""" src = TextDocument.from_lines( [f"# coding: {encoding}", "print ( 'touché' )"], @@ -71,7 +71,7 @@ def test_run_black(tmpdir, encoding, newline): newline=newline, ) - result = run_black(Path(tmpdir / "src.py"), src, BlackArgs()) + result = run_black(src, BlackConfig()) assert result.lines == ( f"# coding: {encoding}", @@ -82,12 +82,12 @@ def test_run_black(tmpdir, encoding, newline): @pytest.mark.parametrize("newline", ["\n", "\r\n"]) -def test_run_black_always_uses_unix_newlines(tmpdir, newline): +def test_run_black_always_uses_unix_newlines(newline): """Content is always passed to Black with Unix newlines""" src = TextDocument.from_str(f"print ( 'touché' ){newline}") with patch.object(black_diff, "format_str") as format_str: format_str.return_value = 'print("touché")\n' - _ = run_black(Path(tmpdir / "src.py"), src, BlackArgs()) + _ = run_black(src, BlackConfig()) format_str.assert_called_once_with("print ( 'touché' )\n", mode=ANY) diff --git a/src/darker/tests/test_main.py b/src/darker/tests/test_main.py index 756d4523c..3fee9ad85 100644 --- a/src/darker/tests/test_main.py +++ b/src/darker/tests/test_main.py @@ -110,17 +110,17 @@ def test_isort_option_with_isort_calls_sortimports(tmpdir, run_isort, isort_args @pytest.mark.kwparametrize( - dict(enable_isort=False, black_args={}, expect=A_PY_BLACK), - dict(enable_isort=True, black_args={}, expect=A_PY_BLACK_ISORT), + dict(enable_isort=False, black_config={}, expect=A_PY_BLACK), + dict(enable_isort=True, black_config={}, expect=A_PY_BLACK_ISORT), dict( enable_isort=False, - black_args={"skip_string_normalization": True}, + black_config={"skip_string_normalization": True}, expect=A_PY_BLACK_UNNORMALIZE, ), ids=["black", "black_isort", "black_unnormalize"], ) @pytest.mark.parametrize("newline", ["\n", "\r\n"], ids=["unix", "windows"]) -def test_format_edited_parts(git_repo, enable_isort, black_args, newline, expect): +def test_format_edited_parts(git_repo, enable_isort, black_config, newline, expect): """Correct reformatting and import sorting changes are produced""" paths = git_repo.add({"a.py": newline, "b.py": newline}, commit="Initial commit") paths["a.py"].write_bytes(newline.join(A_PY).encode("ascii")) @@ -131,7 +131,7 @@ def test_format_edited_parts(git_repo, enable_isort, black_args, newline, expect [Path("a.py")], RevisionRange("HEAD"), enable_isort, - black_args, + black_config, report_unmodified=False, ) @@ -182,7 +182,7 @@ def test_format_edited_parts_ast_changed(git_repo, caplog): [Path("a.py")], RevisionRange("HEAD"), enable_isort=False, - black_args={}, + black_config={}, report_unmodified=False, ) ) @@ -226,7 +226,7 @@ def test_format_edited_parts_isort_on_already_formatted(git_repo): {Path("a.py")}, RevisionRange("HEAD"), enable_isort=True, - black_args={}, + black_config={}, report_unmodified=False, ) @@ -281,7 +281,7 @@ def test_format_edited_parts_historical(git_repo, rev1, rev2, expect): {Path("a.py")}, RevisionRange(rev1, rev2), enable_isort=True, - black_args={}, + black_config={}, report_unmodified=False, ) From bc93fe3a28002dd47eac6498b8ed0a33c7f3bb40 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Fri, 30 Jul 2021 22:25:19 +0300 Subject: [PATCH 5/6] Skip Black for files excluded in Black config Add unit tests. --- src/darker/__main__.py | 56 +++++++----- src/darker/black_diff.py | 46 ++++++++-- src/darker/tests/test_black_diff.py | 85 ++++++++++++++++- src/darker/tests/test_main.py | 136 ++++++++++++++++++++++++---- 4 files changed, 274 insertions(+), 49 deletions(-) diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 8067263f2..2bea7785f 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -6,9 +6,14 @@ from datetime import datetime from difflib import unified_diff from pathlib import Path -from typing import Generator, Iterable, List, Tuple +from typing import Collection, Generator, List, Tuple -from darker.black_diff import BlackConfig, read_black_config, run_black +from darker.black_diff import ( + BlackConfig, + apply_black_excludes, + read_black_config, + run_black, +) from darker.chooser import choose_lines from darker.command_line import parse_command_line from darker.config import OutputMode, dump_config @@ -31,7 +36,7 @@ def format_edited_parts( git_root: Path, - changed_files: Iterable[Path], + changed_files: Collection[Path], revrange: RevisionRange, enable_isort: bool, black_config: BlackConfig, @@ -39,6 +44,10 @@ def format_edited_parts( ) -> Generator[Tuple[Path, TextDocument, TextDocument], None, None]: """Black (and optional isort) formatting for chunks with edits since the last commit + Files excluded by Black's configuration are not reformatted using Black, but their + imports are still sorted. Also, linters will be run for all files in a separate step + after this function has completed. + :param git_root: The root of the Git repository the files are in :param changed_files: Files which have been modified in the repository between the given Git revisions @@ -50,6 +59,7 @@ def format_edited_parts( be reformatted, and skips unchanged files. """ + files_to_blacken = apply_black_excludes(changed_files, git_root, black_config) for path_in_repo in sorted(changed_files): src = git_root / path_in_repo rev2_content = git_get_content_at_revision( @@ -66,20 +76,24 @@ def format_edited_parts( ) else: rev2_isorted = rev2_content - # 9. A re-formatted Python file which produces an identical AST was - # created successfully - write an updated file or print the diff if - # there were any changes to the original - src, rev2_content, chosen = _reformat_single_file( - git_root, - path_in_repo, - revrange, - rev2_content, - rev2_isorted, - enable_isort, - black_config, - ) - if report_unmodified or chosen != rev2_content: - yield (src, rev2_content, chosen) + if src in files_to_blacken: + # 9. A re-formatted Python file which produces an identical AST was + # created successfully - write an updated file or print the diff if + # there were any changes to the original + content_after_reformatting = _reformat_single_file( + git_root, + path_in_repo, + revrange, + rev2_content, + rev2_isorted, + enable_isort, + black_config, + ) + else: + # File was excluded by Black configuration, don't reformat + content_after_reformatting = rev2_isorted + if report_unmodified or content_after_reformatting != rev2_content: + yield (src, rev2_content, content_after_reformatting) def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals @@ -90,7 +104,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals rev2_isorted: TextDocument, enable_isort: bool, black_config: BlackConfig, -) -> Tuple[Path, TextDocument, TextDocument]: +) -> TextDocument: """In a Python file, reformat chunks with edits since the last commit using Black :param git_root: The root of the Git repository the files are in @@ -100,7 +114,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals :param rev2_isorted: Contents of the file after optional import sorting :param enable_isort: ``True`` if ``isort`` was already run for the file :param black_config: Configuration to use for running Black - :return: Details about changes for the reformatted file + :return: Contents of the file after reformatting :raise: NotEquivalentError """ @@ -125,7 +139,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals ) if enable_isort and not edited_linenums and rev2_isorted == rev2_content: logger.debug("No changes in %s after isort", src) - last_successful_reformat = (src, rev2_content, rev2_isorted) + last_successful_reformat = rev2_isorted break # 4. run black @@ -169,7 +183,7 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals minimum_context_lines.respond(False) else: minimum_context_lines.respond(True) - last_successful_reformat = (src, rev2_content, chosen) + last_successful_reformat = chosen if not last_successful_reformat: raise NotEquivalentError(path_in_repo) return last_successful_reformat diff --git a/src/darker/black_diff.py b/src/darker/black_diff.py index cb73ab6a0..276792c14 100644 --- a/src/darker/black_diff.py +++ b/src/darker/black_diff.py @@ -35,7 +35,8 @@ import logging import sys -from typing import Optional, Pattern, Set, Tuple +from pathlib import Path +from typing import Collection, Optional, Pattern, Set, Tuple # `FileMode as Mode` required to satisfy mypy==0.782. Strange. from black import FileMode as Mode @@ -46,6 +47,9 @@ parse_pyproject_toml, re_compile_maybe_verbose, ) +from black.const import DEFAULT_EXCLUDES, DEFAULT_INCLUDES +from black.files import gen_python_files +from black.report import Report from darker.utils import TextDocument @@ -59,7 +63,12 @@ logger = logging.getLogger(__name__) +DEFAULT_EXCLUDE_RE = re_compile_maybe_verbose(DEFAULT_EXCLUDES) +DEFAULT_INCLUDE_RE = re_compile_maybe_verbose(DEFAULT_INCLUDES) + + class BlackConfig(TypedDict, total=False): + """Type definition for Black configuration dictionaries""" config: str exclude: Pattern[str] extend_exclude: Pattern[str] @@ -70,6 +79,7 @@ class BlackConfig(TypedDict, total=False): class BlackModeAttributes(TypedDict, total=False): + """Type definition for items accepted by ``black.Mode``""" target_versions: Set[TargetVersion] line_length: int string_normalization: bool @@ -107,6 +117,32 @@ def read_black_config(src: Tuple[str, ...], value: Optional[str]) -> BlackConfig return config +def apply_black_excludes( + paths: Collection[Path], root: Path, black_config: BlackConfig +) -> Set[Path]: + """Get the subset of files which are not excluded by Black's configuration + + :param paths: Relative paths from ``root`` to Python source file paths to consider + :param root: A common root directory for all ``paths`` + :param black_config: Black configuration which contains the exclude options read + from Black's configuration files + :return: Absolute paths of files which should be reformatted using Black + + """ + return set( + gen_python_files( + (root / path for path in paths), + root, + include=DEFAULT_INCLUDE_RE, + exclude=black_config.get("exclude", DEFAULT_EXCLUDE_RE), + extend_exclude=black_config.get("extend_exclude"), + force_exclude=black_config.get("force_exclude"), + report=Report(), + gitignore=None, + ) + ) + + def run_black(src_contents: TextDocument, black_config: BlackConfig) -> TextDocument: """Run the black formatter for the Python source code given as a string @@ -123,17 +159,13 @@ def run_black(src_contents: TextDocument, black_config: BlackConfig) -> TextDocu if "line_length" in black_config: mode["line_length"] = black_config["line_length"] if "skip_magic_trailing_comma" in black_config: - mode["magic_trailing_comma"] = not black_config[ - "skip_magic_trailing_comma" - ] + mode["magic_trailing_comma"] = not black_config["skip_magic_trailing_comma"] if "skip_string_normalization" in black_config: # The ``black`` command line argument is # ``--skip-string-normalization``, but the parameter for # ``black.Mode`` needs to be the opposite boolean of # ``skip-string-normalization``, hence the inverse boolean - mode["string_normalization"] = not black_config[ - "skip_string_normalization" - ] + mode["string_normalization"] = not black_config["skip_string_normalization"] contents_for_black = src_contents.string_with_newline("\n") return TextDocument.from_str( diff --git a/src/darker/tests/test_black_diff.py b/src/darker/tests/test_black_diff.py index 38cbef267..615f8354d 100644 --- a/src/darker/tests/test_black_diff.py +++ b/src/darker/tests/test_black_diff.py @@ -5,7 +5,12 @@ import regex as re from darker import black_diff -from darker.black_diff import BlackConfig, read_black_config, run_black +from darker.black_diff import ( + BlackConfig, + apply_black_excludes, + read_black_config, + run_black, +) from darker.utils import TextDocument @@ -61,6 +66,66 @@ def test_black_config(tmpdir, config_path, config_lines, expect): assert config == expect +@pytest.mark.kwparametrize( + dict( + expect={ + "none", + "exclude", + "extend", + "force", + "exclude+extend", + "exclude+force", + "extend+force", + "exclude+extend+force", + } + ), + dict(exclude="exclude", expect={"none", "extend", "force", "extend+force"}), + dict(extend_exclude="extend", expect={"none", "exclude", "force", "exclude+force"}), + dict(force_exclude="force", expect={"none", "exclude", "extend", "exclude+extend"}), + dict(exclude="exclude", extend_exclude="extend", expect={"none", "force"}), + dict(exclude="exclude", force_exclude="force", expect={"none", "extend"}), + dict(extend_exclude="extend", force_exclude="force", expect={"none", "exclude"}), + dict( + exclude="exclude", + extend_exclude="extend", + force_exclude="force", + expect={"none"}, + ), + exclude=None, + extend_exclude=None, + force_exclude=None, +) +def test_apply_black_excludes(tmp_path, exclude, extend_exclude, force_exclude, expect): + """``apply_black_excludes()`` skips excluded files correctly""" + names = { + Path(name) + for name in { + "none.py", + "exclude.py", + "extend.py", + "force.py", + "exclude+extend.py", + "exclude+force.py", + "extend+force.py", + "exclude+extend+force.py", + } + } + paths = {tmp_path / name for name in names} + for path in paths: + path.touch() + black_config = BlackConfig( + { + "exclude": re.compile(exclude) if exclude else None, + "extend_exclude": re.compile(extend_exclude) if extend_exclude else None, + "force_exclude": re.compile(force_exclude) if force_exclude else None, + } + ) + + result = apply_black_excludes(names, tmp_path, black_config) + + assert result == {tmp_path / f"{path}.py" for path in expect} + + @pytest.mark.parametrize("encoding", ["utf-8", "iso-8859-1"]) @pytest.mark.parametrize("newline", ["\n", "\r\n"]) def test_run_black(encoding, newline): @@ -91,3 +156,21 @@ def test_run_black_always_uses_unix_newlines(newline): _ = run_black(src, BlackConfig()) format_str.assert_called_once_with("print ( 'touché' )\n", mode=ANY) + + +def test_run_black_ignores_excludes(): + """Black's exclude configuration is ignored by ``run_black()``""" + src = TextDocument.from_str("a=1\n") + + result = run_black( + src, + BlackConfig( + { + "exclude": re.compile(r".*"), + "extend_exclude": re.compile(r".*"), + "force_exclude": re.compile(r".*"), + } + ), + ) + + assert result.string == "a = 1\n" diff --git a/src/darker/tests/test_main.py b/src/darker/tests/test_main.py index 3fee9ad85..0034d470a 100644 --- a/src/darker/tests/test_main.py +++ b/src/darker/tests/test_main.py @@ -6,6 +6,7 @@ import re from argparse import ArgumentError from pathlib import Path +from textwrap import dedent from types import SimpleNamespace from unittest.mock import Mock, patch @@ -70,6 +71,7 @@ def test_isort_option_with_isort_calls_sortimports(tmpdir, run_isort, isort_args A_PY = ["import sys", "import os", "print( '42')", ""] +A_PY_ISORT = ["import os", "import sys", "", "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")', ""] @@ -110,18 +112,51 @@ def test_isort_option_with_isort_calls_sortimports(tmpdir, run_isort, isort_args @pytest.mark.kwparametrize( - dict(enable_isort=False, black_config={}, expect=A_PY_BLACK), - dict(enable_isort=True, black_config={}, expect=A_PY_BLACK_ISORT), + dict(enable_isort=False, black_config={}, expect=[A_PY_BLACK]), + dict(enable_isort=True, black_config={}, expect=[A_PY_BLACK_ISORT]), dict( enable_isort=False, black_config={"skip_string_normalization": True}, - expect=A_PY_BLACK_UNNORMALIZE, + expect=[A_PY_BLACK_UNNORMALIZE], + ), + dict( + enable_isort=False, + black_config={"exclude": re.compile(r"/a\.py$")}, + expect=[], + ), + dict( + enable_isort=True, + black_config={"exclude": re.compile(r"/a\.py$")}, + expect=[A_PY_ISORT], + ), + dict( + enable_isort=False, + black_config={"extend_exclude": re.compile(r"/a\.py$")}, + expect=[], + ), + dict( + enable_isort=True, + black_config={"extend_exclude": re.compile(r"/a\.py$")}, + expect=[A_PY_ISORT], + ), + dict( + enable_isort=False, + black_config={"force_exclude": re.compile(r"/a\.py$")}, + expect=[], + ), + dict( + enable_isort=True, + black_config={"force_exclude": re.compile(r"/a\.py$")}, + expect=[A_PY_ISORT], ), - ids=["black", "black_isort", "black_unnormalize"], ) @pytest.mark.parametrize("newline", ["\n", "\r\n"], ids=["unix", "windows"]) def test_format_edited_parts(git_repo, enable_isort, black_config, newline, expect): - """Correct reformatting and import sorting changes are produced""" + """Correct reformatting and import sorting changes are produced + + Also, Black reformatting is skipped for files excluded in Black configuration + + """ paths = git_repo.add({"a.py": newline, "b.py": newline}, commit="Initial commit") paths["a.py"].write_bytes(newline.join(A_PY).encode("ascii")) paths["b.py"].write_bytes(f"print(42 ){newline}".encode("ascii")) @@ -140,7 +175,13 @@ def test_format_edited_parts(git_repo, enable_isort, black_config, newline, expe for path, worktree_content, chosen in result ] expect_changes = [ - (paths["a.py"], newline.join(A_PY), newline.join(expect), tuple(expect[:-1])) + ( + paths["a.py"], + newline.join(A_PY), + newline.join(expect_lines), + tuple(expect_lines[:-1]), + ) + for expect_lines in expect ] assert changes == expect_changes @@ -320,8 +361,57 @@ def test_format_edited_parts_historical(git_repo, rev1, rev2, expect): # Windows compatible path assertion using `pathlib.Path()` expect_stdout=A_PY_DIFF_BLACK + [f"a.py:1: message {Path('/a.py')}"], ), + dict( + arguments=[], + pyproject_toml=""" + [tool.black] + exclude = 'a.py' + """, + expect_a_py=A_PY, + ), + dict( + arguments=["--diff"], + pyproject_toml=""" + [tool.black] + exclude = 'a.py' + """, + expect_stdout=[], + ), + dict( + arguments=[], + pyproject_toml=""" + [tool.black] + extend_exclude = 'a.py' + """, + expect_a_py=A_PY, + ), + dict( + arguments=["--diff"], + pyproject_toml=""" + [tool.black] + extend_exclude = 'a.py' + """, + expect_stdout=[], + ), + dict( + arguments=[], + pyproject_toml=""" + [tool.black] + force_exclude = 'a.py' + """, + expect_a_py=A_PY, + ), + dict( + arguments=["--diff"], + pyproject_toml=""" + [tool.black] + force_exclude = 'a.py' + """, + expect_stdout=[], + ), # for all test cases, by default there's no output, `a.py` stays unmodified, and the # return value is a zero: + pyproject_toml="", expect_stdout=[], expect_a_py=A_PY, expect_retval=0, @@ -329,17 +419,20 @@ def test_format_edited_parts_historical(git_repo, rev1, rev2, expect): @pytest.mark.parametrize("newline", ["\n", "\r\n"], ids=["unix", "windows"]) def test_main( git_repo, - monkeypatch, capsys, + find_project_root_cache_clear, arguments, newline, + pyproject_toml, 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": newline, "b.py": newline}, commit="Initial commit") + paths = git_repo.add( + {"pyproject.toml": dedent(pyproject_toml), "a.py": newline, "b.py": newline}, + commit="Initial commit", + ) paths["a.py"].write_bytes(newline.join(A_PY).encode("ascii")) paths["b.py"].write_bytes(f"print(42 ){newline}".encode("ascii")) @@ -347,16 +440,17 @@ def test_main( stdout = capsys.readouterr().out.replace(str(git_repo.root), "") diff_output = stdout.splitlines(False) - if "--diff" in arguments: - assert "\t" in diff_output[0], diff_output[0] - diff_output[0], old_mtime = diff_output[0].split("\t", 1) - assert old_mtime.endswith(" +0000") - assert "\t" in diff_output[1], diff_output[1] - diff_output[1], new_mtime = diff_output[1].split("\t", 1) - assert new_mtime.endswith(" +0000") - assert all("\t" not in line for line in diff_output[2:]) - else: - assert all("\t" not in line for line in diff_output) + if expect_stdout: + if "--diff" in arguments: + assert "\t" in diff_output[0], diff_output[0] + diff_output[0], old_mtime = diff_output[0].split("\t", 1) + assert old_mtime.endswith(" +0000") + assert "\t" in diff_output[1], diff_output[1] + diff_output[1], new_mtime = diff_output[1].split("\t", 1) + assert new_mtime.endswith(" +0000") + assert all("\t" not in line for line in diff_output[2:]) + else: + assert all("\t" not in line for line in diff_output) assert diff_output == expect_stdout assert paths["a.py"].read_bytes().decode("ascii") == newline.join(expect_a_py) assert paths["b.py"].read_bytes().decode("ascii") == f"print(42 ){newline}" @@ -367,7 +461,9 @@ def test_main( "encoding, text", [(b"utf-8", b"touch\xc3\xa9"), (b"iso-8859-1", b"touch\xe9")] ) @pytest.mark.parametrize("newline", [b"\n", b"\r\n"]) -def test_main_encoding(git_repo, encoding, text, newline): +def test_main_encoding( + git_repo, find_project_root_cache_clear, encoding, text, newline +): """Encoding and newline of the file is kept unchanged after reformatting""" paths = git_repo.add({"a.py": newline.decode("ascii")}, commit="Initial commit") edited = [b"# coding: ", encoding, newline, b's="', text, b'"', newline] From b097141d01d84c42e02fafceb7f463946b56125e Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Fri, 30 Jul 2021 22:45:07 +0300 Subject: [PATCH 6/6] Work-around for PyCQA/pylint#2377 https://github.com/PyCQA/pylint/issues/2377 --- src/darker/__main__.py | 2 +- src/darker/black_diff.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 2bea7785f..8fdb431c7 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -36,7 +36,7 @@ def format_edited_parts( git_root: Path, - changed_files: Collection[Path], + changed_files: Collection[Path], # pylint: disable=unsubscriptable-object revrange: RevisionRange, enable_isort: bool, black_config: BlackConfig, diff --git a/src/darker/black_diff.py b/src/darker/black_diff.py index 276792c14..cd9073819 100644 --- a/src/darker/black_diff.py +++ b/src/darker/black_diff.py @@ -118,7 +118,9 @@ def read_black_config(src: Tuple[str, ...], value: Optional[str]) -> BlackConfig def apply_black_excludes( - paths: Collection[Path], root: Path, black_config: BlackConfig + paths: Collection[Path], # pylint: disable=unsubscriptable-object + root: Path, + black_config: BlackConfig, ) -> Set[Path]: """Get the subset of files which are not excluded by Black's configuration