Refactor TOML loading; BLK907 for invalid pyproject.toml

.flake8

@@ -19,6 +19,8 @@ per-file-ignores =
     tests/test_fail/mixed_tab_spaces.py: E101,E999,W191
     tests/with_pyproject_toml/ordinary_quotes.py: Q000
     tests/test_cases/mixed_tab_spaces.py: E101,E999,W191
+    # The bad TOML file breaks black checking this file:
+    tests/with_bad_toml/hello_world.py: BLK997,
 
 # =====================
 # flake-quote settings:

README.rst

@@ -50,6 +50,7 @@ BLK100 Black would make changes.
 BLK9## Internal error (*various, listed below*):
 BLK900 Failed to load file: ...
 BLK901 Invalid input.
+BLK997 Invalid TOML file: ...
 BLK998 Could not access flake8 line length setting (*no longer used*).
 BLK999 Unexpected exception.
 ====== =======================================================================
@@ -147,6 +148,7 @@ Version Release date   Changes
 v0.1.1  *pending*    - Option to use a (global) black configuration file,
                        contribution from
                        `Tomasz Grining <https://github.com/098799>`_.
+                     - New ``BLK997`` if can't parse ``pyproject.toml`` file.
                      - Logs configuration files, use ``-v`` or ``--verbose``.
                      - Fixed flake8 "builtins" parameter warning.
                      - Now requires black 19.3b0 or later.

flake8_black.py

