Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isort only if imports at the top of the module were edited #238

Merged
merged 6 commits into from
Apr 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ Added
- The ``--workers``/``-W`` option now specifies how many Darker jobs are used to
process files in parallel to complete reformatting/linting faster.
- Linters can now be installed and run in the GitHub Action using the ``lint:`` option.

- Sort imports only if the range of modified lines overlaps with changes resulting from
sorting the imports.

Fixed
-----
- Avoid memory leak from using ``@lru_cache`` on a method.
Expand Down
94 changes: 56 additions & 38 deletions src/darker/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
from darker.command_line import parse_command_line
from darker.concurrency import get_executor
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,
WORKTREE,
EditedLinenumsDiffer,
RevisionRange,
get_missing_at_revision,
get_rev1_path,
get_path_in_repo,
git_get_content_at_revision,
git_get_modified_python_files,
git_is_repository,
Expand Down Expand Up @@ -80,63 +80,77 @@ def format_edited_parts(
with get_executor(max_workers=workers) as executor:
# pylint: disable=unsubscriptable-object
futures: List[concurrent.futures.Future[ProcessedDocument]] = []
for path_in_repo in sorted(changed_files):
edited_linenums_differ = EditedLinenumsDiffer(root, revrange)
for relative_path_in_rev2 in sorted(changed_files):
future = executor.submit(
_isort_and_blacken_single_file,
root,
path_in_repo,
relative_path_in_rev2,
edited_linenums_differ,
black_exclude,
revrange,
black_config,
enable_isort,
enable_black=path_in_repo not in black_exclude,
black_config,
)
futures.append(future)

for future in concurrent.futures.as_completed(futures):
src, rev2_content, content_after_reformatting = future.result()
(
absolute_path_in_rev2,
rev2_content,
content_after_reformatting,
) = future.result()
if report_unmodified or content_after_reformatting != rev2_content:
yield (src, rev2_content, content_after_reformatting)
yield (absolute_path_in_rev2, rev2_content, content_after_reformatting)


def _isort_and_blacken_single_file( # pylint: disable=too-many-arguments
root: Path,
relative_path: Path,
relative_path_in_rev2: Path,
edited_linenums_differ: EditedLinenumsDiffer,
black_exclude: Collection[Path], # pylint: disable=unsubscriptable-object
revrange: RevisionRange,
black_config: BlackConfig,
enable_isort: bool,
enable_black: bool,
black_config: BlackConfig,
) -> ProcessedDocument:
"""Black and/or isort formatting for modified chunks in a single file

:param root: Root directory for the relative path
:param relative_path: Relative path to a Python source code file
:param relative_path_in_rev2: Relative path to a Python source code file
:param black_exclude: Python files to not reformat using Black, according to Black
configuration
:param revrange: The Git revisions to compare
:param black_config: Configuration to use for running Black
:param enable_isort: ``True`` to run ``isort`` first on the file contents
:param enable_black: ``True`` to also run ``black`` on the file contents
:param black_config: Configuration to use for running Black
:return: Details about changes for the file

"""
src = root / relative_path
rev2_content = git_get_content_at_revision(relative_path, revrange.rev2, root)
# With VSCode, `relative_path_in_rev2` may be a `.py.<HASH>.tmp` file in the
# working tree instead of a `.py` file.
absolute_path_in_rev2 = root / relative_path_in_rev2
rev2_content = git_get_content_at_revision(
relative_path_in_rev2, revrange.rev2, root
)
# 1. run isort
if enable_isort:
rev2_isorted = apply_isort(
rev2_content,
src,
relative_path_in_rev2,
edited_linenums_differ,
black_config.get("config"),
black_config.get("line_length"),
)
else:
rev2_isorted = rev2_content
if enable_black:
if relative_path_in_rev2 not in black_exclude:
# 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 = _blacken_single_file(
root,
relative_path,
revrange,
relative_path_in_rev2,
get_path_in_repo(relative_path_in_rev2),
edited_linenums_differ,
rev2_content,
rev2_isorted,
enable_isort,
Expand All @@ -145,23 +159,27 @@ def _isort_and_blacken_single_file( # pylint: disable=too-many-arguments
else:
# File was excluded by Black configuration, don't reformat
content_after_reformatting = rev2_isorted
return src, rev2_content, content_after_reformatting
return absolute_path_in_rev2, rev2_content, content_after_reformatting


def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
root: Path,
relative_path: Path,
revrange: RevisionRange,
relative_path_in_rev2: Path,
relative_path_in_repo: Path,
edited_linenums_differ: EditedLinenumsDiffer,
rev2_content: TextDocument,
rev2_isorted: TextDocument,
enable_isort: bool,
black_config: BlackConfig,
) -> TextDocument:
"""In a Python file, reformat chunks with edits since the last commit using Black

:param root: Root directory for the relative path
:param relative_path: Relative path to a Python source code file
:param revrange: The Git revisions to compare
:param root: The common root of all files to reformat
:param relative_path_in_rev2: Relative path to a Python source code file. Possibly a
VSCode ``.py.<HASH>.tmp`` file in the working tree.
:param relative_path_in_repo: Relative path to source in the Git repository. Same as
``relative_path_in_rev2`` save for VSCode temp files.
:param edited_linenums_differ: Helper for finding out which lines were edited
: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
Expand All @@ -170,20 +188,20 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
:raise: NotEquivalentError

"""
src = root / relative_path
rev1_relative_path = get_rev1_path(relative_path)
edited_linenums_differ = EditedLinenumsDiffer(root, revrange)
absolute_path_in_rev2 = root / relative_path_in_rev2

# 4. run black
formatted = run_black(rev2_isorted, black_config)
logger.debug("Read %s lines from edited file %s", len(rev2_isorted.lines), src)
logger.debug(
"Read %s lines from edited file %s",
len(rev2_isorted.lines),
absolute_path_in_rev2,
)
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 All @@ -201,15 +219,15 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
logger.debug(
"Trying with %s lines of context for `git diff -U %s`",
context_lines,
src,
absolute_path_in_rev2,
)
# 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(
rev1_relative_path, rev2_isorted, context_lines
relative_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)
logger.debug("No changes in %s after isort", absolute_path_in_rev2)
last_successful_reformat = rev2_isorted
break

