👌 Change non-fatal directive parsing errors to warnings (#682)

For non-fatal errors, such as;
faulty options syntax, unknown option keys, and invalid option values,
a warning is raised, but the directive is still run (without the erroneous options).
The warning is given the `myst.directive_parse` type, which can be suppressed.

Author: chrisjsewell

docs/conf.py

@@ -79,6 +79,7 @@
     ("py:class", "StringList"),
     ("py:class", "DocutilsRenderer"),
     ("py:class", "MockStateMachine"),
+    ("py:exc", "MarkupError"),
 ]
 
 # -- MyST settings ---------------------------------------------------

myst_parser/mdit_to_docutils/base.py

@@ -52,7 +52,7 @@
     MockState,
     MockStateMachine,
 )
-from myst_parser.parsers.directives import DirectiveParsingError, parse_directive_text
+from myst_parser.parsers.directives import MarkupError, parse_directive_text
 from myst_parser.warnings_ import MystWarnings, create_warning
 from .html_to_nodes import html_to_nodes
 from .utils import is_external_url
@@ -1462,19 +1462,16 @@ def run_directive(
         :param position: The line number of the first line
 
         """
-        # TODO directive name white/black lists
-
         self.document.current_line = position
 
         # get directive class
-        output: tuple[Directive, list] = directives.directive(
+        output: tuple[Directive | None, list] = directives.directive(
             name, self.language_module_rst, self.document
         )
         directive_class, messages = output
         if not directive_class:
             error = self.reporter.error(
                 f'Unknown directive type "{name}".\n',
-                # nodes.literal_block(content, content),
                 line=position,
             )
             return [error] + messages
@@ -1486,26 +1483,32 @@ def run_directive(
             directive_class.option_spec["relative-docs"] = directives.path
 
         try:
-            arguments, options, body_lines, content_offset = parse_directive_text(
-                directive_class, first_line, content
-            )
-        except DirectiveParsingError as error:
+            parsed = parse_directive_text(directive_class, first_line, content)
+        except MarkupError as error:
             error = self.reporter.error(
                 f"Directive '{name}': {error}",
-                nodes.literal_block(content, content),
                 line=position,
             )
             return [error]
 
+        if parsed.warnings:
+            _errors = ",\n".join(parsed.warnings)
+            self.create_warning(
+                f"{name!r}: {_errors}",
+                MystWarnings.DIRECTIVE_PARSING,
+                line=position,
+                append_to=self.current_node,
+            )
+
         # initialise directive
         if issubclass(directive_class, Include):
             directive_instance = MockIncludeDirective(
                 self,
                 name=name,
                 klass=directive_class,
-                arguments=arguments,
-                options=options,
-                body=body_lines,
+                arguments=parsed.arguments,
+                options=parsed.options,
+                body=parsed.body,
                 lineno=position,
             )
         else:
@@ -1514,17 +1517,17 @@ def run_directive(
             directive_instance = directive_class(
                 name=name,
                 # the list of positional arguments
-                arguments=arguments,
+                arguments=parsed.arguments,
                 # a dictionary mapping option names to values
-                options=options,
+                options=parsed.options,
                 # the directive content line by line
-                content=StringList(body_lines, self.document["source"]),
+                content=StringList(parsed.body, self.document["source"]),
                 # the absolute line number of the first line of the directive
                 lineno=position,
                 # the line offset of the first line of the content
-                content_offset=content_offset,
+                content_offset=parsed.body_offset,
                 # a string containing the entire directive
-                block_text="\n".join(body_lines),
+                block_text="\n".join(parsed.body),
                 state=state,
                 state_machine=state_machine,
             )

myst_parser/mocking.py

@@ -17,7 +17,7 @@
 from docutils.statemachine import StringList
 from docutils.utils import unescape
 
-from .parsers.directives import parse_directive_text
+from .parsers.directives import MarkupError, parse_directive_text
 
 if TYPE_CHECKING:
     from .mdit_to_docutils.base import DocutilsRenderer
@@ -133,19 +133,21 @@ def parse_directive_block(
     ) -> tuple[list, dict, StringList, int]:
         """Parse the full directive text
 
+        :raises MarkupError: for errors in parsing the directive
         :returns: (arguments, options, content, content_offset)
         """
+        # note this is essentially only used by the docutils `role` directive
         if option_presets:
             raise MockingError("parse_directive_block: option_presets not implemented")
         # TODO should argument_str always be ""?
-        arguments, options, body_lines, content_offset = parse_directive_text(
-            directive, "", "\n".join(content)
-        )
+        parsed = parse_directive_text(directive, "", "\n".join(content))
+        if parsed.warnings:
+            raise MarkupError(",".join(parsed.warnings))
         return (
-            arguments,
-            options,
-            StringList(body_lines, source=content.source),
-            line_offset + content_offset,
+            parsed.arguments,
+            parsed.options,
+            StringList(parsed.body, source=content.source),
+            line_offset + parsed.body_offset,
         )
 
     def nested_parse(

myst_parser/parsers/directives.py

@@ -37,39 +37,51 @@
 
 import datetime
 import re
+from dataclasses import dataclass
 from textwrap import dedent
 from typing import Any, Callable
 
 import yaml
 from docutils.parsers.rst import Directive
 from docutils.parsers.rst.directives.misc import TestDirective
+from docutils.parsers.rst.states import MarkupError
 
 
-class DirectiveParsingError(Exception):
-    """Raise on parsing/validation error."""
-
-    pass
+@dataclass
+class DirectiveParsingResult:
+    arguments: list[str]
+    """The arguments parsed from the first line."""
+    options: dict
+    """Options parsed from the YAML block."""
+    body: list[str]
+    """The lines of body content"""
+    body_offset: int
+    """The number of lines to the start of the body content."""
+    warnings: list[str]
+    """List of non-fatal errors encountered during parsing."""
 
 
 def parse_directive_text(
     directive_class: type[Directive],
     first_line: str,
     content: str,
     validate_options: bool = True,
-) -> tuple[list[str], dict, list[str], int]:
+) -> DirectiveParsingResult:
     """Parse (and validate) the full directive text.
 
     :param first_line: The text on the same line as the directive name.
         May be an argument or body text, dependent on the directive
     :param content: All text after the first line. Can include options.
     :param validate_options: Whether to validate the values of options
 
-    :returns: (arguments, options, body_lines, content_offset)
+    :raises MarkupError: if there is a fatal parsing/validation error
     """
+    parse_errors: list[str] = []
     if directive_class.option_spec:
-        body, options = parse_directive_options(
+        body, options, option_errors = parse_directive_options(
             content, directive_class, validate=validate_options
         )
+        parse_errors.extend(option_errors)
         body_lines = body.splitlines()
         content_offset = len(content.splitlines()) - len(body_lines)
     else:
@@ -94,16 +106,22 @@ def parse_directive_text(
 
     # check for body content
     if body_lines and not directive_class.has_content:
-        raise DirectiveParsingError("No content permitted")
+        parse_errors.append("Has content, but none permitted")
 
-    return arguments, options, body_lines, content_offset
+    return DirectiveParsingResult(
+        arguments, options, body_lines, content_offset, parse_errors
+    )
 
 
 def parse_directive_options(
     content: str, directive_class: type[Directive], validate: bool = True
-):
-    """Parse (and validate) the directive option section."""
+) -> tuple[str, dict, list[str]]:
+    """Parse (and validate) the directive option section.
+
+    :returns: (content, options, validation_errors)
+    """
     options: dict[str, Any] = {}
+    validation_errors: list[str] = []
     if content.startswith("---"):
         content = "\n".join(content.splitlines()[1:])
         match = re.search(r"^-{3,}", content, re.MULTILINE)
@@ -116,8 +134,8 @@ def parse_directive_options(
         yaml_block = dedent(yaml_block)
         try:
             options = yaml.safe_load(yaml_block) or {}
-        except (yaml.parser.ParserError, yaml.scanner.ScannerError) as error:
-            raise DirectiveParsingError("Invalid options YAML: " + str(error))
+        except (yaml.parser.ParserError, yaml.scanner.ScannerError):
+            validation_errors.append("Invalid options format (bad YAML)")
     elif content.lstrip().startswith(":"):
         content_lines = content.splitlines()  # type: list
         yaml_lines = []
@@ -129,62 +147,75 @@ def parse_directive_options(
         content = "\n".join(content_lines)
         try:
             options = yaml.safe_load(yaml_block) or {}
-        except (yaml.parser.ParserError, yaml.scanner.ScannerError) as error:
-            raise DirectiveParsingError("Invalid options YAML: " + str(error))
-        if not isinstance(options, dict):
-            raise DirectiveParsingError(f"Invalid options (not dict): {options}")
+        except (yaml.parser.ParserError, yaml.scanner.ScannerError):
+            validation_errors.append("Invalid options format (bad YAML)")
+
+    if not isinstance(options, dict):
+        options = {}
+        validation_errors.append("Invalid options format (not a dict)")
+
+    if validation_errors:
+        return content, options, validation_errors
 
     if (not validate) or issubclass(directive_class, TestDirective):
         # technically this directive spec only accepts one option ('option')
         # but since its for testing only we accept all options
-        return content, options
+        return content, options, validation_errors
 
     # check options against spec
     options_spec: dict[str, Callable] = directive_class.option_spec
-    for name, value in list(options.items()):
+    unknown_options: list[str] = []
+    new_options: dict[str, Any] = {}
+    for name, value in options.items():
         try:
             convertor = options_spec[name]
         except KeyError:
-            raise DirectiveParsingError(f"Unknown option: {name}")
+            unknown_options.append(name)
+            continue
         if not isinstance(value, str):
             if value is True or value is None:
                 value = None  # flag converter requires no argument
             elif isinstance(value, (int, float, datetime.date, datetime.datetime)):
                 # convertor always requires string input
                 value = str(value)
             else:
-                raise DirectiveParsingError(
+                validation_errors.append(
                     f'option "{name}" value not string (enclose with ""): {value}'
                 )
+                continue
         try:
             converted_value = convertor(value)
         except (ValueError, TypeError) as error:
-            raise DirectiveParsingError(
-                "Invalid option value: (option: '{}'; value: {})\n{}".format(
-                    name, value, error
-                )
+            validation_errors.append(
+                f"Invalid option value for {name!r}: {value}: {error}"
             )
-        options[name] = converted_value
+        else:
+            new_options[name] = converted_value
+
+    if unknown_options:
+        validation_errors.append(
+            f"Unknown option keys: {sorted(unknown_options)} "
+            f"(allowed: {sorted(options_spec)})"
+        )
 
-    return content, options
+    return content, new_options, validation_errors
 
 
-def parse_directive_arguments(directive, arg_text):
+def parse_directive_arguments(
+    directive_cls: type[Directive], arg_text: str
+) -> list[str]:
     """Parse (and validate) the directive argument section."""
-    required = directive.required_arguments
-    optional = directive.optional_arguments
+    required = directive_cls.required_arguments
+    optional = directive_cls.optional_arguments
     arguments = arg_text.split()
     if len(arguments) < required:
-        raise DirectiveParsingError(
-            f"{required} argument(s) required, {len(arguments)} supplied"
-        )
+        raise MarkupError(f"{required} argument(s) required, {len(arguments)} supplied")
     elif len(arguments) > required + optional:
-        if directive.final_argument_whitespace:
+        if directive_cls.final_argument_whitespace:
             arguments = arg_text.split(None, required + optional - 1)
         else:
-            raise DirectiveParsingError(
-                "maximum {} argument(s) allowed, {} supplied".format(
-                    required + optional, len(arguments)
-                )
+            raise MarkupError(
+                f"maximum {required + optional} argument(s) allowed, "
+                f"{len(arguments)} supplied"
             )
     return arguments

myst_parser/warnings_.py

@@ -29,6 +29,9 @@ class MystWarnings(Enum):
     MD_HEADING_NESTED = "nested_header"
     """Header found nested in another element."""
 
+    DIRECTIVE_PARSING = "directive_parse"
+    """Issue parsing directive."""
+
     # cross-reference resolution
     XREF_AMBIGUOUS = "xref_ambiguous"
     """Multiple targets were found for a cross-reference."""

tests/test_renderers/fixtures/directive_options.md

@@ -131,15 +131,13 @@ foo
 ```
 .
 <document source="<src>/index.md">
-    <system_message level="3" line="1" source="<src>/index.md" type="ERROR">
+    <system_message level="2" line="1" source="<src>/index.md" type="WARNING">
+        <paragraph>
+            'restructuredtext-test-directive': Invalid options format (bad YAML) [myst.directive_parse]
+    <system_message level="1" line="1" source="<src>/index.md" type="INFO">
         <paragraph>
-            Directive 'restructuredtext-test-directive': Invalid options YAML: mapping values are not allowed here
-              in "<unicode string>", line 2, column 8:
-                option2: b
-                       ^
+            Directive processed. Type="restructuredtext-test-directive", arguments=[], options={}, content:
         <literal_block xml:space="preserve">
-            :option1
-            :option2: b
             foo
 .