Merge pull request #310 from cclauss/patch-1

Check Darker code with `bandit`, `codespell`, `pyupgrade` and `safety`

.github/workflows/python-package.yml

@@ -81,3 +81,20 @@ jobs:
       - name: Test with pytest
         run: |
           pytest
+      - name: Bandit security audit for non-test code
+        run: bandit --recursive --exclude=./src/darker/tests .
+      - name: Bandit security audit for unit tests, allowing asserts
+        run: bandit --recursive --skip B101 ./src/darker/tests
+      - name: Check English spelling in the code base using codespell
+        run: codespell
+      - name: Ensure modern Python style using pyupgrade
+        # This script is written in a Linux / macos / windows portable way
+        run: |
+          python -c "
+          from pyupgrade._main import main
+          from glob import glob
+          files = glob('**/*.py', recursive=True)
+          main(files + ['--py36-plus'])
+          "
+      - name: Check dependencies for known security vulterabilities using Safety
+        run: safety check

action/main.py

@@ -2,7 +2,7 @@
 import shlex
 import sys
 from pathlib import Path
-from subprocess import run, PIPE, STDOUT
+from subprocess import PIPE, STDOUT, run  # nosec
 
 ACTION_PATH = Path(os.environ["GITHUB_ACTION_PATH"])
 ENV_PATH = ACTION_PATH / ".darker-env"
@@ -19,8 +19,9 @@
 req = "darker[isort]"
 if VERSION:
     req += f"=={VERSION}"
