From 138c47f6028496ba35a12a22fc84388d4405ae97 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 09:18:56 +0200 Subject: [PATCH 1/6] Move validation to jsonschema --- README.md | 4 +- pyproject.toml | 5 + src/hatch_fancy_pypi_readme/_config.py | 116 ++++++++--- src/hatch_fancy_pypi_readme/_fragments.py | 112 ++++------ .../_humanize_validation_errors.py | 92 +++++++++ src/hatch_fancy_pypi_readme/_substitutions.py | 39 +--- src/hatch_fancy_pypi_readme/hooks.py | 2 +- tests/test_cli.py | 8 +- tests/test_config.py | 192 +++++++++++++++++- tests/test_end_to_end.py | 8 +- tests/test_fragments.py | 78 +------ tests/test_humanize_validation_errors.py | 30 +++ tests/test_substitutions.py | 50 +---- 13 files changed, 463 insertions(+), 273 deletions(-) create mode 100644 src/hatch_fancy_pypi_readme/_humanize_validation_errors.py create mode 100644 tests/test_humanize_validation_errors.py diff --git a/README.md b/README.md index 3992b49..d7358fd 100644 --- a/README.md +++ b/README.md @@ -182,8 +182,8 @@ After a readme is assembled out of fragments, it's possible to run an arbitrary ```toml [[tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions]] pattern = "This is a (.*) that we'll replace later." -replacement = "It was a '\\1'!" -ignore_case = true # optional; false by default +replacement = 'It was a '\1'!' +ignore-case = true # optional; false by default ``` --- diff --git a/pyproject.toml b/pyproject.toml index c4498b9..a2a06c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ classifiers = [ ] dependencies = [ "hatchling", + "jsonschema", "tomli; python_version<'3.11'", "typing-extensions; python_version<'3.8'", ] @@ -116,9 +117,13 @@ profile = "attrs" [tool.mypy] +show_error_codes = true +enable_error_code = ["ignore-without-code"] strict = true follow_imports = "normal" warn_no_return = true +ignore_missing_imports = true + [[tool.mypy.overrides]] module = "tests.*" diff --git a/src/hatch_fancy_pypi_readme/_config.py b/src/hatch_fancy_pypi_readme/_config.py index e0dc124..51911c4 100644 --- a/src/hatch_fancy_pypi_readme/_config.py +++ b/src/hatch_fancy_pypi_readme/_config.py @@ -7,8 +7,11 @@ from dataclasses import dataclass from typing import Any -from ._fragments import Fragment, load_fragments -from ._substitutions import Substituter, load_substitutions +from jsonschema import Draft202012Validator + +from ._fragments import VALID_FRAGMENTS, Fragment +from ._humanize_validation_errors import errors_to_human_strings +from ._substitutions import Substituter from .exceptions import ConfigurationError @@ -19,36 +22,97 @@ class Config: substitutions: list[Substituter] +SCHEMA = { + "type": "object", + "properties": { + "content-type": { + "type": "string", + "enum": ["text/markdown", "text/x-rst"], + }, + "fragments": { + "type": "array", + "minItems": 1, + # Items are validated separately for better error messages. + "items": {"type": "object"}, + }, + "substitutions": { + "type": "array", + "items": { + "type": "object", + "properties": { + "pattern": {"type": "string", "format": "regex"}, + "replacement": {"type": "string"}, + "ignore-case": {"type": "boolean"}, + }, + "required": ["pattern", "replacement"], + "additionalProperties": False, + }, + }, + }, + "required": ["content-type", "fragments"], + "additionalProperties": False, +} + +V = Draft202012Validator( + SCHEMA, format_checker=Draft202012Validator.FORMAT_CHECKER +) + + def load_and_validate_config(config: dict[str, Any]) -> Config: - errs = [] + errs = sorted( + V.iter_errors(config), + key=lambda e: e.path, # type: ignore[no-any-return] + ) + if errs: + raise ConfigurationError(errors_to_human_strings(errs)) + + return Config( + config["content-type"], + _load_fragments(config["fragments"]), + [ + Substituter.from_config(sub_cfg) + for sub_cfg in config.get("substitutions", []) + ], + ) + + +def _load_fragments(config: list[dict[str, str]]) -> list[Fragment]: + """ + Load fragments from *config*. + + This is a bit more complicated because validating the fragments field using + `oneOf` leads to unhelpful error messages that are difficult to convert + into something humanly meaningful. + + So we detect first, validate using jsonschema and try to load them. They + still may fail loading if they refer to files and lack markers / the + pattern doesn't match. + """ frags = [] + errs = [] - if "content-type" not in config: - errs.append( - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme.content-type " - "setting." - ) - - try: - try: - frag_cfg_list = config["fragments"] - except KeyError: - errs.append( - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme.fragments" - " setting." - ) - else: - frags = load_fragments(frag_cfg_list) + for i, frag_cfg in enumerate(config): + for frag in VALID_FRAGMENTS: + if frag.key not in frag_cfg: + continue - except ConfigurationError as e: - errs.extend(e.errors) + try: + ves = tuple(frag.validator.iter_errors(frag_cfg)) + if ves: + raise ConfigurationError( + errors_to_human_strings(ves, ("fragments", i)) + ) + frags.append(frag.from_config(frag_cfg)) + except ConfigurationError as e: + errs.extend(e.errors) - try: - subs = load_substitutions(config.get("substitutions", [])) - except ConfigurationError as e: - errs.extend(e.errors) + # We have either detecte and added or detected and errored, but in + # any case we're done with this fragment. + break + else: + errs.append(f"Unknown fragment type {frag_cfg!r}.") if errs: raise ConfigurationError(errs) - return Config(config["content-type"], frags, subs) + return frags diff --git a/src/hatch_fancy_pypi_readme/_fragments.py b/src/hatch_fancy_pypi_readme/_fragments.py index d01a540..faccf49 100644 --- a/src/hatch_fancy_pypi_readme/_fragments.py +++ b/src/hatch_fancy_pypi_readme/_fragments.py @@ -11,52 +11,46 @@ from pathlib import Path from typing import ClassVar, Iterable +from jsonschema import Draft202012Validator, Validator + if sys.version_info >= (3, 8): from typing import Protocol else: from typing_extensions import Protocol -from .exceptions import ConfigurationError - - -def load_fragments(config: list[dict[str, str]]) -> list[Fragment]: - """ - Load all fragments from the fragments config list. - - Raise ConfigurationError on unknown or misconfigured ones. - """ - if not config: - raise ConfigurationError( - [ - "tool.hatch.metadata.hooks.fancy-pypi-readme.fragments must " - "not be empty." - ] - ) - - frags = [] - errs = [] - for frag_cfg in config: - for frag in _VALID_FRAGMENTS: - if frag.key not in frag_cfg: - continue - try: - frags.append(frag.from_config(frag_cfg)) - except ConfigurationError as e: - errs.extend(e.errors) - - break - else: - errs.append(f"Unknown fragment type {frag_cfg!r}.") +from .exceptions import ConfigurationError - if errs: - raise ConfigurationError(errs) - return frags +TEXT_V = Draft202012Validator( + { + "type": "object", + "properties": {"text": {"type": "string", "pattern": ".+"}}, + "required": ["text"], + "additionalProperties": False, + }, + format_checker=Draft202012Validator.FORMAT_CHECKER, +) + +FILE_V = Draft202012Validator( + { + "type": "object", + "properties": { + "path": {"type": "string", "pattern": ".+"}, + "start-after": {"type": "string", "pattern": ".+"}, + "end-before": {"type": "string", "pattern": ".+"}, + "pattern": {"type": "string", "format": "regex"}, + }, + "required": ["path"], + "additionalProperties": False, + }, + format_checker=Draft202012Validator.FORMAT_CHECKER, +) class Fragment(Protocol): key: ClassVar[str] + validator: ClassVar[Validator] @classmethod def from_config(self, cfg: dict[str, str]) -> Fragment: @@ -73,24 +67,13 @@ class TextFragment: """ key: ClassVar[str] = "text" + validator: ClassVar[Validator] = TEXT_V _text: str @classmethod def from_config(cls, cfg: dict[str, str]) -> Fragment: - contents = cfg.pop(cls.key) - - if not contents: - raise ConfigurationError( - [f"text fragment: {cls.key} can't be empty."] - ) - - if cfg: - raise ConfigurationError( - [f"text fragment: unknown option: {o}" for o in cfg.keys()] - ) - - return cls(contents) + return cls(cfg[cls.key]) def render(self) -> str: return self._text @@ -103,6 +86,7 @@ class FileFragment: """ key: ClassVar[str] = "path" + validator: ClassVar[Validator] = FILE_V _contents: str @@ -114,12 +98,11 @@ def from_config(cls, cfg: dict[str, str]) -> Fragment: pattern = cfg.pop("pattern", None) errs: list[str] = [] - if cfg: - errs.extend( - f"file fragment: unknown option: {o!r}" for o in cfg.keys() - ) - contents = path.read_text(encoding="utf-8") + try: + contents = path.read_text(encoding="utf-8") + except FileNotFoundError: + raise ConfigurationError([f"Fragment file '{path}' not found."]) if start_after is not None: try: @@ -138,22 +121,17 @@ def from_config(cls, cfg: dict[str, str]) -> Fragment: ) if pattern: - try: - m = re.search(pattern, contents, re.DOTALL) - if not m: + m = re.search(pattern, contents, re.DOTALL) + if not m: + errs.append(f"file fragment: pattern {pattern!r} not found.") + else: + try: + contents = m.group(1) + except IndexError: errs.append( - f"file fragment: pattern {pattern!r} not found." + "file fragment: pattern matches, but no group " + "defined." ) - else: - try: - contents = m.group(1) - except IndexError: - errs.append( - "file fragment: pattern matches, but no group " - "defined." - ) - except re.error as e: - errs.append(f"file fragment: invalid pattern {pattern!r}: {e}") if errs: raise ConfigurationError(errs) @@ -164,4 +142,4 @@ def render(self) -> str: return self._contents -_VALID_FRAGMENTS: Iterable[type[Fragment]] = (TextFragment, FileFragment) +VALID_FRAGMENTS: Iterable[type[Fragment]] = (TextFragment, FileFragment) diff --git a/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py new file mode 100644 index 0000000..25fbdba --- /dev/null +++ b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py @@ -0,0 +1,92 @@ +# SPDX-FileCopyrightText: 2022 Hynek Schlawack +# +# SPDX-License-Identifier: MIT + +from __future__ import annotations + +from typing import Iterable, Tuple, Union + +from jsonschema.exceptions import ValidationError + + +PathElement = Union[int, str] +FieldPath = Tuple[PathElement, ...] + + +def errors_to_human_strings( + excs: Iterable[ValidationError], extra_path: FieldPath = () +) -> list[str]: + """ + Make *excs* as human-readable as possible. + + Add *extra_path* between base path and the error's path (which is + incomplete from the user's view). + """ + errs = [] + + for e in excs: + full_path = BASE_PATH + extra_path + tuple(e.path) + errs.append(_VALIDATOR_TO_FORMATTER[e.validator](full_path, e)) + + return errs + + +BASE_PATH = ("tool.hatch", "metadata", "hooks", "fancy-pypi-readme") + + +def _format_missing(path: FieldPath, e: ValidationError) -> str: + missing = e.message[1:].split("'", 1)[0] + + return f"{_dot_path(path, missing)} is missing." + + +def _format_not_enough(path: FieldPath, e: ValidationError) -> str: + assert e.validator_value == 1 + + return f"{_dot_path(path)} must not be empty." + + +def _format_additional_fields(path: FieldPath, e: ValidationError) -> str: + extra = e.message.split("'")[-2] + return f"{_dot_path(path, extra)}: extra field not permitted." + + +def _format_wrong_type(path: FieldPath, e: ValidationError) -> str: + return f"{_dot_path(path)} is of wrong type: {e.message}" + + +def _format_pattern_mismatch(path: FieldPath, e: ValidationError) -> str: + if e.validator_value == ".+": + return f"{_dot_path(path)} must not be empty." + + return f"{_dot_path(path)}: {e.message}" + + +def _format_path_w_message(path: FieldPath, e: ValidationError) -> str: + return f"{_dot_path(path)}: {e.message}" + + +def _dot_path(path: FieldPath, last: str | None = None) -> str: + """ + Concat our base path with `e.absolute_path` and append *last* if specified. + """ + common = ".".join(str(p) for p in path) + + if not last: + return common + + # We end up with two dots if there's no path. + maybe_dot = "." if path else "" + + return f"{common}{maybe_dot}{last}" + + +_VALIDATOR_TO_FORMATTER = { + "required": _format_missing, + "type": _format_wrong_type, + "minItems": _format_not_enough, + "additionalProperties": _format_additional_fields, + "format": _format_path_w_message, + "enum": _format_path_w_message, + "pattern": _format_pattern_mismatch, +} diff --git a/src/hatch_fancy_pypi_readme/_substitutions.py b/src/hatch_fancy_pypi_readme/_substitutions.py index 291d42e..f22a299 100644 --- a/src/hatch_fancy_pypi_readme/_substitutions.py +++ b/src/hatch_fancy_pypi_readme/_substitutions.py @@ -8,24 +8,6 @@ from dataclasses import dataclass -from hatch_fancy_pypi_readme.exceptions import ConfigurationError - - -def load_substitutions(config: list[dict[str, str]]) -> list[Substituter]: - errs = [] - subs = [] - - for cfg in config: - try: - subs.append(Substituter.from_config(cfg)) - except ConfigurationError as e: - errs.extend(e.errors) - - if errs: - raise ConfigurationError([f"substitution: {e}" for e in errs]) - - return subs - @dataclass class Substituter: @@ -34,29 +16,14 @@ class Substituter: @classmethod def from_config(cls, cfg: dict[str, str]) -> Substituter: - errs = [] flags = 0 - ignore_case = cfg.get("ignore_case", False) - if not isinstance(ignore_case, bool): - errs.append("`ignore_case` must be a bool.") + ignore_case = cfg.get("ignore-case", False) if ignore_case: flags += re.IGNORECASE - try: - pattern = re.compile(cfg["pattern"], flags=flags) - except KeyError: - errs.append("missing `pattern` key.") - except re.error as e: - errs.append(f"can't compile pattern: {e}") - - try: - replacement = cfg["replacement"] - except KeyError: - errs.append("missing `replacement` key.") - - if errs: - raise ConfigurationError(errs) + pattern = re.compile(cfg["pattern"], flags=flags) + replacement = cfg["replacement"] return cls(pattern, replacement) diff --git a/src/hatch_fancy_pypi_readme/hooks.py b/src/hatch_fancy_pypi_readme/hooks.py index 9162873..7472be0 100644 --- a/src/hatch_fancy_pypi_readme/hooks.py +++ b/src/hatch_fancy_pypi_readme/hooks.py @@ -29,6 +29,6 @@ def update(self, metadata: dict[str, Any]) -> None: } -@hookimpl # type: ignore +@hookimpl # type: ignore[misc] def hatch_register_metadata_hook() -> type[MetadataHookInterface]: return FancyReadmeMetadataHook diff --git a/tests/test_cli.py b/tests/test_cli.py index b814852..80fbc5a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -121,10 +121,10 @@ def test_cli_run_config_error(self, capfd, empty_pyproject): assert ( "Configuration has errors:\n\n" - "- Missing tool.hatch.metadata.hooks.fancy-pypi-readme." - "content-type setting.\n" - "- Missing tool.hatch.metadata.hooks.fancy-pypi-readme.fragments " - "setting.\n" == err + "- tool.hatch.metadata.hooks.fancy-pypi-readme." + "content-type is missing.\n" + "- tool.hatch.metadata.hooks.fancy-pypi-readme.fragments " + "is missing.\n" == err ) assert "" == out diff --git a/tests/test_config.py b/tests/test_config.py index 77919aa..f8322cb 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -28,13 +28,39 @@ def test_missing_content_type(self): assert ( [ - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme." - "content-type setting." + "tool.hatch.metadata.hooks.fancy-pypi-readme." + "content-type is missing." ] == ei.value.errors == ei.value.args[0] ) + def test_wrong_content_type(self): + """ + Missing content-type is caught. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + {"content-type": "text/html", "fragments": [{"text": "foo"}]} + ) + + assert [ + "tool.hatch.metadata.hooks.fancy-pypi-readme.content-type: " + "'text/html' is not one of ['text/markdown', 'text/x-rst']" + ] == ei.value.errors + + +VALID_FOR_FRAG = {"content-type": "text/markdown"} + + +def cow_add_frag(**kw): + d = VALID_FOR_FRAG.copy() + d["fragments"] = [kw] + + return d + + +class TestValidateConfigFragments: def test_empty_fragments(self): """ Empty fragments are caught. @@ -62,13 +88,86 @@ def test_missing_fragments(self): assert ( [ - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme.fragments" - " setting." + "tool.hatch.metadata.hooks.fancy-pypi-readme.fragments" + " is missing." ] == ei.value.errors == ei.value.args[0] ) + def test_empty_fragment_dict(self): + """ + Empty fragment dicts are handled gracefully. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + {"content-type": "text/markdown", "fragments": [{}]} + ) + + assert ["Unknown fragment type {}."] == ei.value.errors + + def test_empty_text_fragment(self): + """ + Text fragments can't be empty. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config(cow_add_frag(text="")) + + assert [ + "tool.hatch.metadata.hooks.fancy-pypi-readme.fragments.0.text " + "must not be empty." + ] == ei.value.errors + + def test_invalid_fragments(self): + """ + Invalid fragments are caught. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + { + "content-type": "text/markdown", + "fragments": [ + {"text": "this is ok"}, + {"foo": "this is not"}, + {"bar": "neither is this"}, + ], + } + ) + + assert { + "Unknown fragment type {'foo': 'this is not'}.", + "Unknown fragment type {'bar': 'neither is this'}.", + } == set(ei.value.errors) + + def test_fragment_loading_errors(self): + """ + Errors that happen while loading a fragment are propagated. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + { + "content-type": "text/markdown", + "fragments": [{"path": "yolo"}], + } + ) + + assert ["Fragment file 'yolo' not found."] == ei.value.errors + + +VALID_FOR_SUB = { + "content-type": "text/markdown", + "fragments": [{"text": "foobar"}], +} + + +def cow_add_sub(**kw): + d = VALID_FOR_SUB.copy() + d["substitutions"] = [kw] + + return d + + +class TestValidateConfigSubstitutions: def test_invalid_substitution(self): """ Invalid substitutions are caught and reported. @@ -82,7 +181,84 @@ def test_invalid_substitution(self): } ) - assert [ - "substitution: missing `pattern` key.", - "substitution: missing `replacement` key.", - ] == ei.value.errors + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions" + ".0.pattern is missing.", + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions" + ".0.replacement is missing.", + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions.0" + ".foo: extra field not permitted.", + } == set(ei.value.errors) + + def test_empty(self): + """ + Empty dict is not valid. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config(cow_add_sub()) + + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions" + ".0.pattern is missing.", + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions" + ".0.replacement is missing.", + } == set(ei.value.errors) + + def test_ignore_case_not_bool(self): + """ + Ignore case is either bool or nothing. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + cow_add_sub( + pattern="foo", replacement="bar", **{"ignore-case": 42} + ) + ) + + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions.0." + "ignore-case is of wrong type: 42 is not of type 'boolean'" + } == set(ei.value.errors) + + def test_pattern_no_valid_regexp(self): + """ + Pattern must be a valid re-regexp. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + cow_add_sub(pattern="foo???", replacement="bar") + ) + + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions.0." + "pattern: 'foo???' is not a 'regex'", + } == set(ei.value.errors) + + def test_replacement_not_a_string(self): + """ + Replacements must be strings. + """ + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config( + cow_add_sub(pattern="foo", replacement=42) + ) + + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions.0." + "replacement is of wrong type: 42 is not of type 'string'", + } == set(ei.value.errors) + + def test_substitutions_not_array(self): + """ + Substitutions key must be a list. + """ + cfg = VALID_FOR_SUB.copy() + cfg["substitutions"] = {} + + with pytest.raises(ConfigurationError) as ei: + load_and_validate_config(cfg) + + assert { + "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions is of " + "wrong type: {} is not of type 'array'" + } == set(ei.value.errors) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index adf6e3c..0dbf057 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -74,10 +74,10 @@ def test_invalid_config(new_project): assert "hatch_fancy_pypi_readme.exceptions.ConfigurationError:" in out assert ( - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme.content-type " - "setting." in out + "tool.hatch.metadata.hooks.fancy-pypi-readme.content-type " + "is missing." in out ) assert ( - "Missing tool.hatch.metadata.hooks.fancy-pypi-readme.fragments " - "setting." in out + "tool.hatch.metadata.hooks.fancy-pypi-readme.fragments " + "is missing." in out ) diff --git a/tests/test_fragments.py b/tests/test_fragments.py index fc90180..1eb66ff 100644 --- a/tests/test_fragments.py +++ b/tests/test_fragments.py @@ -10,22 +10,11 @@ import pytest -from hatch_fancy_pypi_readme._fragments import ( - FileFragment, - TextFragment, - load_fragments, -) +from hatch_fancy_pypi_readme._fragments import FileFragment, TextFragment from hatch_fancy_pypi_readme.exceptions import ConfigurationError class TestTextFragment: - def test_cfg_empty_text(self): - """ - Empty text keys raise a ConfigurationError. - """ - with pytest.raises(ConfigurationError, match="can't be empty"): - TextFragment.from_config({"text": ""}) - def test_ok(self): """ The text that is passed in is rendered without changes. @@ -109,20 +98,6 @@ def test_start_end_ok(self, txt_path): ).render() ) - def test_unknown_options(self, txt_path): - """ - Unknown options are caught and raised at ConfiguratinErrors - """ - with pytest.raises(ConfigurationError) as ei: - FileFragment.from_config( - {"path": str(txt_path), "foo": "bar", "baz": "qux"} - ) - - assert [ - "file fragment: unknown option: 'foo'", - "file fragment: unknown option: 'baz'", - ] == ei.value.errors - def test_start_after_end_before_not_found(self, txt_path): """ If `start-after` and/or `end-before` don't exist, a helpful error is @@ -142,22 +117,6 @@ def test_start_after_end_before_not_found(self, txt_path): "file fragment: 'end-before' 'also nope' not found.", ] == ei.value.errors - def test_invalid_pattern(self, txt_path): - """ - re-compilation errors are caught and reported. - """ - with pytest.raises(ConfigurationError) as ei: - FileFragment.from_config( - { - "path": str(txt_path), - "pattern": r"**", - } - ) - assert [ - "file fragment: invalid pattern '**': nothing to repeat at " - "position 0" - ] == ei.value.errors - def test_pattern_no_match(self, txt_path): """ If the pattern doesn't match, a helpful error is raises. @@ -201,38 +160,3 @@ def test_pattern_ok(self, txt_path): } ).render() ) - - -class TestLoadFragments: - def test_invalid_fragment_type(self): - """ - Invalid fragment types are reported. - """ - with pytest.raises(ConfigurationError) as ei: - load_fragments( - [ - {"text": "this is ok"}, - {"foo": "this is not"}, - {"bar": "neither is this"}, - ] - ) - - assert [ - "Unknown fragment type {'foo': 'this is not'}.", - "Unknown fragment type {'bar': 'neither is this'}.", - ] == ei.value.errors - - def test_invalid_config(self): - """ - If the config of a fragment raiss a Configuration error, collect it and - raise it at the end. - """ - with pytest.raises(ConfigurationError) as ei: - load_fragments( - [ - {"text": "this is ok"}, - {"text": "this is not", "because": "of this"}, - ] - ) - - assert ["text fragment: unknown option: because"] == ei.value.errors diff --git a/tests/test_humanize_validation_errors.py b/tests/test_humanize_validation_errors.py new file mode 100644 index 0000000..847e0dc --- /dev/null +++ b/tests/test_humanize_validation_errors.py @@ -0,0 +1,30 @@ +# SPDX-FileCopyrightText: 2022 Hynek Schlawack +# +# SPDX-License-Identifier: MIT + +import jsonschema +import pytest + +from hatch_fancy_pypi_readme._humanize_validation_errors import ( + errors_to_human_strings, +) + + +def test_pattern_mismatch(): + """ + We currently don't use the format with other values than .+ -- i.e. not + empty. It's easiest to write a unit test that ensures it does the correct + thing for both cases. + """ + v = jsonschema.Draft202012Validator( + {"type": "string", "pattern": "foo"}, + format_checker=jsonschema.Draft202012Validator.FORMAT_CHECKER, + ) + + with pytest.raises(jsonschema.ValidationError) as ei: + v.validate("bar") + + assert [ + "tool.hatch.metadata.hooks.fancy-pypi-readme.some-field: 'bar' does " + "not match 'foo'" + ] == errors_to_human_strings([ei.value], ("some-field",)) diff --git a/tests/test_substitutions.py b/tests/test_substitutions.py index a1cec8c..f899614 100644 --- a/tests/test_substitutions.py +++ b/tests/test_substitutions.py @@ -4,33 +4,7 @@ from __future__ import annotations -import pytest - -from hatch_fancy_pypi_readme._substitutions import ( - Substituter, - load_substitutions, -) -from hatch_fancy_pypi_readme.exceptions import ConfigurationError - - -class TestLoadSubstitutions: - def test_empty(self): - """ - Having no substitutions is fine. - """ - assert [] == load_substitutions([]) - - def test_error(self): - """ - Invalid substitutions are caught and reported. - """ - with pytest.raises(ConfigurationError) as ei: - load_substitutions([{"in": "valid"}]) - - assert [ - "substitution: missing `pattern` key.", - "substitution: missing `replacement` key.", - ] == ei.value.errors +from hatch_fancy_pypi_readme._substitutions import Substituter VALID = {"pattern": "f(o)o", "replacement": r"bar\g<1>bar"} @@ -52,26 +26,6 @@ def test_ok(self): assert "xxx barobar yyy" == sub.substitute("xxx foo yyy") - @pytest.mark.parametrize( - "cfg, errs", - [ - ({}, ["missing `pattern` key.", "missing `replacement` key."]), - (cow_valid(ignore_case=42), ["`ignore_case` must be a bool."]), - ( - cow_valid(pattern="???"), - ["can't compile pattern: nothing to repeat at position 0"], - ), - ], - ) - def test_catches_all_errors(self, cfg, errs): - """ - All errors are caught and reported. - """ - with pytest.raises(ConfigurationError) as ei: - Substituter.from_config(cfg) - - assert errs == ei.value.errors - def test_twisted(self): """ Twisted example works. @@ -84,7 +38,7 @@ def test_twisted(self): { "pattern": r"`([^`]+)\s+<(?!https?://)([^>]+)>`_", "replacement": r"`\1 `_", # noqa - "ignore_case": True, + "ignore-case": True, } ).substitute( "For information on changes in this release, see the `NEWS `_ file." # noqa From 25c179826f52e3798f95ba6dbc88dc30dfe26e90 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 09:48:16 +0200 Subject: [PATCH 2/6] Add unit tests for substitution samples https://github.com/hynek/hatch-fancy-pypi-readme/issues/9#issuecomment-1238584908 --- tests/test_substitutions.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_substitutions.py b/tests/test_substitutions.py index f899614..55d7338 100644 --- a/tests/test_substitutions.py +++ b/tests/test_substitutions.py @@ -4,6 +4,8 @@ from __future__ import annotations +import pytest + from hatch_fancy_pypi_readme._substitutions import Substituter @@ -43,3 +45,35 @@ def test_twisted(self): ).substitute( "For information on changes in this release, see the `NEWS `_ file." # noqa ) + + @pytest.mark.parametrize( + "pat,repl,text,expect", + [ + ( + r"#(\d+)", + r"[#\1](https://github.com/pydantic/pydantic/issues/\1)", + "* Foo #4224, #4470 Bar", + "* Foo [#4224](https://github.com/pydantic/pydantic/issues/" + "4224), [#4470](https://github.com/pydantic/pydantic/issues/" + "4470) Bar", + ), + ( + r"( +)@([\w\-]+)", + r"\1[@\2](https://github.com/\2)", + "foo @github-user bar", + "foo [@github-user](https://github.com/github-user) bar", + ), + ], + ) + def test_pydantic(self, pat, repl, text, expect): + """ + Pydantic examples work. + https://github.com/hynek/hatch-fancy-pypi-readme/issues/9#issuecomment-1238584908 + """ + assert expect == Substituter.from_config( + { + "pattern": pat, + "replacement": repl, + "ignore-case": True, + } + ).substitute(text) From 87dcea6d879a90d0d0956ac3e0842d163b5a323a Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 10:43:12 +0200 Subject: [PATCH 3/6] Address first round of feedback Co-authored-by: Julian Berman <329822+Julian@users.noreply.github.com> --- src/hatch_fancy_pypi_readme/_config.py | 13 ++++---- src/hatch_fancy_pypi_readme/_fragments.py | 4 ++- .../_humanize_validation_errors.py | 9 +----- tests/test_humanize_validation_errors.py | 30 ------------------- 4 files changed, 11 insertions(+), 45 deletions(-) delete mode 100644 tests/test_humanize_validation_errors.py diff --git a/src/hatch_fancy_pypi_readme/_config.py b/src/hatch_fancy_pypi_readme/_config.py index 51911c4..2a681f5 100644 --- a/src/hatch_fancy_pypi_readme/_config.py +++ b/src/hatch_fancy_pypi_readme/_config.py @@ -7,6 +7,8 @@ from dataclasses import dataclass from typing import Any +import jsonschema + from jsonschema import Draft202012Validator from ._fragments import VALID_FRAGMENTS, Fragment @@ -23,6 +25,7 @@ class Config: SCHEMA = { + "$schema": Draft202012Validator.META_SCHEMA["$id"], "type": "object", "properties": { "content-type": { @@ -53,15 +56,13 @@ class Config: "additionalProperties": False, } -V = Draft202012Validator( - SCHEMA, format_checker=Draft202012Validator.FORMAT_CHECKER -) - def load_and_validate_config(config: dict[str, Any]) -> Config: errs = sorted( - V.iter_errors(config), - key=lambda e: e.path, # type: ignore[no-any-return] + Draft202012Validator( + SCHEMA, format_checker=Draft202012Validator.FORMAT_CHECKER + ).iter_errors(config), + key=jsonschema.exceptions.relevance, # type: ignore[no-any-return] ) if errs: raise ConfigurationError(errors_to_human_strings(errs)) diff --git a/src/hatch_fancy_pypi_readme/_fragments.py b/src/hatch_fancy_pypi_readme/_fragments.py index faccf49..5651d92 100644 --- a/src/hatch_fancy_pypi_readme/_fragments.py +++ b/src/hatch_fancy_pypi_readme/_fragments.py @@ -24,8 +24,9 @@ TEXT_V = Draft202012Validator( { + "$schema": Draft202012Validator.META_SCHEMA["$id"], "type": "object", - "properties": {"text": {"type": "string", "pattern": ".+"}}, + "properties": {"text": {"type": "string", "minLength": 1}}, "required": ["text"], "additionalProperties": False, }, @@ -34,6 +35,7 @@ FILE_V = Draft202012Validator( { + "$schema": Draft202012Validator.META_SCHEMA["$id"], "type": "object", "properties": { "path": {"type": "string", "pattern": ".+"}, diff --git a/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py index 25fbdba..772f2eb 100644 --- a/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py +++ b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py @@ -55,13 +55,6 @@ def _format_wrong_type(path: FieldPath, e: ValidationError) -> str: return f"{_dot_path(path)} is of wrong type: {e.message}" -def _format_pattern_mismatch(path: FieldPath, e: ValidationError) -> str: - if e.validator_value == ".+": - return f"{_dot_path(path)} must not be empty." - - return f"{_dot_path(path)}: {e.message}" - - def _format_path_w_message(path: FieldPath, e: ValidationError) -> str: return f"{_dot_path(path)}: {e.message}" @@ -85,8 +78,8 @@ def _dot_path(path: FieldPath, last: str | None = None) -> str: "required": _format_missing, "type": _format_wrong_type, "minItems": _format_not_enough, + "minLength": _format_not_enough, "additionalProperties": _format_additional_fields, "format": _format_path_w_message, "enum": _format_path_w_message, - "pattern": _format_pattern_mismatch, } diff --git a/tests/test_humanize_validation_errors.py b/tests/test_humanize_validation_errors.py deleted file mode 100644 index 847e0dc..0000000 --- a/tests/test_humanize_validation_errors.py +++ /dev/null @@ -1,30 +0,0 @@ -# SPDX-FileCopyrightText: 2022 Hynek Schlawack -# -# SPDX-License-Identifier: MIT - -import jsonschema -import pytest - -from hatch_fancy_pypi_readme._humanize_validation_errors import ( - errors_to_human_strings, -) - - -def test_pattern_mismatch(): - """ - We currently don't use the format with other values than .+ -- i.e. not - empty. It's easiest to write a unit test that ensures it does the correct - thing for both cases. - """ - v = jsonschema.Draft202012Validator( - {"type": "string", "pattern": "foo"}, - format_checker=jsonschema.Draft202012Validator.FORMAT_CHECKER, - ) - - with pytest.raises(jsonschema.ValidationError) as ei: - v.validate("bar") - - assert [ - "tool.hatch.metadata.hooks.fancy-pypi-readme.some-field: 'bar' does " - "not match 'foo'" - ] == errors_to_human_strings([ei.value], ("some-field",)) From c814d3a574a87c8932cfa94b08a6be566820e287 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 10:51:35 +0200 Subject: [PATCH 4/6] Fixes --- src/hatch_fancy_pypi_readme/_config.py | 2 +- src/hatch_fancy_pypi_readme/_fragments.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hatch_fancy_pypi_readme/_config.py b/src/hatch_fancy_pypi_readme/_config.py index 2a681f5..01b2384 100644 --- a/src/hatch_fancy_pypi_readme/_config.py +++ b/src/hatch_fancy_pypi_readme/_config.py @@ -62,7 +62,7 @@ def load_and_validate_config(config: dict[str, Any]) -> Config: Draft202012Validator( SCHEMA, format_checker=Draft202012Validator.FORMAT_CHECKER ).iter_errors(config), - key=jsonschema.exceptions.relevance, # type: ignore[no-any-return] + key=jsonschema.exceptions.relevance, ) if errs: raise ConfigurationError(errors_to_human_strings(errs)) diff --git a/src/hatch_fancy_pypi_readme/_fragments.py b/src/hatch_fancy_pypi_readme/_fragments.py index 5651d92..4925882 100644 --- a/src/hatch_fancy_pypi_readme/_fragments.py +++ b/src/hatch_fancy_pypi_readme/_fragments.py @@ -38,9 +38,9 @@ "$schema": Draft202012Validator.META_SCHEMA["$id"], "type": "object", "properties": { - "path": {"type": "string", "pattern": ".+"}, - "start-after": {"type": "string", "pattern": ".+"}, - "end-before": {"type": "string", "pattern": ".+"}, + "path": {"type": "string", "minLength": 1}, + "start-after": {"type": "string", "minLength": 1}, + "end-before": {"type": "string", "minLength": 1}, "pattern": {"type": "string", "format": "regex"}, }, "required": ["path"], From 98317ea9eab3d242eadf0e78c69c5f3524f95e48 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 10:56:32 +0200 Subject: [PATCH 5/6] Sort fragment errors by relevance too --- src/hatch_fancy_pypi_readme/_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hatch_fancy_pypi_readme/_config.py b/src/hatch_fancy_pypi_readme/_config.py index 01b2384..1c649de 100644 --- a/src/hatch_fancy_pypi_readme/_config.py +++ b/src/hatch_fancy_pypi_readme/_config.py @@ -98,7 +98,10 @@ def _load_fragments(config: list[dict[str, str]]) -> list[Fragment]: continue try: - ves = tuple(frag.validator.iter_errors(frag_cfg)) + ves = sorted( + frag.validator.iter_errors(frag_cfg), + key=jsonschema.exceptions.relevance, + ) if ves: raise ConfigurationError( errors_to_human_strings(ves, ("fragments", i)) From 5ab5602db8e0c32a9872027fb188942094b899d7 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 10 Sep 2022 11:54:09 +0200 Subject: [PATCH 6/6] Use a custom validator Co-authored-by: Julian Berman <329822+Julian@users.noreply.github.com> --- src/hatch_fancy_pypi_readme/_config.py | 11 ++-- src/hatch_fancy_pypi_readme/_fragments.py | 19 +++--- .../_humanize_validation_errors.py | 1 + src/hatch_fancy_pypi_readme/_validators.py | 41 ++++++++++++ tests/test_config.py | 2 +- tests/test_validators.py | 62 +++++++++++++++++++ 6 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 src/hatch_fancy_pypi_readme/_validators.py create mode 100644 tests/test_validators.py diff --git a/src/hatch_fancy_pypi_readme/_config.py b/src/hatch_fancy_pypi_readme/_config.py index 1c649de..dba5edc 100644 --- a/src/hatch_fancy_pypi_readme/_config.py +++ b/src/hatch_fancy_pypi_readme/_config.py @@ -9,11 +9,10 @@ import jsonschema -from jsonschema import Draft202012Validator - from ._fragments import VALID_FRAGMENTS, Fragment from ._humanize_validation_errors import errors_to_human_strings from ._substitutions import Substituter +from ._validators import CustomValidator from .exceptions import ConfigurationError @@ -25,7 +24,7 @@ class Config: SCHEMA = { - "$schema": Draft202012Validator.META_SCHEMA["$id"], + "$schema": CustomValidator.META_SCHEMA["$id"], "type": "object", "properties": { "content-type": { @@ -43,7 +42,7 @@ class Config: "items": { "type": "object", "properties": { - "pattern": {"type": "string", "format": "regex"}, + "pattern": {"type": "string", "regex": True}, "replacement": {"type": "string"}, "ignore-case": {"type": "boolean"}, }, @@ -59,9 +58,7 @@ class Config: def load_and_validate_config(config: dict[str, Any]) -> Config: errs = sorted( - Draft202012Validator( - SCHEMA, format_checker=Draft202012Validator.FORMAT_CHECKER - ).iter_errors(config), + CustomValidator(SCHEMA).iter_errors(config), key=jsonschema.exceptions.relevance, ) if errs: diff --git a/src/hatch_fancy_pypi_readme/_fragments.py b/src/hatch_fancy_pypi_readme/_fragments.py index 4925882..8eff677 100644 --- a/src/hatch_fancy_pypi_readme/_fragments.py +++ b/src/hatch_fancy_pypi_readme/_fragments.py @@ -11,7 +11,7 @@ from pathlib import Path from typing import ClassVar, Iterable -from jsonschema import Draft202012Validator, Validator +from jsonschema import Validator if sys.version_info >= (3, 8): @@ -19,34 +19,33 @@ else: from typing_extensions import Protocol +from ._validators import CustomValidator from .exceptions import ConfigurationError -TEXT_V = Draft202012Validator( +TEXT_V = CustomValidator( { - "$schema": Draft202012Validator.META_SCHEMA["$id"], + "$schema": CustomValidator.META_SCHEMA["$id"], "type": "object", "properties": {"text": {"type": "string", "minLength": 1}}, "required": ["text"], "additionalProperties": False, - }, - format_checker=Draft202012Validator.FORMAT_CHECKER, + } ) -FILE_V = Draft202012Validator( +FILE_V = CustomValidator( { - "$schema": Draft202012Validator.META_SCHEMA["$id"], + "$schema": CustomValidator.META_SCHEMA["$id"], "type": "object", "properties": { "path": {"type": "string", "minLength": 1}, "start-after": {"type": "string", "minLength": 1}, "end-before": {"type": "string", "minLength": 1}, - "pattern": {"type": "string", "format": "regex"}, + "pattern": {"type": "string", "regex": True}, }, "required": ["path"], "additionalProperties": False, - }, - format_checker=Draft202012Validator.FORMAT_CHECKER, + } ) diff --git a/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py index 772f2eb..6b333a9 100644 --- a/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py +++ b/src/hatch_fancy_pypi_readme/_humanize_validation_errors.py @@ -82,4 +82,5 @@ def _dot_path(path: FieldPath, last: str | None = None) -> str: "additionalProperties": _format_additional_fields, "format": _format_path_w_message, "enum": _format_path_w_message, + "regex": _format_path_w_message, } diff --git a/src/hatch_fancy_pypi_readme/_validators.py b/src/hatch_fancy_pypi_readme/_validators.py new file mode 100644 index 0000000..9470c09 --- /dev/null +++ b/src/hatch_fancy_pypi_readme/_validators.py @@ -0,0 +1,41 @@ +# SPDX-FileCopyrightText: 2022 Hynek Schlawack +# +# SPDX-License-Identifier: MIT + +""" +This defines our CustomValidator that uses the correct draft and adds custom +validators. +""" + +from __future__ import annotations + +import re + +from typing import Any, Generator + +import jsonschema + + +def is_regex( + validator: jsonschema.validators.Draft202012Validator, + regex: bool, # value from schema + instance: Any, + schema: jsonschema.Schema, +) -> Generator[jsonschema.ValidationError, None, None]: + try: + re.compile(instance) + except (re.error, ValueError): + if regex: + yield jsonschema.ValidationError( + f"'{instance}' is not a valid Python regular expression" + ) + else: + if not regex: + yield jsonschema.ValidationError( + f"'{instance}' is a valid Python regular expression" + ) + + +CustomValidator = jsonschema.validators.extend( + jsonschema.validators.Draft202012Validator, validators={"regex": is_regex} +) diff --git a/tests/test_config.py b/tests/test_config.py index f8322cb..5facc5a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -231,7 +231,7 @@ def test_pattern_no_valid_regexp(self): assert { "tool.hatch.metadata.hooks.fancy-pypi-readme.substitutions.0." - "pattern: 'foo???' is not a 'regex'", + "pattern: 'foo???' is not a valid Python regular expression", } == set(ei.value.errors) def test_replacement_not_a_string(self): diff --git a/tests/test_validators.py b/tests/test_validators.py new file mode 100644 index 0000000..dc67f49 --- /dev/null +++ b/tests/test_validators.py @@ -0,0 +1,62 @@ +# SPDX-FileCopyrightText: 2022 Hynek Schlawack +# +# SPDX-License-Identifier: MIT + +import pytest + +from jsonschema import ValidationError + +from hatch_fancy_pypi_readme._validators import CustomValidator + + +validator = CustomValidator({"type": "string", "regex": True}) +validator_false = CustomValidator({"type": "string", "regex": False}) + + +class TestIsRegexValidator: + def test_ok_true(self): + """ + A valid regex passes. + """ + validator.validate(".+") + + def test_fail_true(self): + """ + An invalid regex fails. + """ + with pytest.raises(ValidationError) as ei: + validator.validate("???") + + assert ( + "'???' is not a valid Python regular expression" + == ei.value.message + ) + + def test_ok_false(self): + """ + If a string must not be valid regex and it isn't, it passes. + + No, I don't know when this could ever be useful. + """ + validator_false.validate("???") + + def test_fail_false(self): + """ + If a string must not be valid regex and it is, it fails. + + No, I don't know when this could ever be useful. + """ + with pytest.raises(ValidationError) as ei: + validator_false.validate(".+") + + assert "'.+' is a valid Python regular expression" == ei.value.message + + @pytest.mark.parametrize("v", [validator, validator_false]) + def test_not_a_string(self, v): + """ + Non-strings are not regexes, but also not strings that aren't regexes. + """ + with pytest.raises(ValidationError) as ei: + v.validate(42) + + assert "42 is not of type 'string'" == ei.value.message