Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

👌 Improve directive parsing warnings #893

Merged
merged 1 commit into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions myst_parser/mdit_to_docutils/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down
2 changes: 1 addition & 1 deletion myst_parser/mocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
69 changes: 50 additions & 19 deletions myst_parser/parsers/directives.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
"""
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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


Expand Down Expand Up @@ -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] = {}
Expand All @@ -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,
)

Expand All @@ -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:
Expand All @@ -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,
)
)

Expand Down
4 changes: 4 additions & 0 deletions myst_parser/warnings_.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_renderers/fixtures/directive_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 11 additions & 11 deletions tests/test_renderers/fixtures/directive_parsing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/test_renderers/fixtures/myst-config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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]
.
4 changes: 2 additions & 2 deletions tests/test_renderers/fixtures/reporter_warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
.

Expand Down
4 changes: 2 additions & 2 deletions tests/test_renderers/test_parse_directives.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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
Loading