From 9f36bb54efbbdae262982cdbe8dabfd659a4b519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= Date: Sun, 13 Sep 2026 13:51:47 +0000 Subject: [PATCH] refactor: relax all plugins validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Timothée Mazzucotelli --- python/tests/integration/test_tags.py | 59 ++++---- python/tests/unit/test_config.py | 46 +++++- python/tests/unit/test_plugin_config.py | 136 ++++++++++++++++-- python/zensical/config.py | 181 +++++++++++++++--------- 4 files changed, 319 insertions(+), 103 deletions(-) diff --git a/python/tests/integration/test_tags.py b/python/tests/integration/test_tags.py index 1c92141..4bd3822 100644 --- a/python/tests/integration/test_tags.py +++ b/python/tests/integration/test_tags.py @@ -31,6 +31,7 @@ from typing import TYPE_CHECKING, Any import pytest import zensical +from zensical.config import ConfigurationError, parse_config if TYPE_CHECKING: from pathlib import Path @@ -549,37 +550,47 @@ def test_leading_hierarchy_separator_keeps_identity_and_listing_link( @pytest.mark.parametrize( - ("option", "replacement"), + "option", [ - ("tags_compare", "tags_sort_by"), - ("tags_compare_reverse", "tags_sort_reverse"), - ("tags_pages_compare", "listings_sort_by"), - ("tags_pages_compare_reverse", "listings_sort_reverse"), - ("tags_file", "material/tags"), - ("tags_extra_files", "material/tags"), + "tags_compare", + "tags_compare_reverse", + "tags_pages_compare", + "tags_pages_compare_reverse", + "tags_file", + "tags_extra_files", + "export", + "export_file", + "export_only", ], ) -def test_rust_rejects_deprecated_tags_options( - tmp_path: Path, option: str, replacement: str -) -> None: - """The native configuration boundary owns deprecated-option errors.""" - config = _write_project(tmp_path, plugin=f" {option}: value\n") - - with pytest.raises(ValueError, match=option) as error: - zensical.build(str(config), _BUILD_OPTIONS) - - assert replacement in str(error.value) - - -@pytest.mark.parametrize("option", ["export_only", "tags_hierachy"]) -def test_rust_rejects_unsupported_tags_options( +def test_ignores_unimplemented_tags_options( tmp_path: Path, option: str ) -> None: - """Unsupported behavior and misspellings cannot silently disappear.""" + """Legacy options never reach Rust and do not suppress native listings.""" config = _write_project(tmp_path, plugin=f" {option}: true\n") - with pytest.raises(ValueError, match=option): - zensical.build(str(config), _BUILD_OPTIONS) + zensical.build(str(config), _BUILD_OPTIONS) + + listing = (tmp_path / "site" / "index.html").read_text() + assert '

' in listing + assert "Rust page" in listing + assert not (tmp_path / "site" / "tags.json").exists() + assert not (tmp_path / "site" / "ignored-tags.json").exists() + + +@pytest.mark.parametrize("option", ["unknown", "tags_hierachy"]) +def test_config_rejects_unknown_tags_options( + tmp_path: Path, option: str +) -> None: + """Unknown options fail even when known legacy options are ignored.""" + config = _write_project( + tmp_path, plugin=f" export_only: true\n {option}: true\n" + ) + + with pytest.raises( + ConfigurationError, match=rf"unknown tags option: {option}" + ): + parse_config(str(config)) def test_scalar_configuration_and_metadata_match_python_names( diff --git a/python/tests/unit/test_config.py b/python/tests/unit/test_config.py index 8da3b33..e437e49 100644 --- a/python/tests/unit/test_config.py +++ b/python/tests/unit/test_config.py @@ -275,6 +275,45 @@ class TestPluginShimming: config = self._parse_yaml(tmp_path, plugins={"glightbox": {}}) assert GlightboxExtension.name in config["markdown_extensions"] + def test_ignored_plugin_settings_do_not_affect_hashes_or_shims( + self, tmp_path: Path + ) -> None: + plugins: dict[str, dict[str, Any]] = { + "autorefs": {}, + "glightbox": {"auto": False}, + "macros": {"render_by_default": False}, + "mike": {"version_selector": False}, + "mkdocstrings": {"enabled": False}, + "search": {"separator": r"\s+"}, + "material/tags": {"enabled": False}, + } + baseline = self._parse_yaml(tmp_path, plugins=plugins) + for name, options in { + "autorefs": {"link_titles": "external"}, + "glightbox": {"slide_effect": "fade"}, + "macros": {"force_render_paths": "guides/**"}, + "mike": {"javascript_dir": "scripts"}, + "mkdocstrings": {"enable_inventory": False, "watch": ["src"]}, + "search": {"lang": ["en", "fr"]}, + "material/tags": {"tags_file": "tags.md", "export_only": True}, + }.items(): + plugins[name].update(options) + plugins["external"] = {"enabled": ["not", "a", "boolean"]} + config_file = tmp_path / "mkdocs.yml" + config_file.write_text(_minimal_yaml(plugins=plugins)) + + configured = parse_config(str(config_file)) + + for key in ( + "plugins", + "plugins_hash", + "markdown_extensions", + "mdx_configs", + "mdx_configs_hash", + "watched_files", + ): + assert configured[key] == baseline[key] + @pytest.mark.parametrize( "entry", ["meta", {"meta": None}, "material/meta", {"material/meta": None}], @@ -540,10 +579,13 @@ class TestPluginShimming: self, tmp_path: Path ) -> None: config = self._parse_yaml( - tmp_path, plugins={"glightbox": {"loop": True}} + tmp_path, + plugins={"glightbox": {"width": "80%", "slide_effect": "fade"}}, ) assert GlightboxExtension.name in config["markdown_extensions"] - assert config["mdx_configs"][GlightboxExtension.name] == {"loop": True} + assert config["mdx_configs"][GlightboxExtension.name] == { + "width": "80%" + } def test_macros_plugin_shimmed(self, tmp_path: Path) -> None: config = self._parse_yaml(tmp_path, plugins={"macros": {}}) diff --git a/python/tests/unit/test_plugin_config.py b/python/tests/unit/test_plugin_config.py index f72af18..0a37682 100644 --- a/python/tests/unit/test_plugin_config.py +++ b/python/tests/unit/test_plugin_config.py @@ -118,6 +118,27 @@ def test_preserves_plugin_presence_semantics() -> None: assert not set(SHIM_PLUGINS) & set(plugins) +def test_preserves_zensical_plugin_options() -> None: + plugins = _convert_plugins( + { + "minify": { + "enabled": False, + "minify_inline_js": True, + "minify_inline_css": True, + }, + "glightbox": {"auto": False, "slide_effect": "fade"}, + "macros": {"include_yaml": {"data": "data.yml"}}, + } + ) + + minify = plugins["minify"]["config"] + assert minify["enabled"] is False + assert minify["minify_inline_js"] is True + assert minify["minify_inline_css"] is True + assert plugins["glightbox"]["config"] == {"auto": False} + assert plugins["macros"]["config"]["include_yaml"] == {"data": "data.yml"} + + def test_normalizes_mike_defaults() -> None: plugins = _convert_plugins({"mike": {}}) assert plugins["mike"]["config"] == { @@ -158,7 +179,7 @@ def test_silently_discards_unsupported_mike_options( assert capsys.readouterr().err == "" -@pytest.mark.parametrize("name", [*PYTHON_PLUGINS, "tags", "external"]) +@pytest.mark.parametrize("name", [*PYTHON_PLUGINS, "tags"]) def test_plugin_configuration_must_be_a_mapping(name: str) -> None: with pytest.raises( ConfigurationError, @@ -167,7 +188,88 @@ def test_plugin_configuration_must_be_a_mapping(name: str) -> None: _convert_plugins({name: []}) -@pytest.mark.parametrize("name", PYTHON_PLUGINS) +@pytest.mark.parametrize( + "name", + [ + "external", # does not exist + "material/blog", # exists but isn't supported yet + "literate_nav", # misspelling (`_` instead of `-`) + ], +) +@pytest.mark.parametrize("data", [None, True, 42, "config", [], {42: object()}]) +@pytest.mark.parametrize("as_list", [False, True]) +def test_ignores_unsupported_plugins( + name: str, data: Any, as_list: bool, capsys: pytest.CaptureFixture[str] +) -> None: + value = {name: data} + plugins = _convert_plugins([value] if as_list else value) + + assert plugins == _convert_plugins([]) + assert capsys.readouterr().err == "" + + +@pytest.mark.parametrize("name", ["external", "material/blog", "literate_nav"]) +def test_ignores_unsupported_plugin_names(name: str) -> None: + assert _convert_plugins([name]) == _convert_plugins([]) + + +@pytest.mark.parametrize("prefix", ["", "material/"]) +@pytest.mark.parametrize("value", [True, False, "auto", 42, [], {}, None]) +@pytest.mark.parametrize( + ("plugin", "option"), + [ + ("autorefs", "resolve_closest"), + ("autorefs", "link_titles"), + ("autorefs", "strip_title_tags"), + ("glightbox", "touchNavigation"), + ("glightbox", "loop"), + ("glightbox", "effect"), + ("glightbox", "slide_effect"), + ("glightbox", "zoomable"), + ("glightbox", "draggable"), + ("glightbox", "background"), + ("glightbox", "shadow"), + ("macros", "force_render_paths"), + ("macros", "verbose"), + ("mike", "css_dir"), + ("mike", "javascript_dir"), + ("mkdocstrings", "enable_inventory"), + ("mkdocstrings", "watch"), + ("search", "fields"), + ("search", "indexing"), + ("search", "jieba_dict"), + ("search", "jieba_dict_user"), + ("search", "lang"), + ("search", "min_search_length"), + ("search", "pipeline"), + ("search", "prebuild_index"), + ("tags", "tags_compare"), + ("tags", "tags_compare_reverse"), + ("tags", "tags_pages_compare"), + ("tags", "tags_pages_compare_reverse"), + ("tags", "tags_file"), + ("tags", "tags_extra_files"), + ("tags", "export"), + ("tags", "export_file"), + ("tags", "export_only"), + ], +) +def test_silently_discards_unimplemented_options( + plugin: str, + option: str, + value: Any, + prefix: str, + capsys: pytest.CaptureFixture[str], +) -> None: + data = {"enabled": False, option: value} + plugins = _convert_plugins({prefix + plugin: data}) + + assert plugins == _convert_plugins({plugin: {"enabled": False}}) + assert data == {"enabled": False, option: value} + assert capsys.readouterr().err == "" + + +@pytest.mark.parametrize("name", [*PYTHON_PLUGINS, "tags"]) def test_rejects_unknown_python_plugin_options(name: str) -> None: with pytest.raises( ConfigurationError, @@ -176,6 +278,21 @@ def test_rejects_unknown_python_plugin_options(name: str) -> None: _convert_plugins({name: {"unknown": True}}) +@pytest.mark.parametrize("plugin", ["table-reader", "material/table-reader"]) +@pytest.mark.parametrize( + ("option", "value"), + [("base_path", "docs_dir"), ("search_page_directory", False)], +) +def test_rejects_removed_table_reader_options( + plugin: str, option: str, value: Any +) -> None: + with pytest.raises( + ConfigurationError, + match=rf"unknown table-reader option: {option}", + ): + _convert_plugins({plugin: {option: value}}) + + @pytest.mark.parametrize("plugin", ["search", "material/search"]) def test_silently_discards_unsupported_search_options( plugin: str, capsys: pytest.CaptureFixture[str] @@ -226,7 +343,6 @@ def test_normalizes_null_shim_configuration(name: str) -> None: "enabled": False, "handlers": {"python": {"options": {}}}, "custom_templates": None, - "enable_inventory": None, "default_handler": "python", "locale": "fr", }, @@ -243,13 +359,6 @@ def test_normalizes_null_shim_configuration(name: str) -> None: "auto_themed": True, "auto_caption": True, "caption_position": "top", - "touchNavigation": False, - "loop": True, - "effect": "fade", - "zoomable": False, - "draggable": False, - "background": "black", - "shadow": False, "manual": None, }, id="glightbox", @@ -265,7 +374,6 @@ def test_normalizes_null_shim_configuration(name: str) -> None: "render_by_default": False, "on_error_fail": True, "on_undefined": "strict", - "verbose": True, "j2_block_start_string": "<%", "j2_block_end_string": "%>", "j2_variable_start_string": "<@", @@ -371,7 +479,11 @@ def test_silently_discards_unsupported_autorefs_options( "languages must be a list of supported language names", ), ("mkdocstrings", {"handlers": []}, "handlers must be a mapping"), - ("glightbox", {"effect": "slide"}, "effect must be"), + ( + "glightbox", + {"caption_position": "center"}, + "caption_position must be", + ), ("macros", {"include_yaml": [42]}, "include_yaml must be a list"), ("macros", {"on_undefined": "silent"}, "on_undefined must be"), ( diff --git a/python/zensical/config.py b/python/zensical/config.py index 2a34244..90df0e3 100644 --- a/python/zensical/config.py +++ b/python/zensical/config.py @@ -109,6 +109,104 @@ DEFAULT_MARKDOWN_EXTENSIONS = { "pymdownx.tilde": {}, } +# Supported MkDocs plugins and their recognized but unimplemented options. +# Discard these before validation, hashing and forwarding to native modules or +# Markdown extensions. Empty tuples mark plugins with no ignored options. +_PLUGIN_UNSUPPORTED_OPTIONS = { + "autorefs": ( + # TODO: Configure native URL selection and link title rendering. + "resolve_closest", + "link_titles", + "strip_title_tags", + ), + "awesome-nav": (), + "glightbox": ( + "touchNavigation", + "loop", + "effect", + "slide_effect", + "zoomable", + "draggable", + "background", + "shadow", + ), + "literate-nav": (), + "macros": ( + # TODO: Match page paths before deciding whether to render macros. + "force_render_paths", + # TODO: Add diagnostics for module loading and macro rendering. + "verbose", + ), + "markdown-exec": (), + "meta": (), + "mike": ( + "css_dir", + "javascript_dir", + ), + "minify": (), + "mkdocstrings": ( + # TODO: Gate native objects.inv generation on this setting. + "enable_inventory", + "watch", + ), + "offline": (), + "redirects": (), + "search": ( + "fields", + "indexing", + "jieba_dict", + "jieba_dict_user", + "lang", + "min_search_length", + "pipeline", + "prebuild_index", + ), + "table-reader": (), + "tags": ( + "tags_compare", + "tags_compare_reverse", + "tags_pages_compare", + "tags_pages_compare_reverse", + "tags_file", + "tags_extra_files", + "export", + "export_file", + "export_only", + ), +} + +# Tags options forwarded to the native configuration after Python removes +# recognized but unimplemented options. +_TAGS_SUPPORTED_OPTIONS = { + "enabled", + "filters", + "tags", + "tags_slugify", + "tags_slugify_separator", + "tags_slugify_format", + "tags_hierarchy", + "tags_hierarchy_separator", + "tags_sort_by", + "tags_sort_reverse", + "tags_name_property", + "tags_name_variable", + "tags_allowed", + "listings", + "listings_map", + "listings_sort_by", + "listings_sort_reverse", + "listings_tags_sort_by", + "listings_tags_sort_reverse", + "listings_directive", + "listings_layout", + "listings_toc", + "shadow", + "shadow_on_serve", + "shadow_tags", + "shadow_tags_prefix", + "shadow_tags_suffix", +} + # ---------------------------------------------------------------------------- # Classes @@ -709,11 +807,8 @@ def _apply_defaults(config: dict, path: str) -> dict: # Hash all templates, so we rebuild if something changes config["template_hash"] = _hash(theme_files) - # Hash the entire plugins configuration. - # This is a special case for plugins because we currently only source - # the plugin configuration that we support in Rust, - # which means config on other plugins doesn't contribute to the hash, - # in turn not triggering full rebuilds. + # Include Python-only plugin settings in rebuilds. Unsupported plugins and + # ignored legacy options have already been discarded during normalization. config["plugins_hash"] = _hash(config["plugins"]) return config @@ -1411,13 +1506,18 @@ def _convert_plugins(value: Any, config: dict) -> dict: if not isinstance(name, str): raise ConfigurationError("Plugin names must be strings") name = name.removeprefix("material/") + if name not in _PLUGIN_UNSUPPORTED_OPTIONS: + return if data is None: data = {} elif not isinstance(data, dict): raise ConfigurationError(f"{name} configuration must be a mapping") else: data = dict(data) + for option in _PLUGIN_UNSUPPORTED_OPTIONS[name]: + data.pop(option, None) if name == "tags": + _reject_unknown_options("tags", data, _TAGS_SUPPORTED_OPTIONS) tags.append({"name": name, "config": data}) else: plugins[name] = data @@ -1443,29 +1543,13 @@ def _convert_plugins(value: Any, config: dict) -> dict: else: raise ConfigurationError("plugins must be a list or mapping") - # Rust owns all tags defaults, validation, scalar coercion and callable - # lowering. Python only preserves ordered plugin instances and their raw - # configuration, as it does for future native compatibility modules. + # Rust owns tags defaults, value validation, scalar coercion and callable + # lowering. Python validates option names and preserves ordered instances. plugins["tags"] = tags # Search is enabled by default, even when it isn't explicitly configured. search = plugins.pop("search", {}) - supported = {"enabled", "separator"} - # Keep recognized upstream options non-fatal during migration, but discard - # them before extracting the typed native search configuration in Rust. - unsupported = { - "fields", - "indexing", - "jieba_dict", - "jieba_dict_user", - "lang", - "min_search_length", - "pipeline", - "prebuild_index", - } - _reject_unknown_options("search", search, supported | unsupported) - for name in sorted(unsupported & search.keys()): - search.pop(name) + _reject_unknown_options("search", search, {"enabled", "separator"}) set_default(search, "enabled", True) set_default(search, "separator", '[\\s\\-_,:!=\\[\\]()\\\\"`/]+|\\.(?!\\d)') _validate_boolean_options("search", search, ("enabled",)) @@ -1678,17 +1762,16 @@ def _convert_plugins(value: Any, config: dict) -> dict: "deploy_prefix": "", } nullable_strings = ("redirect_template", "canonical_version") - # Zensical bundles its own version selector assets, so accept and - # discard Mike's asset directory options for compatibility. - unsupported = {"css_dir", "javascript_dir"} _reject_unknown_options( "mike", mike, - {"enabled", "version_selector", *string_defaults, *nullable_strings} - | unsupported, + { + "enabled", + "version_selector", + *string_defaults, + *nullable_strings, + }, ) - for name in sorted(unsupported & mike.keys()): - mike.pop(name) _validate_boolean_options("mike", mike, ("enabled", "version_selector")) for name, default in string_defaults.items(): set_default(mike, name, default) @@ -1704,16 +1787,8 @@ def _convert_plugins(value: Any, config: dict) -> dict: # Validate settings forwarded by the plugin-to-extension shims. if "autorefs" in plugins: autorefs = plugins["autorefs"] - _reject_unknown_options( - "autorefs", - autorefs, - {"enabled", "resolve_closest", "link_titles", "strip_title_tags"}, - ) + _reject_unknown_options("autorefs", autorefs, {"enabled"}) _validate_boolean_options("autorefs", autorefs, ("enabled",)) - # Ignore these upstream settings: the Rust resolver currently uses - # fixed resolution and title behavior. - for name in ("resolve_closest", "link_titles", "strip_title_tags"): - autorefs.pop(name, None) if "markdown-exec" in plugins: markdown_exec = plugins["markdown-exec"] @@ -1769,7 +1844,6 @@ def _convert_plugins(value: Any, config: dict) -> dict: "enabled", "handlers", "custom_templates", - "enable_inventory", }, ) _validate_boolean_options("mkdocstrings", mkdocstrings, ("enabled",)) @@ -1789,29 +1863,16 @@ def _convert_plugins(value: Any, config: dict) -> dict: raise ConfigurationError( "mkdocstrings custom_templates must be a string or null" ) - if ( - "enable_inventory" in mkdocstrings - and mkdocstrings["enable_inventory"] is not None - and not isinstance(mkdocstrings["enable_inventory"], bool) - ): - raise ConfigurationError( - "mkdocstrings enable_inventory must be a boolean or null" - ) _validate_string_options("mkdocstrings", mkdocstrings, string_options) if "glightbox" in plugins: glightbox = plugins["glightbox"] - string_options = {"width", "height", "background"} + string_options = {"width", "height"} boolean_options = { "enabled", "auto", "auto_themed", "auto_caption", - "touchNavigation", - "loop", - "zoomable", - "draggable", - "shadow", } _reject_unknown_options( "glightbox", @@ -1821,7 +1882,6 @@ def _convert_plugins(value: Any, config: dict) -> dict: | { "skip_classes", "caption_position", - "effect", "manual", }, ) @@ -1843,14 +1903,6 @@ def _convert_plugins(value: Any, config: dict) -> dict: "glightbox caption_position must be 'bottom', 'top', 'left' " "or 'right'" ) - if "effect" in glightbox and glightbox["effect"] not in { - "zoom", - "fade", - "none", - }: - raise ConfigurationError( - "glightbox effect must be 'zoom', 'fade' or 'none'" - ) if ( "manual" in glightbox and glightbox["manual"] is not None @@ -1877,7 +1929,6 @@ def _convert_plugins(value: Any, config: dict) -> dict: "enabled", "render_by_default", "on_error_fail", - "verbose", } _reject_unknown_options( "macros",