diff --git a/myst_parser/config/main.py b/myst_parser/config/main.py index 8331481f..bb637707 100644 --- a/myst_parser/config/main.py +++ b/myst_parser/config/main.py @@ -587,17 +587,34 @@ def merge_file_level( old_value, field = fields[name] + if field.metadata.get("global_only"): + warning( + MystWarnings.MD_TOPMATTER, + f"'{name}' is a global-only config and cannot be set in topmatter", + ) + continue + + # Assign before validating, and let the validator have the last word. + # A validator may normalise what it is given and write the result back + # on the instance -- check_fence_as_directive turns the sequence into a + # set, check_url_schemes turns the list form into a dict. + # Validating first and assigning afterwards overwrote that with the raw + # topmatter value. This is also the order __post_init__ uses, where the + # value is already on the instance before validate_fields runs. try: + if field.metadata.get("merge_topmatter"): + # Validate the topmatter value itself before merging it into + # the global mapping, so that a non-mapping (a list, a string, + # null) is reported as a warning rather than raised. + validate_field(new, field, value) + value = {**old_value, **value} + setattr(new, name, value) validate_field(new, field, value) except Exception as exc: + setattr(new, name, old_value) warning(MystWarnings.MD_TOPMATTER, str(exc)) continue - if field.metadata.get("merge_topmatter"): - value = {**old_value, **value} - - setattr(new, name, value) - return new diff --git a/tests/test_config_topmatter.py b/tests/test_config_topmatter.py new file mode 100644 index 00000000..58be4461 --- /dev/null +++ b/tests/test_config_topmatter.py @@ -0,0 +1,242 @@ +"""Tests for merging file-level topmatter into the global config.""" + +import io + +import pytest +from docutils.core import publish_doctree + +from myst_parser.config.main import MdParserConfig, merge_file_level +from myst_parser.parsers.docutils_ import Parser +from myst_parser.warnings_ import MystWarnings + + +def merge(topmatter): + """Merge topmatter into a default config, returning (config, warnings).""" + warnings: list[tuple[MystWarnings, str]] = [] + config = merge_file_level( + MdParserConfig(), topmatter, lambda kind, msg: warnings.append((kind, msg)) + ) + return config, [msg for _kind, msg in warnings] + + +class TestValidatorNormalisation: + """A validator that normalises its value must have the last word. + + ``validate_field`` was called *before* ``setattr``, so a validator that + resolved or converted the value and wrote it back on the instance had that + result immediately overwritten with the raw topmatter value. + """ + + def test_a_sequence_is_stored_as_the_set_the_field_holds(self): + """`check_fence_as_directive` converts to a set; the raw value is a list. + + Downstream code does set operations on this field, so a list here is a + wrong type, not merely a different one. + """ + config, warnings = merge({"myst": {"fence_as_directive": ["mermaid"]}}) + assert warnings == [] + assert isinstance(config.fence_as_directive, set) + assert config.fence_as_directive == {"mermaid"} + + def test_the_global_value_is_a_set_too(self): + """Pins the invariant the test above is protecting.""" + assert isinstance(MdParserConfig().fence_as_directive, set) + + +class TestGlobalOnlyFields: + """``global_only`` metadata must actually be enforced. + + It was declared on five fields and checked nowhere, so topmatter could set + them. For ``heading_slug_func`` that produced + ``'str' object is not callable`` for every heading, because the field then + held the preset *name* rather than the function it names. + """ + + @pytest.mark.parametrize( + "name,value", + [ + ("heading_slug_func", "github"), + ("update_mathjax", False), + ("mathjax_classes", "tex2jax_process"), + ("suppress_warnings", ["myst.xref_missing"]), + ("inventories", {"x": ["https://example.com", None]}), + ], + ) + def test_a_global_only_field_is_rejected(self, name, value): + config, warnings = merge({"myst": {name: value}}) + assert warnings == [ + f"'{name}' is a global-only config and cannot be set in topmatter" + ] + assert getattr(config, name) == getattr(MdParserConfig(), name) + + def test_the_rejected_field_keeps_the_global_value(self): + """Not just unchanged from the default -- unchanged from the global.""" + warnings: list[str] = [] + global_config = MdParserConfig(heading_slug_func=str.upper) + config = merge_file_level( + global_config, + {"myst": {"heading_slug_func": "github"}}, + lambda kind, msg: warnings.append(msg), + ) + assert config.heading_slug_func is str.upper + assert len(warnings) == 1 + + def test_a_non_global_field_alongside_one_is_still_applied(self): + """Rejecting one key must not abandon the rest of the topmatter.""" + config, warnings = merge( + { + "myst": { + "heading_slug_func": "github", + "enable_extensions": ["dollarmath"], + } + } + ) + assert len(warnings) == 1 + assert "dollarmath" in config.enable_extensions + + +class TestInvalidValues: + """An invalid value must be reported and must not be half-applied.""" + + def test_an_invalid_value_is_rolled_back(self): + """The value is now assigned before validation, so it has to be undone. + + Otherwise a rejected value would be reported *and* kept. + """ + config, warnings = merge({"myst": {"fence_as_directive": "not-a-sequence"}}) + assert len(warnings) == 1 + assert config.fence_as_directive == MdParserConfig().fence_as_directive + + def test_an_invalid_value_rolls_back_to_the_global_not_the_default(self): + warnings: list[str] = [] + global_config = MdParserConfig(fence_as_directive={"mermaid"}) + config = merge_file_level( + global_config, + {"myst": {"fence_as_directive": "not-a-sequence"}}, + lambda kind, msg: warnings.append(msg), + ) + assert len(warnings) == 1 + assert config.fence_as_directive == {"mermaid"} + + def test_an_unknown_field_is_reported(self): + config, warnings = merge({"myst": {"not_a_field": 1}}) + assert warnings == ["Unknown field: not_a_field"] + assert not hasattr(config, "not_a_field") + + +class TestUnaffectedBehaviour: + """Guards for the paths the reordering runs through.""" + + def test_a_plain_field_is_applied(self): + config, warnings = merge({"myst": {"enable_extensions": ["dollarmath"]}}) + assert warnings == [] + assert "dollarmath" in config.enable_extensions + + def test_merge_topmatter_still_merges(self): + """`substitutions` merges with the global value rather than replacing.""" + warnings: list[str] = [] + global_config = MdParserConfig(substitutions={"a": "global"}) + config = merge_file_level( + global_config, + {"myst": {"substitutions": {"b": "file"}}}, + lambda kind, msg: warnings.append(msg), + ) + assert warnings == [] + assert config.substitutions == {"a": "global", "b": "file"} + + def test_the_file_level_value_wins_on_a_merged_field(self): + warnings: list[str] = [] + global_config = MdParserConfig(substitutions={"a": "global"}) + config = merge_file_level( + global_config, + {"myst": {"substitutions": {"a": "file"}}}, + lambda kind, msg: warnings.append(msg), + ) + assert config.substitutions == {"a": "file"} + + def test_the_global_config_is_not_mutated(self): + global_config = MdParserConfig() + merge_file_level( + global_config, + {"myst": {"enable_extensions": ["dollarmath"]}}, + lambda k, m: None, + ) + assert "dollarmath" not in global_config.enable_extensions + + def test_a_non_dict_myst_key_is_reported(self): + _config, warnings = merge({"myst": ["not", "a", "dict"]}) + assert len(warnings) == 1 + assert "not a dict" in warnings[0] + + +class TestMergedFieldsRejectNonMappings: + """A ``merge_topmatter`` field given a non-mapping must warn, not raise. + + Merging ``{**global, **value}`` before validating ``value`` raised + ``TypeError`` out of ``merge_file_level``, which aborted the whole build + (docutils or Sphinx) where a warning is expected. + """ + + @pytest.mark.parametrize("name", ["substitutions", "html_meta"]) + @pytest.mark.parametrize("value", [["a", "b"], "just-a-string", 3, None], ids=repr) + def test_a_non_mapping_is_reported_and_the_global_kept(self, name, value): + warnings: list[tuple[MystWarnings, str]] = [] + global_config = MdParserConfig(**{name: {"a": "global"}}) + config = merge_file_level( + global_config, + {"myst": {name: value}}, + lambda kind, msg: warnings.append((kind, msg)), + ) + assert len(warnings) == 1 + kind, msg = warnings[0] + assert kind is MystWarnings.MD_TOPMATTER + assert f"'{name}' must be of type" in msg + assert getattr(config, name) == {"a": "global"} + + def test_a_non_mapping_does_not_abort_a_docutils_build(self): + """End to end: the document is still built, with one topmatter warning.""" + stream = io.StringIO() + doctree = publish_doctree( + source="---\nmyst:\n substitutions: [a, b]\n---\n\ntext\n", + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + assert "'substitutions' must be of type" in stream.getvalue() + assert "[myst.topmatter]" in stream.getvalue() + assert "text" in doctree.astext() + + +class TestUrlSchemes: + """``check_url_schemes`` converts both of its accepted forms to a dict. + + The converted form has to survive the merge: the raw list form, left on + the config, crashed the build with ``list indices must be integers``. + """ + + def test_the_list_form_becomes_a_dict(self): + config, warnings = merge({"myst": {"url_schemes": ["wiki"]}}) + assert warnings == [] + assert config.url_schemes == {"wiki": None} + + def test_the_dict_form_keeps_its_templates(self): + config, warnings = merge( + {"myst": {"url_schemes": {"wiki": "https://w.org/{{path}}"}}} + ) + assert warnings == [] + assert config.url_schemes == {"wiki": {"url": "https://w.org/{{path}}"}} + + +class TestGlobalOnlyWarningKind: + """The global-only rejection is a ``myst.topmatter`` warning. + + So it is reported under that code, and can be suppressed by it. + """ + + def test_the_warning_is_a_topmatter_warning(self): + warnings: list[tuple[MystWarnings, str]] = [] + merge_file_level( + MdParserConfig(), + {"myst": {"heading_slug_func": "github"}}, + lambda kind, msg: warnings.append((kind, msg)), + ) + assert [kind for kind, _msg in warnings] == [MystWarnings.MD_TOPMATTER]