@@ -3,7 +3,7 @@
 This is a plugin for the tool flake8 tool for checking Python
 soucre code using the tool black.
 """
-from functools import lru_cache
+
 from os import path
 from pathlib import Path
 
@@ -38,88 +38,145 @@ def find_diff_start(old_src, new_src):
     return min(len(old_lines), len(new_lines)), 0
 
 
+class BadBlackConfig(ValueError):
+    """Bad black TOML configuration file."""
+
+    pass
+
+
+def load_black_mode(toml_filename=None):
+    """Load a black configuration TOML file (or return defaults) as FileMode."""
+    if not toml_filename:
+        return black.FileMode(
+            target_versions=set(),
+            line_length=black.DEFAULT_LINE_LENGTH,  # Expect to be 88
+            string_normalization=True,
+        )
+
+    LOG.info("flake8-black: loading black settings from %s", toml_filename)
+    try:
+        pyproject_toml = toml.load(str(toml_filename))
+    except toml.decoder.TomlDecodeError:
+        LOG.info("flake8-black: invalid TOML file %s", toml_filename)
+        raise BadBlackConfig(path.relpath(toml_filename))
+    config = pyproject_toml.get("tool", {}).get("black", {})
+    black_config = {k.replace("--", "").replace("-", "_"): v for k, v in config.items()}
+
+    # Extract the fields we care about:
+    return black.FileMode(
+        target_versions={
+            black.TargetVersion[val.upper()]
+            for val in black_config.get("target_version", [])
+        },
+        line_length=black_config.get("line_length", black.DEFAULT_LINE_LENGTH),
+        string_normalization=not black_config.get("skip_string_normalization", False),
+    )
+
+
+black_config = {None: load_black_mode()}  # None key's value is default config
+
+
 class BlackStyleChecker(object):
     """Checker of Python code using black."""
 
     name = "black"
     version = __version__
+    override_config = None
 
     STDIN_NAMES = {"stdin", "-", "(none)", None}
 
     def __init__(self, tree, filename="(none)"):
         """Initialise."""
         self.tree = tree
         self.filename = filename
-        self.line_length = black.DEFAULT_LINE_LENGTH  # Expect to be 88
 
     @property
-    @lru_cache()
-    def config_file(self):
-        """File path to the black configuration file."""
-        if self.flake8_black_config:
-            flake8_black_path = Path(self.flake8_black_config)
-
-            if self.flake8_config:
-                flake8_config_path = path.dirname(path.abspath(self.flake8_config))
-                return Path(flake8_config_path) / flake8_black_path
-
-            return flake8_black_path
+    def _file_mode(self):
+        """Return black.FileMode object, using local pyproject.toml as needed."""
+        if self.override_config:
+            return self.override_config
 
+        # Unless using override, we look for pyproject.toml
         project_root = black.find_project_root(
             ("." if self.filename in self.STDIN_NAMES else self.filename,)
         )
-        return project_root / "pyproject.toml"
-
-    def _load_black_config(self):
-        if self.config_file.is_file():
-            LOG.info("flake8-black: Loading black config from %s" % self.config_file)
-            pyproject_toml = toml.load(str(self.config_file))
-            config = pyproject_toml.get("tool", {}).get("black", {})
-            return {k.replace("--", "").replace("-", "_"): v for k, v in config.items()}
-        elif self.config_file:
-            LOG.info("flake8-black: Did not find %s" % self.config_file)
-        return None
-
-    @property
-    def _file_mode(self):
-        target_versions = set()
-        skip_string_normalization = False
-
-        black_config = self._load_black_config()
-        if black_config:
-            target_versions = {
-                black.TargetVersion[val.upper()]
-                for val in black_config.get("target_version", [])
-            }
-            self.line_length = black_config.get("line_length", self.line_length)
-            skip_string_normalization = black_config.get(
-                "skip_string_normalization", False
-            )
-        # Requires black 19.3b0 or later:
-        return black.FileMode(
-            target_versions=target_versions,
-            line_length=self.line_length,
-            string_normalization=not skip_string_normalization,
-        )
+        path = project_root / "pyproject.toml"
+
+        if path in black_config:
+            # Already loaded
+            LOG.debug("flake8-black: %s using pre-loaded %s", self.filename, path)
+            return black_config[path]
+        elif path.is_file():
+            # Use this pyproject.toml for this python file,
+            # (unless configured with global override config)
+            # This should be thread safe - does not matter even if
+            # two workers load and cache this file at the same time
+            black_config[path] = load_black_mode(path)
+            LOG.debug("flake8-black: %s using newly loaded %s", self.filename, path)
+            return black_config[path]
+        else:
+            # No project specific file, use default
+            LOG.debug("flake8-black: %s using defaults", self.filename)
+            return black_config[None]
 
     @classmethod
     def add_options(cls, parser):
         """Adding black-config option."""
         parser.add_option(
             "--black-config",
+            metavar="TOML_FILENAME",
             default=None,
             action="store",
-            type="string",
+            # type="string",  <- breaks using None as a sentinel
+            # normalize_paths=True,  <- broken and breaks None as a sentinel
+            # https://gitlab.com/pycqa/flake8/issues/562
+            # https://gitlab.com/pycqa/flake8/merge_requests/337
             parse_from_config=True,
-            help="Path to black configuration file "
-            "(overrides the default pyproject.toml)",
+            help="Path to black TOML configuration file (overrides the "
+            "default 'pyproject.toml' detection; use empty string '' to mean "
+            "ignore all 'pyproject.toml' files).",
         )
 
     @classmethod
     def parse_options(cls, options):
         """Adding black-config option."""
-        cls.flake8_black_config = options.black_config
-        cls.flake8_config = options.config
+        # We have one and only one flake8 plugin configuration
+        if options.black_config is None:
+            LOG.info("flake8-black: No black configuration set")
+            cls.override_config = None
+            return
+        elif not options.black_config:
+            LOG.info("flake8-black: Explicitly using no black configuration file")
+            cls.override_config = black_config[None]  # explicitly use defaults
+            return
+
+        # Validate the path setting - handling relative paths ourselves,
+        # see https://gitlab.com/pycqa/flake8/issues/562
+        black_config_path = Path(options.black_config)
+        if options.config:
+            # Assume black config path was via flake8 config file
+            base_path = Path(path.dirname(path.abspath(options.config)))
+            black_config_path = base_path / black_config_path
+        if not black_config_path.is_file():
+            # Want flake8 to abort, see:
+            # https://gitlab.com/pycqa/flake8/issues/559
+            raise ValueError(
+                "Plugin flake8-black could not find specified black config file: "
+                "--black-config %s" % black_config_path
+            )
+
+        # Now load the TOML file, and the black section within it
+        # This configuration is to override any local pyproject.toml
+        try:
+            cls.override_config = black_config[black_config_path] = load_black_mode(
+                black_config_path
+            )
+        except BadBlackConfig:
+            # Could raise BLK997, but view this as an abort condition
+            raise ValueError(
+                "Plugin flake8-black could not parse specified black config file: "
+                "--black-config %s" % black_config_path
+            )
 
     def run(self):
         """Use black to check code style."""
@@ -151,8 +208,10 @@ def run(self):
                 return
             except black.InvalidInput:
                 msg = "901 Invalid input."
-            except Exception as e:
-                msg = "999 Unexpected exception: %s" % e
+            except BadBlackConfig as err:
+                msg = "997 Invalid TOML file: %s" % err
+            except Exception as err:
+                msg = "999 Unexpected exception: %s" % err
             else:
                 assert (
                     new_code != source

tests/run_tests.sh

@@ -5,21 +5,41 @@ IFS=$'\n\t'
 # Assumes in the tests/ directory
 
 echo "Checking our configuration option appears in help"
-flake8 -h 2>&1  | grep "black-config"
+flake8 -h 2>&1 | grep "black-config"
+
+set +o pipefail
+
+echo "Checking we report an error when can't find specified config file"
+flake8 --black-config does_not_exist.toml 2>&1 | grep -i "could not find"
+
+echo "Checking failure with mal-formed TOML file"
+flake8 --select BLK test_cases/ --black-config with_bad_toml/pyproject.toml 2>&1 | grep -i "could not parse"
+
+set -o pipefail
 
 echo "Checking we report no errors on these test cases"
 flake8 --select BLK test_cases/*.py
+# Adding --black-config '' meaning ignore any pyproject.toml should have no effect:
+flake8 --select BLK test_cases/*.py --black-config ''
 flake8 --select BLK --max-line-length 50 test_cases/*.py
 flake8 --select BLK --max-line-length 90 test_cases/*.py
 flake8 --select BLK with_pyproject_toml/*.py
 flake8 --select BLK with_pyproject_toml/*.py --black-config with_pyproject_toml/pyproject.toml
 flake8 --select BLK without_pyproject_toml/*.py --config=flake8_config/flake8
 flake8 --select BLK --max-line-length 88 with_pyproject_toml/
+flake8 --select BLK without_pyproject_toml/*.py --black-config with_pyproject_toml/pyproject.toml
+# Adding --black-config '' should have no effect:
+#flake8 --select BLK --max-line-length 88 with_pyproject_toml/ --black-config ''
 flake8 --select BLK non_conflicting_configurations/*.py
 flake8 --select BLK conflicting_configurations/*.py
+# Here using --black-config '' meaning ignore any (bad) pyproject.toml files:
+flake8 --select BLK with_bad_toml/hello_world.py --black-config ''
 
 echo "Checking we report expected black changes"
 diff test_changes/hello_world.txt <(flake8 --select BLK test_changes/hello_world.py)
 diff test_changes/hello_world_EOF.txt <(flake8 --select BLK test_changes/hello_world_EOF.py)
+diff test_changes/hello_world_EOF.txt <(flake8 --select BLK test_changes/hello_world_EOF.py --black-config '')
+diff with_bad_toml/hello_world.txt <(flake8 --select BLK with_bad_toml/hello_world.py)
+diff with_pyproject_toml/ignoring_toml.txt <(flake8 with_pyproject_toml/ --select BLK --black-config '')
 
 echo "Tests passed."

tests/with_bad_toml/hello_world.py

@@ -0,0 +1,9 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+"""Print 'Hello world' to the terminal.
+
+This is a simple test script using a hashbang line
+and a PEP263 encoding line.
+"""
+
+print("Hello world")

tests/with_bad_toml/hello_world.txt

@@ -0,0 +1 @@
+with_bad_toml/hello_world.py:0:1: BLK997 Invalid TOML file: with_bad_toml/pyproject.toml

tests/with_bad_toml/pyproject.toml

@@ -0,0 +1,4 @@
+[tool.black]
+skip-string-normalization = true
+# This line is (a) in the wrong file, and (b) invalid syntax
+black-config=

tests/with_pyproject_toml/ignoring_toml.txt

@@ -0,0 +1 @@
+with_pyproject_toml/ordinary_quotes.py:10:7: BLK100 Black would make changes.