-pip_proc = run(
+pip_proc = run(  # nosec
     [str(ENV_BIN / "python"), "-m", "pip", "install", req],
+    check=False,
     stdout=PIPE,
     stderr=STDOUT,
     encoding="utf-8",
@@ -32,8 +33,9 @@
 
 
 base_cmd = [str(ENV_BIN / "darker")]
-proc = run(
-    [*base_cmd, *shlex.split(OPTIONS), "--revision", REVISION, *shlex.split(SRC)]
+proc = run(  # nosec
+    [*base_cmd, *shlex.split(OPTIONS), "--revision", REVISION, *shlex.split(SRC)],
+    check=False,
 )
 
 sys.exit(proc.returncode)

constraints-oldest.txt

@@ -3,12 +3,19 @@
 # still works against oldest supported versions of both the Python
 # interpreter and Python ependencies. Keep this up-to-date with minimum
 # versions in `setup.cfg`.
+bandit==1.7.1
 black==21.8b0
+codespell==2.1.0
+flake8-2020==1.6.1
+flake8-bugbear==22.1.11
+flake8-comprehensions==3.7.0
 mypy==0.931
 pytest==6.1.0
 pytest-flake8==1.0.6
 pytest-isort==1.1.0
 pytest-kwparametrize==0.0.3
+pyupgrade==2.31.0
 regex==2021.4.4
+safety==1.10.3
 toml==0.10.0
 types-toml==0.10.4

setup.cfg

@@ -50,18 +50,25 @@ isort =
     isort>=5.0.1
 test =
     # NOTE: remember to keep `constraints-oldest.txt` in sync with these
+    bandit>=1.7.1
     black>=21.7b1  # to prevent Mypy error about `gen_python_files`, see issue #189
+    codespell>=2.1.0
     flake8<4
+    flake8-2020>=1.6.1
+    flake8-bugbear>=22.1.11
+    flake8-comprehensions>=3.7.0
     mypy>=0.931
+    pygments
     pylint
     pytest>=6.1.0
     pytest-darker
     pytest-flake8>=1.0.6
     pytest-isort>=1.1.0
     pytest-kwparametrize>=0.0.3
     pytest-mypy
-    pygments
+    pyupgrade>=2.31.0
     regex>=2021.4.4
+    safety>=1.10.3
     types-dataclasses ; python_version < "3.7"
     types-toml>=0.10.4
 
@@ -70,7 +77,13 @@ test =
 max-line-length = 88
 # Ignore rules which conflict with Black
 ignore =
+    # C408 Unnecessary dict call - rewrite as a literal.
+    C408
     # E231 missing whitespace after ','
     E231
     # W503 line break before binary operator
     W503
+
+[codespell]
+ignore-words-list = nd
+skip = .git,*.json

src/darker/argparse_helpers.py

@@ -101,8 +101,6 @@ def __call__(
         values: Union[str, Sequence[Any], None],
         option_string: str = None,
     ) -> None:
-        assert isinstance(values, list)
-        assert all(isinstance(v, str) for v in values)
         current_level = getattr(namespace, self.dest, self.default)
         new_level = current_level + self.const
         new_level = max(new_level, logging.DEBUG)

src/darker/command_line.py

@@ -1,7 +1,7 @@
 """Command line parsing for the ``darker`` binary"""
 
 from argparse import SUPPRESS, ArgumentParser, Namespace
-from typing import Any, List, Optional, Text, Tuple
+from typing import Any, List, Optional, Tuple
 
 from darker import help as hlp
 from darker.argparse_helpers import (
@@ -31,7 +31,7 @@ def make_argument_parser(require_src: bool) -> ArgumentParser:
     )
     parser.register("action", "log_level", LogLevelAction)
 
-    def add_arg(help_text: Optional[Text], *name_or_flags: Text, **kwargs: Any) -> None:
+    def add_arg(help_text: Optional[str], *name_or_flags: str, **kwargs: Any) -> None:
         kwargs["help"] = help_text
         parser.add_argument(*name_or_flags, **kwargs)
 

src/darker/diff.py

@@ -101,10 +101,11 @@ def diff_and_get_opcodes(
 
 def _validate_opcodes(opcodes: List[Tuple[str, int, int, int, int]]) -> None:
     """Make sure every other opcode is an 'equal' tag"""
-    assert all(
+    if not all(
         (tag1 == "equal") != (tag2 == "equal")
         for (tag1, _, _, _, _), (tag2, _, _, _, _) in zip(opcodes[:-1], opcodes[1:])
-    ), opcodes
+    ):
+        raise ValueError(f"Unexpected opcodes in {opcodes!r}")
 
 
 def opcodes_to_edit_linenums(
@@ -147,5 +148,5 @@ 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]
+    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]

src/darker/git.py

@@ -8,7 +8,7 @@
 from datetime import datetime
 from functools import lru_cache
 from pathlib import Path
-from subprocess import DEVNULL, PIPE, CalledProcessError, check_output, run
+from subprocess import DEVNULL, PIPE, CalledProcessError, check_output, run  # nosec
 from typing import Dict, Iterable, List, Set, Tuple
 
 from darker.diff import diff_and_get_opcodes, opcodes_to_edit_linenums
@@ -69,9 +69,10 @@ def git_get_content_at_revision(path: Path, revision: str, cwd: Path) -> TextDoc
     :param cwd: The root of the Git repository
 
     """
-    assert (
-        not path.is_absolute()
-    ), f"the 'path' parameter must receive a relative path, got {path!r} instead"
+    if path.is_absolute():
+        raise ValueError(
+            f"the 'path' parameter must receive a relative path, got {path!r} instead"
+        )
 
     if revision == WORKTREE:
         abspath = cwd / path
@@ -208,7 +209,7 @@ def _git_check_output_lines(
     """Log command line, run Git, split stdout to lines, exit with 123 on error"""
     logger.debug("[%s]$ git %s", cwd, " ".join(cmd))
     try:
-        return check_output(
+        return check_output(  # nosec
             ["git"] + cmd,
             cwd=str(cwd),
             encoding="utf-8",
@@ -243,7 +244,7 @@ def _git_exists_in_revision(path: Path, rev2: str, cwd: Path) -> bool:
     # separators in paths. We need to use Posix paths and forward slashes instead.
     cmd = ["git", "cat-file", "-e", f"{rev2}:{path.as_posix()}"]
     logger.debug("[%s]$ %s", cwd, " ".join(cmd))
-    result = run(
+    result = run(  # nosec
         cmd,
         cwd=str(cwd),
         check=False,

src/darker/import_sorting.py

@@ -19,7 +19,7 @@
     # Work around Mypy problem
     # https://github.com/python/mypy/issues/7030#issuecomment-504128883
     try:
-        isort_code = getattr(isort, "code")
+        isort_code = getattr(isort, "code")  # noqa: B009
     except AttributeError:
         # Postpone error message about incompatbile `isort` version until `--isort` is
         # actually used.

src/darker/linting.py

@@ -22,7 +22,7 @@
 import logging
 from contextlib import contextmanager
 from pathlib import Path
-from subprocess import PIPE, Popen
+from subprocess import PIPE, Popen  # nosec
 from typing import IO, Generator, List, Set, Tuple
 
 from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange
@@ -47,11 +47,11 @@ def _parse_linter_line(line: str, root: Path) -> Tuple[Path, int, str, str]:
             # Make sure it column looks like an int on "<path>:<linenum>:<column>"
             _column = int(rest[0])  # noqa: F841
     except ValueError:
-        # Encountered a non-parseable line which doesn't express a linting error.
+        # Encountered a non-parsable line which doesn't express a linting error.
         # For example, on Mypy:
         # "Found XX errors in YY files (checked ZZ source files)"
         # "Success: no issues found in 1 source file"
-        logger.debug("Unparseable linter output: %s", line[:-1])
+        logger.debug("Unparsable linter output: %s", line[:-1])
         return Path(), 0, "", ""
     path_from_cwd = Path(path_str).absolute()
     path_in_repo = path_from_cwd.relative_to(root)
@@ -85,13 +85,14 @@ def _check_linter_output(
     :return: The standard output stream of the linter subprocess
 
     """
-    with Popen(
+    with Popen(  # nosec
         cmdline.split() + [str(root / path) for path in sorted(paths)],
         stdout=PIPE,
         encoding="utf-8",
     ) as linter_process:
-        # assert needed for MyPy (see https://stackoverflow.com/q/57350490/15770)
-        assert linter_process.stdout is not None
+        # condition needed for MyPy (see https://stackoverflow.com/q/57350490/15770)
+        if linter_process.stdout is None:
+            raise RuntimeError("Stdout piping failed")
         yield linter_process.stdout
 
 

src/darker/tests/conftest.py

@@ -2,7 +2,7 @@
 
 import os
 from pathlib import Path
-from subprocess import check_call
+from subprocess import check_call  # nosec
 from typing import Dict, Optional
 
 import pytest
@@ -32,7 +32,7 @@ def create_repository(cls, root: Path) -> "GitRepoFixture":
 
     def _run(self, *args: str) -> None:
         """Helper method to run a Git command line in the repository root"""
-        check_call(["git"] + list(args), cwd=self.root, env=self.env)
+        check_call(["git"] + list(args), cwd=self.root, env=self.env)  # nosec
 
     def _run_and_get_first_line(self, *args: str) -> str:
         """Helper method to run Git in repo root and return first line of output"""

src/darker/tests/test_difflib.py

@@ -11,7 +11,7 @@
 
 
 def test_sequencematcher():
-    """``SequenceMatcher`` detects a single changed line in between correcly"""
+    """``SequenceMatcher`` detects a single changed line in between correctly"""
     matcher = SequenceMatcher(
         None, ORIGINAL.splitlines(), CHANGED.splitlines(), autojunk=False
     )

src/darker/tests/test_git.py

@@ -6,7 +6,7 @@
 import re
 from datetime import datetime, timedelta
 from pathlib import Path
-from subprocess import DEVNULL, PIPE, CalledProcessError, check_call
+from subprocess import DEVNULL, PIPE, CalledProcessError, check_call  # nosec
 from typing import List, Union
 from unittest.mock import call, patch
 
@@ -810,7 +810,7 @@ def test_local_gitconfig_ignored_by_gitrepofixture(tmp_path):
     # Note: once we decide to drop support for git < 2.28, the HEAD file
     # creation above can be removed, and setup can simplify to
     # check_call("git config --global init.defaultBranch main".split())
-    check_call(
+    check_call(  # nosec
         "git config --global init.templateDir".split() + [str(tmp_path)],
         env={"HOME": str(tmp_path), "PATH": os.environ["PATH"]},
     )

src/darker/tests/test_linting.py

@@ -2,6 +2,7 @@
 
 """Unit tests for :mod:`darker.linting`"""
 
+import re
 from pathlib import Path
 from textwrap import dedent
 from unittest.mock import call, patch
@@ -70,14 +71,14 @@ def test_check_linter_output():
         _descr="Check one file, report on a modified line in test.py",
         paths=["one.py"],
         location="test.py:1:",
-        expect_output=["", "test.py:1: {git_repo.root / 'one.py'}"],
+        expect_output=["", "test.py:1: {root/one.py}"],
         expect_log=[],
     ),
     dict(
         _descr="Check one file, report on a column of a modified line in test.py",
         paths=["one.py"],
         location="test.py:1:42:",
-        expect_output=["", "test.py:1:42: {git_repo.root / 'one.py'}"],
+        expect_output=["", "test.py:1:42: {root/one.py}"],
         expect_log=[],
     ),
     dict(
@@ -98,20 +99,14 @@ def test_check_linter_output():
         _descr="Check two files, report on a modified line in test.py",
         paths=["one.py", "two.py"],
         location="test.py:1:",
-        expect_output=[
-            "",
-            "test.py:1: {git_repo.root / 'one.py'} {git_repo.root / 'two.py'}"
-        ],
+        expect_output=["", "test.py:1: {root/one.py} {root/two.py}"],
         expect_log=[],
     ),
     dict(
         _descr="Check two files, rpeort on a column of a modified line in test.py",
         paths=["one.py", "two.py"],
         location="test.py:1:42:",
-        expect_output=[
-            "",
-            "test.py:1:42: {git_repo.root / 'one.py'} {git_repo.root / 'two.py'}"
-        ],
+        expect_output=["", "test.py:1:42: {root/one.py} {root/two.py}"],
         expect_log=[],
     ),
     dict(
@@ -163,10 +158,9 @@ def test_run_linter(
     # by checking standard output from the our `echo` "linter".
     # The test cases also verify that only linter reports on modified lines are output.
     result = capsys.readouterr().out.splitlines()
-    # Use evil `eval()` so we get Windows compatible expected paths:
-    # pylint: disable=eval-used
     assert result == [
-        eval(f'f"{line}"', {"git_repo": git_repo}) for line in expect_output
+        re.sub(r"\{root/(.*?)\}", lambda m: str(git_repo.root / m.group(1)), line)
+        for line in expect_output
     ]
     logs = [f"{record.levelname} {record.message}" for record in caplog.records]
     assert logs == expect_log

src/darker/utils.py

@@ -225,7 +225,8 @@ def __iter__(self) -> "Buf":
         return self
 
     def seek_line(self, lines_delta: int) -> None:
-        assert lines_delta <= 0
+        if lines_delta > 0:
+            raise NotImplementedError("Seeking forwards is not supported")
         for _ in range(-lines_delta):
             self._buf.seek(self._line_starts.pop())