👌 Improve directive parsing warnings (#893)

Minor improvement to the specificity of warnings regarding problems encountered when parsing directives

myst_parser/mdit_to_docutils/base.py

@@ -1758,11 +1758,11 @@ def run_directive(
             )
             return [error]
 
-        for warning_msg, warning_line in parsed.warnings:
+        for _warning in parsed.warnings:
             self.create_warning(
-                f"{name!r}: {warning_msg}",
-                MystWarnings.DIRECTIVE_PARSING,
-                line=warning_line if warning_line is not None else position,
+                f"{name!r}: {_warning.msg}",
+                _warning.type,
+                line=_warning.lineno if _warning.lineno is not None else position,
                 append_to=self.current_node,
             )
 

myst_parser/mocking.py

@@ -140,7 +140,7 @@ def parse_directive_block(
         # TODO should argument_str always be ""?
         parsed = parse_directive_text(directive, "", "\n".join(content))
         if parsed.warnings:
-            raise MarkupError(",".join(w for w, _ in parsed.warnings))
+            raise MarkupError(",".join(w.msg for w in parsed.warnings))
         return (
             parsed.arguments,
             parsed.options,

myst_parser/parsers/directives.py

@@ -46,10 +46,19 @@
 from docutils.parsers.rst.directives.misc import TestDirective
 from docutils.parsers.rst.states import MarkupError
 
+from myst_parser.warnings_ import MystWarnings
+
 from .options import TokenizeError
 from .options import to_items as options_to_items
 
 
+@dataclass
+class ParseWarnings:
+    msg: str
+    lineno: int | None = None
+    type: MystWarnings = MystWarnings.DIRECTIVE_PARSING
+
+
 @dataclass
 class DirectiveParsingResult:
     arguments: list[str]
@@ -60,7 +69,7 @@ class DirectiveParsingResult:
     """The lines of body content"""
     body_offset: int
     """The number of lines to the start of the body content."""
-    warnings: list[tuple[str, int | None]]
+    warnings: list[ParseWarnings]
     """List of non-fatal errors encountered during parsing.
     (message, line_number)
     """
@@ -88,7 +97,7 @@ def parse_directive_text(
 
     :raises MarkupError: if there is a fatal parsing/validation error
     """
-    parse_errors: list[tuple[str, int | None]]
+    parse_warnings: list[ParseWarnings]
     options: dict[str, Any]
     body_lines: list[str]
     content_offset: int
@@ -104,13 +113,13 @@ def parse_directive_text(
             as_yaml=not validate_options,
             additional_options=additional_options,
         )
-        parse_errors = result.errors
+        parse_warnings = result.warnings
         has_options_block = result.has_options
         options = result.options
         body_lines = result.content.splitlines()
         content_offset = len(content.splitlines()) - len(body_lines)
     else:
-        parse_errors = []
+        parse_warnings = []
         has_options_block = False
         options = {}
         body_lines = content.splitlines()
@@ -120,11 +129,10 @@ def parse_directive_text(
         # If there are no possible arguments, then the body can start on the argument line
         if first_line.strip():
             if has_options_block and any(body_lines):
-                parse_errors.append(
-                    (
-                        "Cannot split content across first line and body, "
-                        "when options block is present (move first line to body)",
-                        None,
+                parse_warnings.append(
+                    ParseWarnings(
+                        "Splitting content across first line and body, "
+                        "when an options block is present, is not recommended"
                     )
                 )
             body_lines.insert(0, first_line)
@@ -141,18 +149,18 @@ def parse_directive_text(
 
     # check for body content
     if body_lines and not directive_class.has_content:
-        parse_errors.append(("Has content, but none permitted", None))
+        parse_warnings.append(ParseWarnings("Has content, but none permitted"))
 
     return DirectiveParsingResult(
-        arguments, options, body_lines, content_offset, parse_errors
+        arguments, options, body_lines, content_offset, parse_warnings
     )
 
 
 @dataclass
 class _DirectiveOptions:
     content: str
     options: dict[str, Any]
-    errors: list[tuple[str, int | None]]
+    warnings: list[ParseWarnings]
     has_options: bool
 
 
@@ -195,15 +203,27 @@ def _parse_directive_options(
     has_options_block = yaml_block is not None
 
     if as_yaml:
-        yaml_errors: list[tuple[str, int | None]] = []
+        yaml_errors: list[ParseWarnings] = []
         try:
             yaml_options = yaml.safe_load(yaml_block or "") or {}
         except (yaml.parser.ParserError, yaml.scanner.ScannerError):
             yaml_options = {}
-            yaml_errors.append(("Invalid options format (bad YAML)", line))
+            yaml_errors.append(
+                ParseWarnings(
+                    "Invalid options format (bad YAML)",
+                    line,
+                    MystWarnings.DIRECTIVE_OPTION,
+                )
+            )
         if not isinstance(yaml_options, dict):
             yaml_options = {}
-            yaml_errors.append(("Invalid options format (not a dict)", line))
+            yaml_errors.append(
+                ParseWarnings(
+                    "Invalid options format (not a dict)",
+                    line,
+                    MystWarnings.DIRECTIVE_OPTION,
+                )
+            )
         return _DirectiveOptions(content, yaml_options, yaml_errors, has_options_block)
 
     options: dict[str, str] = {}
@@ -214,7 +234,13 @@ def _parse_directive_options(
             return _DirectiveOptions(
                 content,
                 options,
-                [(f"Invalid options format: {err.problem}", line)],
+                [
+                    ParseWarnings(
+                        f"Invalid options format: {err.problem}",
+                        line,
+                        MystWarnings.DIRECTIVE_OPTION,
+                    )
+                ],
                 has_options_block,
             )
 
@@ -231,7 +257,7 @@ def _parse_directive_options(
     options_spec: dict[str, Callable] = directive_class.option_spec
     unknown_options: list[str] = []
     new_options: dict[str, Any] = {}
-    validation_errors: list[tuple[str, int | None]] = []
+    validation_errors: list[ParseWarnings] = []
     value: str | None
     for name, value in options.items():
         try:
@@ -250,17 +276,22 @@ def _parse_directive_options(
             converted_value = convertor(value)
         except (ValueError, TypeError) as error:
             validation_errors.append(
-                (f"Invalid option value for {name!r}: {value}: {error}", line)
+                ParseWarnings(
+                    f"Invalid option value for {name!r}: {value}: {error}",
+                    line,
+                    MystWarnings.DIRECTIVE_OPTION,
+                )
             )
         else:
             new_options[name] = converted_value
 
     if unknown_options:
         validation_errors.append(
-            (
+            ParseWarnings(
                 f"Unknown option keys: {sorted(unknown_options)} "
                 f"(allowed: {sorted(options_spec)})",
                 line,
+                MystWarnings.DIRECTIVE_OPTION,
             )
         )
 

myst_parser/warnings_.py

@@ -31,6 +31,10 @@ class MystWarnings(Enum):
 
     DIRECTIVE_PARSING = "directive_parse"
     """Issue parsing directive."""
+    DIRECTIVE_OPTION = "directive_option"
+    """Issue parsing directive options."""
+    DIRECTIVE_BODY = "directive_body"
+    """Issue parsing directive body."""
     UNKNOWN_DIRECTIVE = "directive_unknown"
     """Unknown directive."""
     UNKNOWN_ROLE = "role_unknown"

tests/test_renderers/fixtures/directive_options.md

@@ -133,7 +133,7 @@ foo
 <document source="<src>/index.md">
     <system_message level="2" line="1" source="<src>/index.md" type="WARNING">
         <paragraph>
-            'restructuredtext-test-directive': Invalid options format: expected ':' after key [myst.directive_parse]
+            'restructuredtext-test-directive': Invalid options format: expected ':' after key [myst.directive_option]
     <system_message level="1" line="1" source="<src>/index.md" type="INFO">
         <paragraph>
             Directive processed. Type="restructuredtext-test-directive", arguments=[], options={}, content:

tests/test_renderers/fixtures/directive_parsing.txt

@@ -112,9 +112,9 @@ options:
   class:
   - tip
 warnings:
-- - Cannot split content across first line and body, when options block is present
-    (move first line to body)
-  - null
+- 'ParseWarnings(msg=''Splitting content across first line and body, when an options
+  block is present, is not recommended'', lineno=None, type=<MystWarnings.DIRECTIVE_PARSING:
+  ''directive_parse''>)'
 .
 
 admonition: no options, no new line
@@ -180,8 +180,8 @@ body: []
 content_offset: 3
 options: {}
 warnings:
-- - 'Unknown option keys: [''a''] (allowed: [''class'', ''name''])'
-  - 1
+- 'ParseWarnings(msg="Unknown option keys: [''a''] (allowed: [''class'', ''name''])",
+  lineno=1, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: yaml not a dict
@@ -197,8 +197,8 @@ body: []
 content_offset: 3
 options: {}
 warnings:
-- - 'Invalid options format: expected '':'' after key'
-  - 1
+- 'ParseWarnings(msg="Invalid options format: expected '':'' after key", lineno=1,
+  type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: unknown option name
@@ -212,8 +212,8 @@ body: []
 content_offset: 1
 options: {}
 warnings:
-- - 'Unknown option keys: [''unknown''] (allowed: [''class'', ''name''])'
-  - 0
+- 'ParseWarnings(msg="Unknown option keys: [''unknown''] (allowed: [''class'', ''name''])",
+  lineno=0, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: invalid option value
@@ -227,8 +227,8 @@ body: []
 content_offset: 1
 options: {}
 warnings:
-- - 'Invalid option value for ''class'': 1: cannot make "1" into a class name'
-  - 0
+- 'ParseWarnings(msg=''Invalid option value for \''class\'': 1: cannot make "1" into
+  a class name'', lineno=0, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 error: missing argument

tests/test_renderers/fixtures/myst-config.txt

@@ -497,13 +497,13 @@ content
             Unknown directive type: 'unknown' [myst.directive_unknown]
     <system_message level="2" line="6" source="<string>" type="WARNING">
         <paragraph>
-            'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_parse]
+            'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option]
     <admonition classes="class1" ids="myname" names="myname">
         <title>
             title
         <paragraph>
             content
 
 <string>:1: (WARNING/2) Unknown directive type: 'unknown' [myst.directive_unknown]
-<string>:6: (WARNING/2) 'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_parse]
+<string>:6: (WARNING/2) 'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option]
 .

tests/test_renderers/fixtures/reporter_warnings.md

@@ -143,13 +143,13 @@ lines
 <string>:12: (ERROR/3) Unknown interpreted text role "unknown".
 .
 
-bad-option-value
+directive bad-option-value
 .
 ```{note}
 :class: [1]
 ```
 .
-<string>:1: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_parse]
+<string>:1: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_option]
 <string>:1: (ERROR/3) Content block expected for the "note" directive; none found.
 .
 

tests/test_renderers/test_parse_directives.py

@@ -60,7 +60,7 @@ def test_parsing(file_params):
                 "options": result.options,
                 "body": result.body,
                 "content_offset": result.body_offset,
-                "warnings": result.warnings,
+                "warnings": [repr(w) for w in result.warnings],
             },
             sort_keys=True,
         )
@@ -112,4 +112,4 @@ def test_additional_options():
         Note, "", "content", additional_options={"foo": "bar"}
     )
     assert len(result.warnings) == 1
-    assert "Unknown option" in result.warnings[0][0]
+    assert "Unknown option" in result.warnings[0].msg