From 29603db6a4ee73b030e70e8fd219e542d76524c6 Mon Sep 17 00:00:00 2001 From: peterjc Date: Thu, 8 Aug 2019 14:29:12 +0100 Subject: [PATCH] Refactor TOML loading; BLK907 for invalid pyproject.toml --- .flake8 | 2 + README.rst | 2 + flake8_black.py | 165 +++++++++++++------- tests/run_tests.sh | 22 ++- tests/with_bad_toml/hello_world.py | 9 ++ tests/with_bad_toml/hello_world.txt | 1 + tests/with_bad_toml/pyproject.toml | 4 + tests/with_pyproject_toml/ignoring_toml.txt | 1 + 8 files changed, 152 insertions(+), 54 deletions(-) create mode 100755 tests/with_bad_toml/hello_world.py create mode 100644 tests/with_bad_toml/hello_world.txt create mode 100644 tests/with_bad_toml/pyproject.toml create mode 100644 tests/with_pyproject_toml/ignoring_toml.txt diff --git a/.flake8 b/.flake8 index d32ac0b..fc8b7c1 100644 --- a/.flake8 +++ b/.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: diff --git a/README.rst b/README.rst index 5434636..513970e 100644 --- a/README.rst +++ b/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 `_. + - 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. diff --git a/flake8_black.py b/flake8_black.py index 4670c63..5c107c8 100644 --- a/flake8_black.py +++ b/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,11 +38,50 @@ 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} @@ -50,76 +89,94 @@ 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 diff --git a/tests/run_tests.sh b/tests/run_tests.sh index e7b77c2..c71d25e 100755 --- a/tests/run_tests.sh +++ b/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." diff --git a/tests/with_bad_toml/hello_world.py b/tests/with_bad_toml/hello_world.py new file mode 100755 index 0000000..c8a5332 --- /dev/null +++ b/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") diff --git a/tests/with_bad_toml/hello_world.txt b/tests/with_bad_toml/hello_world.txt new file mode 100644 index 0000000..0b2de4c --- /dev/null +++ b/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 diff --git a/tests/with_bad_toml/pyproject.toml b/tests/with_bad_toml/pyproject.toml new file mode 100644 index 0000000..5bd4be1 --- /dev/null +++ b/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= diff --git a/tests/with_pyproject_toml/ignoring_toml.txt b/tests/with_pyproject_toml/ignoring_toml.txt new file mode 100644 index 0000000..f460bcf --- /dev/null +++ b/tests/with_pyproject_toml/ignoring_toml.txt @@ -0,0 +1 @@ +with_pyproject_toml/ordinary_quotes.py:10:7: BLK100 Black would make changes.