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

Split/join command lines using shlex from Python stdlib #455

Merged
merged 6 commits into from
Jan 13, 2023
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
10 changes: 9 additions & 1 deletion .github/workflows/python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,17 @@ jobs:
if: matrix.constraints == '--constraint constraints-oldest.txt'
run: |
sed -i 's/py311/py39/' pyproject.toml
- name: Run Pytest
- name: Run Pytest with the Darker plugin on recent Black versions
if: matrix.constraints != '--constraint constraints-oldest.txt'
run: |
pytest --darker
- name: Run Pytest without the Darker plugin on oldest Black version
# The reformatting rules used to be a bit different. We don't need to
# test reformatting Darker's own code base with old Black versions.
# Interoperability is ensured by unit tests.
if: matrix.constraints == '--constraint constraints-oldest.txt'
run: |
pytest

build-sdist-validate-dists:
runs-on: ubuntu-latest
Expand Down
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Fixed
- Pass Git errors to stderr correctly both in raw and encoded subprocess output mode.
- Add a work-around for cleaning up temporary directories. Needed for Python 3.7 on
Windows.
- Split and join command lines using ``shlex`` from the Python standard library. This
deals with quoting correctly.


1.6.1_ - 2022-12-28
Expand Down
15 changes: 10 additions & 5 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -762,12 +762,16 @@ Most notably, the following linters/checkers have been verified to work with Dar

*New in version 1.2.0:* Support for test coverage output using `cov_to_lint.py`_.

To run a linter, use the ``--lint`` / ``-L`` command line option:
To run a linter, use the ``--lint`` / ``-L`` command line option with the linter
command or a full command line to pass to a linter. Some examples:

- ``-L mypy``: do static type checking using Mypy_
- ``-L pylint``: analyze code using Pylint_
- ``-L flake8``: enforce the Python style guide using Flake8_
- ``-L cov_to_lint.py``: read ``.coverage`` and list non-covered modified lines
- ``-L flake8``: enforce the Python style guide using Flake8_
- ``-L "mypy --strict"``: do static type checking using Mypy_
- ``--lint="pylint --ignore='setup.py'""``: analyze code using Pylint_
- ``-L cov_to_lint.py``: read ``.coverage`` and list non-covered modified lines

**Note:** Full command lines aren't fully tested on Windows. See issue `#456`_ for a
possible bug.

Darker also groups linter output into blocks of consecutive lines
separated by blank lines.
Expand Down Expand Up @@ -795,6 +799,7 @@ Here's an example of `cov_to_lint.py`_ output::
.. _Pylint: https://pypi.org/project/pylint
.. _Flake8: https://pypi.org/project/flake8
.. _cov_to_lint.py: https://gist.github.com/akaihola/2511fe7d2f29f219cb995649afd3d8d2
.. _#456: https://github.com/akaihola/darker/issues/456


Syntax highlighting
Expand Down
17 changes: 16 additions & 1 deletion src/darker/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging
import os
import re
import shlex
import sys
from dataclasses import dataclass
from datetime import datetime
Expand All @@ -26,6 +27,20 @@
from darker.multiline_strings import get_multiline_string_ranges
from darker.utils import GIT_DATEFORMAT, TextDocument

if sys.version_info < (3, 8):

def shlex_join(split_command: Iterable[str]) -> str:
"""Backport `shlex.join` for Python 3.7

:param split_command: The elements on the command line
:return: The command line as one string, with appropriate quoting

"""
return " ".join(shlex.quote(arg) for arg in split_command)

else:
shlex_join = shlex.join

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -277,7 +292,7 @@ def _git_check_output(
encoding: Optional[str] = None,
) -> Union[str, bytes]:
"""Log command line, run Git, return stdout, exit with 123 on error"""
logger.debug("[%s]$ git %s", cwd, " ".join(cmd))
logger.debug("[%s]$ git %s", cwd, shlex_join(cmd))
try:
return check_output( # nosec
["git"] + cmd,
Expand Down
10 changes: 7 additions & 3 deletions src/darker/linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
"""

import logging
import shlex
from contextlib import contextmanager
from pathlib import Path
from subprocess import PIPE, Popen # nosec
from typing import IO, Generator, List, Set, Tuple

from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange
from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange, shlex_join
from darker.highlighting import colorize
from darker.utils import WINDOWS

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -143,8 +145,10 @@ def _check_linter_output(
:return: The standard output stream of the linter subprocess

"""
cmdline_and_paths = cmdline.split() + [str(root / path) for path in sorted(paths)]
logger.debug("[%s]$ %s", Path.cwd(), " ".join(cmdline_and_paths))
cmdline_and_paths = shlex.split(cmdline, posix=not WINDOWS) + [
str(root / path) for path in sorted(paths)
]
logger.debug("[%s]$ %s", Path.cwd(), shlex_join(cmdline_and_paths))
with Popen( # nosec
cmdline_and_paths,
stdout=PIPE,
Expand Down
28 changes: 25 additions & 3 deletions src/darker/tests/test_linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,37 @@ def test_require_rev2_worktree(rev2, expect):
linting._require_rev2_worktree(rev2)


def test_check_linter_output():
@pytest.mark.kwparametrize(
dict(cmdline="echo", expect=["first.py the 2nd.py\n"]),
dict(cmdline="echo words before", expect=["words before first.py the 2nd.py\n"]),
dict(
cmdline='echo "two spaces"',
expect=["two spaces first.py the 2nd.py\n"],
marks=[
pytest.mark.xfail(
reason=(
"Quotes not removed on Windows."
" See https://github.com/akaihola/darker/issues/456"
)
)
]
if WINDOWS
else [],
),
dict(cmdline="echo eat spaces", expect=["eat spaces first.py the 2nd.py\n"]),
)
def test_check_linter_output(cmdline, expect):
"""``_check_linter_output()`` runs linter and returns the stdout stream"""
with linting._check_linter_output(
"echo", Path("root/of/repo"), {Path("first.py"), Path("second.py")}
cmdline, Path("root/of/repo"), {Path("first.py"), Path("the 2nd.py")}
) as stdout:
lines = list(stdout)

assert lines == [
f"{Path('root/of/repo/first.py')} {Path('root/of/repo/second.py')}\n"
line.replace("first.py", str(Path("root/of/repo/first.py"))).replace(
"the 2nd.py", str(Path("root/of/repo/the 2nd.py"))
)
for line in expect
]


Expand Down