Skip to content

Commit

Permalink
isort only if the changes overlap modifications
Browse files Browse the repository at this point in the history
  • Loading branch information
akaihola committed Feb 6, 2022
1 parent 75cb66b commit d35eb39
Show file tree
Hide file tree
Showing 4 changed files with 238 additions and 27 deletions.
12 changes: 6 additions & 6 deletions src/darker/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from darker.chooser import choose_lines
from darker.command_line import parse_command_line
from darker.config import OutputMode, dump_config
from darker.diff import diff_and_get_opcodes, opcodes_to_chunks
from darker.diff import diff_chunks
from darker.exceptions import DependencyError, MissingPackageError
from darker.git import (
PRE_COMMIT_FROM_TO_REFS,
Expand Down Expand Up @@ -70,6 +70,7 @@ def format_edited_parts(
be reformatted, and skips unchanged files.
"""
edited_linenums_differ = EditedLinenumsDiffer(root, revrange)
for relative_path_in_rev2 in sorted(changed_files):
# With VSCode, `relative_path_in_rev2` may be a `.py.<HASH>.tmp` file in the
# working tree insted of a `.py` file.
Expand All @@ -82,7 +83,8 @@ def format_edited_parts(
if enable_isort:
rev2_isorted = apply_isort(
rev2_content,
absolute_path_in_rev2,
relative_path_in_rev2,
edited_linenums_differ,
black_config.get("config"),
black_config.get("line_length"),
)
Expand All @@ -96,7 +98,7 @@ def format_edited_parts(
root,
relative_path_in_rev2,
_get_path_in_repo(relative_path_in_rev2),
EditedLinenumsDiffer(root, revrange),
edited_linenums_differ,
rev2_content,
rev2_isorted,
enable_isort,
Expand Down Expand Up @@ -147,10 +149,8 @@ def _reformat_single_file( # pylint: disable=too-many-arguments,too-many-locals
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))
black_chunks = diff_chunks(rev2_isorted, formatted)

# Exit early if nothing to do
if not black_chunks:
Expand Down
26 changes: 26 additions & 0 deletions src/darker/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,29 @@ def opcodes_to_chunks(
_validate_opcodes(opcodes)
for tag, i1, i2, j1, j2 in opcodes:
yield i1 + 1, src.lines[i1:i2], dst.lines[j1:j2]


def diff_chunks(src: TextDocument, dst: TextDocument) -> List[DiffChunk]:
"""Diff two documents and return the list of chunks in the diff
Each chunk is a 3-tuple::
(
linenum: int,
old_lines: List[str],
new_lines: List[str],
)
``old_lines`` and ``new_lines`` may be
- identical to indicate a chunk with no changes,
- of the same length but different items to indicate some modified lines, or
- of different lengths to indicate removed or inserted lines.
For the return value ``retval``, the following always holds::
retval[n + 1][0] == retval[n][0] + len(retval[n][old_lines])
"""
opcodes = diff_and_get_opcodes(src, dst)
return list(opcodes_to_chunks(opcodes, src, dst))
102 changes: 93 additions & 9 deletions src/darker/import_sorting.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging
import sys
from pathlib import Path
from typing import Any, Optional
from typing import Any, List, Optional

from darker.black_compat import find_project_root
from darker.diff import diff_chunks
from darker.exceptions import IncompatiblePackageError, MissingPackageError
from darker.utils import TextDocument
from darker.git import EditedLinenumsDiffer
from darker.utils import DiffChunk, TextDocument

if sys.version_info >= (3, 8):
from typing import TypedDict
Expand Down Expand Up @@ -55,31 +57,113 @@ class IsortArgs(TypedDict, total=False):
def apply_isort(
content: TextDocument,
src: Path,
edited_linenums_differ: EditedLinenumsDiffer,
config: Optional[str] = None,
line_length: Optional[int] = None,
) -> TextDocument:
isort_args = IsortArgs()
"""Run isort on the given Python source file content
:param content: The contents of the Python source code file to sort imports in
:param src: The relative path to the file. This must be the actual path in the
repository, which may differ from the path given on the command line in
case of VSCode temporary files.
:param edited_linenums_differ: Helper for finding out which lines were edited
:param config: Path to configuration file
:param line_length: Maximum line length to use
"""
edited_linenums = edited_linenums_differ.revision_vs_lines(
src,
content,
context_lines=0,
)
if not edited_linenums:
return content
isort_args = _build_isort_args(src, config, line_length)
rev2_isorted = _call_isort_code(content, isort_args)
# Get the chunks in the diff between the edited and import-sorted file
isort_chunks = diff_chunks(content, rev2_isorted)
if not isort_chunks:
# No imports were sorted. Return original content.
return content
if not _diff_overlaps_with_edits(edited_linenums, isort_chunks):
# No lines had been modified in the range of modified import lines. Return
# original content.
return content
# The range lines modified by sorted imports overlaps with user modifications in the
# code. Return the import-sorted file.
return rev2_isorted


def _build_isort_args(
src: Path,
config: Optional[str] = None,
line_length: Optional[int] = None,
) -> IsortArgs:
"""Build ``isort.code()`` keyword arguments
:param src: The relative path to the file. This must be the actual path in the
repository, which may differ from the path given on the command line in
case of VSCode temporary files.
:param config: Path to configuration file
:param line_length: Maximum line length to use
"""
isort_args: IsortArgs = {}
if config:
isort_args["settings_file"] = config
else:
isort_args["settings_path"] = str(find_project_root((str(src),)))
if line_length:
isort_args["line_length"] = line_length
return isort_args

logger.debug(
"isort.code(code=..., {})".format(
", ".join(f"{k}={v!r}" for k, v in isort_args.items())
)
)

def _call_isort_code(content: TextDocument, isort_args: IsortArgs) -> TextDocument:
"""Call ``isort.code()`` and return the result as a `TextDocument` object
:param content: The contents of the Python source code file to sort imports in
:param isort_args: Keyword arguments for ``isort.code()``
"""
code = content.string
logger.debug(
"isort.code(code=..., %s)",
", ".join(f"{k}={v!r}" for k, v in isort_args.items()),
)
try:
code = isort_code(code=code, **isort_args)
except isort.exceptions.FileSkipComment:
pass

return TextDocument.from_str(
code,
encoding=content.encoding,
mtime=content.mtime,
)


def _diff_overlaps_with_edits(
edited_linenums: List[int], isort_chunks: List[DiffChunk]
) -> bool:
"""Return ``True`` if the complete diff overlaps the range of edited lines
:param edited_linenums: The line numbers of all edited lines
:param isort_chunks: The diff chunks
:return: ``True`` if the two overlap
"""
if not edited_linenums:
return False
first_edited_linenum, last_edited_linenum = edited_linenums[0], edited_linenums[-1]
modified_chunks = [
(linenum, old, new) for linenum, old, new in isort_chunks if old != new
]
if not modified_chunks:
return False
(first_isort_line, _, _) = modified_chunks[0]
(last_isort_chunk_start, last_isort_chunk_original_lines, _) = modified_chunks[-1]
last_isort_line = last_isort_chunk_start + len(last_isort_chunk_original_lines)
return (
first_edited_linenum < last_isort_line
and last_edited_linenum >= first_isort_line
)
125 changes: 113 additions & 12 deletions src/darker/tests/test_import_sorting.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Tests for :mod:`darker.import_sorting`"""

# pylint: disable=unused-argument
# pylint: disable=unused-argument,protected-access

from importlib import reload
from pathlib import Path
Expand All @@ -9,11 +9,12 @@
import pytest

import darker.import_sorting
from darker.git import EditedLinenumsDiffer, RevisionRange
from darker.tests.helpers import isort_present
from darker.utils import TextDocument
from darker.utils import TextDocument, joinlines

ORIGINAL_SOURCE = ("import sys", "import os")
ISORTED_SOURCE = ("import os", "import sys")
ORIGINAL_SOURCE = ("import sys", "import os", "", "print(42)")
ISORTED_SOURCE = ("import os", "import sys", "", "print(42)")


@pytest.mark.parametrize("present", [True, False])
Expand All @@ -31,14 +32,32 @@ def test_import_sorting_importable_with_and_without_isort(present):

@pytest.mark.parametrize("encoding", ["utf-8", "iso-8859-1"])
@pytest.mark.parametrize("newline", ["\n", "\r\n"])
def test_apply_isort(encoding, newline):
"""Import sorting is applied correctly, with encoding and newline intact"""
result = darker.import_sorting.apply_isort(
TextDocument.from_lines(ORIGINAL_SOURCE, encoding=encoding, newline=newline),
Path("test1.py"),
@pytest.mark.kwparametrize(
dict(content=ORIGINAL_SOURCE, expect=ORIGINAL_SOURCE),
dict(content=("import sys", "import os"), expect=("import sys", "import os")),
dict(
content=("import sys", "import os", "# foo", "print(42)"),
expect=("import sys", "import os", "# foo", "print(42)"),
),
dict(
content=("import sys", "import os", "", "print(43)"),
expect=("import sys", "import os", "", "print(43)"),
),
dict(content=("import sys", "import os", "", "print(42)"), expect=ISORTED_SOURCE),
dict(content=("import sys", "import os", "", "print(42)"), expect=ISORTED_SOURCE),
)
def test_apply_isort(git_repo, encoding, newline, content, expect):
"""Imports are sorted if edits overlap them, with encoding and newline intact"""
git_repo.add({"test1.py": joinlines(ORIGINAL_SOURCE, newline)}, commit="Initial")
edited_linenums_differ = EditedLinenumsDiffer(
git_repo.root, RevisionRange("HEAD", ":WORKTREE:")
)
src = Path("test1.py")
content_ = TextDocument.from_lines(content, encoding=encoding, newline=newline)

result = darker.import_sorting.apply_isort(content_, src, edited_linenums_differ)

assert result.lines == ISORTED_SOURCE
assert result.lines == expect
assert result.encoding == encoding
assert result.newline == newline

Expand Down Expand Up @@ -100,18 +119,100 @@ def test_isort_config(
config = str(tmpdir / settings_file) if settings_file else None

actual = darker.import_sorting.apply_isort(
TextDocument.from_str(content), Path("test1.py"), config
TextDocument.from_str(content),
Path("test1.py"),
EditedLinenumsDiffer(Path("."), RevisionRange("master", "HEAD")),
config,
)
assert actual.string == expect


@pytest.mark.kwparametrize(
dict(src=Path("file.py"), expect={"settings_path": "{cwd}"}),
dict(
config="myconfig.toml",
expect={"settings_file": "myconfig.toml"},
),
dict(line_length=42, expect={"settings_path": "{cwd}", "line_length": 42}),
src=Path("file.py"),
config=None,
line_length=None,
)
def test_build_isort_args(src, config, line_length, expect):
"""``_build_isort_args`` returns correct arguments for isort"""
result = darker.import_sorting._build_isort_args(src, config, line_length)

if "settings_path" in expect:
expect["settings_path"] = str(expect["settings_path"].format(cwd=Path.cwd()))
assert result == expect


def test_isort_file_skip_comment():
"""``apply_isort()`` handles ``FileSkipComment`` exception correctly"""
# Avoid https://github.com/PyCQA/isort/pull/1833 by splitting the skip string
content = "# iso" + "rt:skip_file"

actual = darker.import_sorting.apply_isort(
TextDocument.from_str(content), Path("test1.py")
TextDocument.from_str(content),
Path("test1.py"),
EditedLinenumsDiffer(Path("."), RevisionRange("master", "HEAD")),
)

assert actual.string == content


@pytest.mark.kwparametrize(
dict(edited_linenums=[], isort_chunks=[], expect=False),
dict(edited_linenums=[1, 2, 3, 4, 5, 6, 7, 8, 9], isort_chunks=[], expect=False),
dict(edited_linenums=[], isort_chunks=[(1, ("a", "b"), ("A", "B"))], expect=False),
dict(edited_linenums=[1], isort_chunks=[(1, ("a", "b"), ("A", "B"))], expect=True),
dict(edited_linenums=[2], isort_chunks=[(1, ("a", "b"), ("A", "B"))], expect=True),
dict(edited_linenums=[3], isort_chunks=[(1, ("a", "b"), ("A", "B"))], expect=False),
dict(edited_linenums=[], isort_chunks=[(1, ("A", "B"), ("A", "B"))], expect=False),
dict(edited_linenums=[1], isort_chunks=[(1, ("A", "B"), ("A", "B"))], expect=False),
dict(edited_linenums=[2], isort_chunks=[(1, ("A", "B"), ("A", "B"))], expect=False),
dict(edited_linenums=[3], isort_chunks=[(1, ("A", "B"), ("A", "B"))], expect=False),
dict(
edited_linenums=[3, 9],
isort_chunks=[
(1, ("a", "b"), ("A", "B")),
(3, ("c", "d", "e", "f", "g"), ("c", "d", "e", "f", "g")),
(8, ("h", "i", "j"), ("h", "i", "j")),
],
expect=False,
),
dict(
edited_linenums=[3, 9],
isort_chunks=[
(1, ("a", "b", "c"), ("A", "B", "C")),
(4, ("d", "e", "f", "g"), ("d", "e", "f", "g")),
(8, ("h", "i", "j"), ("h", "i", "j")),
],
expect=True,
),
dict(
edited_linenums=[3, 9],
isort_chunks=[
(1, ("a", "b", "c"), ("a", "b", "c")),
(4, ("d", "e", "f", "g"), ("d", "e", "f", "g")),
(8, ("h", "i", "j"), ("H", "I", "J")),
],
expect=True,
),
dict(
edited_linenums=[3, 9],
isort_chunks=[
(1, ("a", "b", "c", "d"), ("a", "b", "c", "d")),
(5, ("e", "f", "g", "h", "i"), ("e", "f", "g", "h", "i")),
(10, ("j"), ("J")),
],
expect=False,
),
)
def test_diff_overlaps_with_edits(edited_linenums, isort_chunks, expect):
"""Overlapping edits and import sortings are detected correctly"""
result = darker.import_sorting._diff_overlaps_with_edits(
edited_linenums, isort_chunks
)

assert result == expect

0 comments on commit d35eb39

Please sign in to comment.