Expand All @@ -232,15 +250,15 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
debug_dump(black_chunks, edited_linenums)
logger.debug(
"AST verification of %s with %s lines of context failed",
src,
absolute_path_in_rev2,
context_lines,
)
minimum_context_lines.respond(False)
else:
minimum_context_lines.respond(True)
last_successful_reformat = chosen
if not last_successful_reformat:
raise NotEquivalentError(relative_path)
raise NotEquivalentError(relative_path_in_rev2)
return last_successful_reformat


Expand Down
26 changes: 26 additions & 0 deletions src/darker/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,29 @@ def opcodes_to_chunks(
_validate_opcodes(opcodes)
for _tag, src_start, src_end, dst_start, dst_end in opcodes:
yield src_start + 1, src.lines[src_start:src_end], dst.lines[dst_start:dst_end]


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))
4 changes: 2 additions & 2 deletions src/darker/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def _with_common_ancestor(cls, rev1: str, rev2: str, cwd: Path) -> "RevisionRang
return cls(rev1 if common_ancestor == rev1_hash else common_ancestor, rev2)


def get_rev1_path(path: Path) -> Path:
def get_path_in_repo(path: Path) -> Path:
"""Return the relative path to the file in the old revision

This is usually the same as the relative path on the command line. But in the
Expand All @@ -188,7 +188,7 @@ def get_rev1_path(path: Path) -> Path:


def should_reformat_file(path: Path) -> bool:
return path.exists() and get_rev1_path(path).suffix == ".py"
return path.exists() and get_path_in_repo(path).suffix == ".py"


@lru_cache(maxsize=1)
Expand Down
Loading