From faeacf858bd9529cab10a13ff9d2137c8f2ae17c Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Mar 2025 23:42:30 -0400 Subject: qapi/parser: adjust info location for doc body section Instead of using the info object for the doc block as a whole (which always points to the very first line of the block), update the info pointer for each call to ensure_untagged_section when the existing section is otherwise empty. This way, Sphinx error information will match precisely to where the text actually starts. For example, this patch will move the info pointer for the "Hello!" untagged section ... > ## <-- from here ... > # Hello! <-- ... to here. > ## This doesn't seem to improve error reporting now. It will with the forthcoming QAPI doc transmogrifier. If I stick bad rST into qapi/block-core.json like this: > ## > # @SnapshotInfo: > # > +# rST syntax error: *ahh! > +# > # @id: unique shapshot id > # > # @name: user chosen name The existing code's error message will point to the beginning of the doc comment, which is less than helpful. The transmogrifier's message will point to the erroneous line, but to accomplish this, it needs this patch. Signed-off-by: John Snow Message-ID: <20250311034303.75779-33-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/parser.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 64f0bb824a..97def9f0e4 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -686,7 +686,11 @@ class QAPIDoc: def ensure_untagged_section(self, info: QAPISourceInfo) -> None: if self.all_sections and not self.all_sections[-1].tag: # extend current section - self.all_sections[-1].text += '\n' + section = self.all_sections[-1] + if not section.text: + # Section is empty so far; update info to start *here*. + section.info = info + section.text += '\n' return # start new section section = self.Section(info) -- cgit 1.4.1 From 323c668934a650673548088c6718c633b57b6ce5 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Mar 2025 23:42:31 -0400 Subject: qapi: clean up encoding of section kinds We have several kinds of sections, and to tell them apart, we use Section attribute @tag and also the section object's Python type: type @tag untagged Section None @foo: ArgSection 'foo' Returns: Section 'Returns' Errors: Section 'Errors' Since: Section 'Since' TODO: Section 'TODO' Note: * @foo can be a member or a feature description, depending on context. * tag == 'Since' can be a Since: section or a member or feature description. If it's a Section, it's the former, and if it's an ArgSection, it's the latter. Clean this up as follows. Move the member or feature name to new ArgSection attribute @name, and replace @tag by enum @kind like this: type kind name untagged Section PLAIN @foo: ArgSection MEMBER 'foo' if member or argument ArgSection FEATURE 'foo' if feature Returns: Section RETURNS Errors: Section ERRORS Since: Section SINCE TODO: Section TODO The qapi-schema tests are updated to account for the new section names; "TODO" becomes "Todo" and `None` becomes "Plain" there. Signed-off-by: John Snow Message-ID: <20250311034303.75779-34-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- docs/sphinx/qapidoc.py | 7 +-- scripts/qapi/parser.py | 97 ++++++++++++++++++++++++++++++------------ tests/qapi-schema/doc-good.out | 10 ++--- tests/qapi-schema/test-qapi.py | 2 +- 4 files changed, 80 insertions(+), 36 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index 61997fd21a..d622398f1d 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -35,6 +35,7 @@ from docutils.parsers.rst import Directive, directives from docutils.statemachine import ViewList from qapi.error import QAPIError, QAPISemError from qapi.gen import QAPISchemaVisitor +from qapi.parser import QAPIDoc from qapi.schema import QAPISchema from sphinx import addnodes @@ -258,11 +259,11 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): """Return list of doctree nodes for additional sections""" nodelist = [] for section in doc.sections: - if section.tag and section.tag == 'TODO': + if section.kind == QAPIDoc.Kind.TODO: # Hide TODO: sections continue - if not section.tag: + if section.kind == QAPIDoc.Kind.PLAIN: # Sphinx cannot handle sectionless titles; # Instead, just append the results to the prior section. container = nodes.container() @@ -270,7 +271,7 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): nodelist += container.children continue - snode = self._make_section(section.tag) + snode = self._make_section(section.kind.name.title()) self._parse_text_into_node(dedent(section.text), snode) nodelist.append(snode) return nodelist diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 97def9f0e4..94d5322f8a 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -14,6 +14,7 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. +import enum import os import re from typing import ( @@ -574,7 +575,10 @@ class QAPISchemaParser: ) raise QAPIParseError(self, emsg) - doc.new_tagged_section(self.info, match.group(1)) + doc.new_tagged_section( + self.info, + QAPIDoc.Kind.from_string(match.group(1)) + ) text = line[match.end():] if text: doc.append_line(text) @@ -585,7 +589,7 @@ class QAPISchemaParser: self, "unexpected '=' markup in definition documentation") else: - # tag-less paragraph + # plain paragraph doc.ensure_untagged_section(self.info) doc.append_line(line) line = self.get_doc_paragraph(doc) @@ -634,14 +638,33 @@ class QAPIDoc: Free-form documentation blocks consist only of a body section. """ + class Kind(enum.Enum): + PLAIN = 0 + MEMBER = 1 + FEATURE = 2 + RETURNS = 3 + ERRORS = 4 + SINCE = 5 + TODO = 6 + + @staticmethod + def from_string(kind: str) -> 'QAPIDoc.Kind': + return QAPIDoc.Kind[kind.upper()] + + def __str__(self) -> str: + return self.name.title() + class Section: # pylint: disable=too-few-public-methods - def __init__(self, info: QAPISourceInfo, - tag: Optional[str] = None): + def __init__( + self, + info: QAPISourceInfo, + kind: 'QAPIDoc.Kind', + ): # section source info, i.e. where it begins self.info = info - # section tag, if any ('Returns', '@name', ...) - self.tag = tag + # section kind + self.kind = kind # section text without tag self.text = '' @@ -649,8 +672,14 @@ class QAPIDoc: self.text += line + '\n' class ArgSection(Section): - def __init__(self, info: QAPISourceInfo, tag: str): - super().__init__(info, tag) + def __init__( + self, + info: QAPISourceInfo, + kind: 'QAPIDoc.Kind', + name: str + ): + super().__init__(info, kind) + self.name = name self.member: Optional['QAPISchemaMember'] = None def connect(self, member: 'QAPISchemaMember') -> None: @@ -662,7 +691,9 @@ class QAPIDoc: # definition doc's symbol, None for free-form doc self.symbol: Optional[str] = symbol # the sections in textual order - self.all_sections: List[QAPIDoc.Section] = [QAPIDoc.Section(info)] + self.all_sections: List[QAPIDoc.Section] = [ + QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) + ] # the body section self.body: Optional[QAPIDoc.Section] = self.all_sections[0] # dicts mapping parameter/feature names to their description @@ -679,12 +710,14 @@ class QAPIDoc: def end(self) -> None: for section in self.all_sections: section.text = section.text.strip('\n') - if section.tag is not None and section.text == '': + if section.kind != QAPIDoc.Kind.PLAIN and section.text == '': raise QAPISemError( - section.info, "text required after '%s:'" % section.tag) + section.info, "text required after '%s:'" % section.kind) def ensure_untagged_section(self, info: QAPISourceInfo) -> None: - if self.all_sections and not self.all_sections[-1].tag: + kind = QAPIDoc.Kind.PLAIN + + if self.all_sections and self.all_sections[-1].kind == kind: # extend current section section = self.all_sections[-1] if not section.text: @@ -692,46 +725,56 @@ class QAPIDoc: section.info = info section.text += '\n' return + # start new section - section = self.Section(info) + section = self.Section(info, kind) self.sections.append(section) self.all_sections.append(section) - def new_tagged_section(self, info: QAPISourceInfo, tag: str) -> None: - section = self.Section(info, tag) - if tag == 'Returns': + def new_tagged_section( + self, + info: QAPISourceInfo, + kind: 'QAPIDoc.Kind', + ) -> None: + section = self.Section(info, kind) + if kind == QAPIDoc.Kind.RETURNS: if self.returns: raise QAPISemError( - info, "duplicated '%s' section" % tag) + info, "duplicated '%s' section" % kind) self.returns = section - elif tag == 'Errors': + elif kind == QAPIDoc.Kind.ERRORS: if self.errors: raise QAPISemError( - info, "duplicated '%s' section" % tag) + info, "duplicated '%s' section" % kind) self.errors = section - elif tag == 'Since': + elif kind == QAPIDoc.Kind.SINCE: if self.since: raise QAPISemError( - info, "duplicated '%s' section" % tag) + info, "duplicated '%s' section" % kind) self.since = section self.sections.append(section) self.all_sections.append(section) - def _new_description(self, info: QAPISourceInfo, name: str, - desc: Dict[str, ArgSection]) -> None: + def _new_description( + self, + info: QAPISourceInfo, + name: str, + kind: 'QAPIDoc.Kind', + desc: Dict[str, ArgSection] + ) -> None: if not name: raise QAPISemError(info, "invalid parameter name") if name in desc: raise QAPISemError(info, "'%s' parameter name duplicated" % name) - section = self.ArgSection(info, '@' + name) + section = self.ArgSection(info, kind, name) self.all_sections.append(section) desc[name] = section def new_argument(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, self.args) + self._new_description(info, name, QAPIDoc.Kind.MEMBER, self.args) def new_feature(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, self.features) + self._new_description(info, name, QAPIDoc.Kind.FEATURE, self.features) def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) @@ -744,7 +787,7 @@ class QAPIDoc: "%s '%s' lacks documentation" % (member.role, member.name)) self.args[member.name] = QAPIDoc.ArgSection( - self.info, '@' + member.name) + self.info, QAPIDoc.Kind.MEMBER, member.name) self.args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 0a9da3efde..5773f1dd6d 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -113,7 +113,7 @@ The _one_ {and only}, description on the same line Also _one_ {and only} feature=enum-member-feat a member feature - section=None + section=Plain @two is undocumented doc symbol=Base body= @@ -171,15 +171,15 @@ description starts on the same line a feature feature=cmd-feat2 another feature - section=None + section=Plain .. note:: @arg3 is undocumented section=Returns @Object section=Errors some - section=TODO + section=Todo frobnicate - section=None + section=Plain .. admonition:: Notes - Lorem ipsum dolor sit amet @@ -212,7 +212,7 @@ If you're bored enough to read this, go see a video of boxed cats a feature feature=cmd-feat2 another feature - section=None + section=Plain .. qmp-example:: -> "this example" diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 8fe951c880..4be930228c 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -122,7 +122,7 @@ def test_frontend(fname): for feat, section in doc.features.items(): print(' feature=%s\n%s' % (feat, section.text)) for section in doc.sections: - print(' section=%s\n%s' % (section.tag, section.text)) + print(' section=%s\n%s' % (section.kind, section.text)) def open_test_result(dir_name, file_name, update): -- cgit 1.4.1 From d7ca9a3a4c55cba910a3556544e90aadd92c4efe Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Mar 2025 23:42:32 -0400 Subject: qapi/schema: add __repr__ to QAPIDoc.Section Makes debugging far more pleasant when you can just print(section) and get something reasonable to display. Signed-off-by: John Snow Message-ID: <20250311034303.75779-35-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/parser.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 94d5322f8a..11c11bb09e 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -668,6 +668,9 @@ class QAPIDoc: # section text without tag self.text = '' + def __repr__(self) -> str: + return f"" + def append_line(self, line: str) -> None: self.text += line + '\n' -- cgit 1.4.1 From 4d7d30b405a16d85ede6184595e8001fe2c412c4 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Mar 2025 23:42:57 -0400 Subject: qapi/parser: add undocumented stub members to all_sections Parser and doc generator cooperate on generating stub documentation for undocumented members. The parser makes up an ArgSection with an empty description, and the doc generator makes up a description. Right now, the made-up ArgSections go into doc.args. However, the new doc generator uses .all_sections, not .args. So put them into .all_sections, too. Insert them right after existing 'member' sections. If there are none, insert directly after the leading section. Doesn't affect the old generator, because that one doesn't use .all_sections. Signed-off-by: John Snow Message-ID: <20250311034303.75779-60-jsnow@redhat.com> Reviewed-by: Markus Armbruster [Commit message rewritten] Signed-off-by: Markus Armbruster --- scripts/qapi/parser.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 11c11bb09e..949d9e8bff 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -789,8 +789,23 @@ class QAPIDoc: raise QAPISemError(member.info, "%s '%s' lacks documentation" % (member.role, member.name)) - self.args[member.name] = QAPIDoc.ArgSection( + # Insert stub documentation section for missing member docs. + # TODO: drop when undocumented members are outlawed + + section = QAPIDoc.ArgSection( self.info, QAPIDoc.Kind.MEMBER, member.name) + self.args[member.name] = section + + # Determine where to insert stub doc - it should go at the + # end of the members section(s), if any. Note that index 0 + # is assumed to be an untagged intro section, even if it is + # empty. + index = 1 + if len(self.all_sections) > 1: + while self.all_sections[index].kind == QAPIDoc.Kind.MEMBER: + index += 1 + self.all_sections.insert(index, section) + self.args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: -- cgit 1.4.1