[PATCH 13/14] qapi/parser: add new _sections mapping

[John Snow <jsnow@redhat.com>]

Modify the _sections field to be a mapping between section kinds and a
list of sections instead of a simple, flat list.

This makes it trivial to insert new auto-generated sections at the
correct position in the collection instead of needing to iterate
carefully over the list every time.

As a result, a lot of helpers we had to help us manage these
insertions are now unnecessary and can be deleted with extreme
prejudice.

NOTE: The remaining fishy use of a member insertion by the qapidoc
generator still remains, though it uses a simpler code pathway
now. The addition of the actual inliner mechanism will be the final
piece to remove this hack.

NOTE 2: Because "TODO:" sections are not strictly ordered in source,
this new section collection mechanism just lumps them in at the
end. For our generated documentation, this is functionally identical
as we do not render those sections anyway, but there is a noticeable
impact on our unit tests which *do* render TODO sections.

NOTE 3: This patch is what makes it obvious as to why we need
_append(), len(), and iter() implementations.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapidoc.py         |  6 +--
 scripts/qapi/parser.py         | 74 ++++++++--------------------------
 tests/qapi-schema/doc-good.out |  4 +-
 3 files changed, 20 insertions(+), 64 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index fd666be131f..41acb560628 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -371,11 +371,7 @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None:
         # other users of the QAPISchema or QAPIDoc objects outside of
         # the document generator. Fishy, but not harmful.
         if ent.doc:
-            ent.doc.append_member_stub(
-                QAPIDoc.ArgSection(
-                    ent.doc.info, QAPIDoc.Kind.MEMBER, "q_dummy"
-                )
-            )
+            ent.doc.new_argument(ent.doc.info, "q_dummy")
 
         sections = list(ent.doc) if ent.doc else []
 
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 894b8979b9e..1d8f1d39ab1 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -758,8 +758,13 @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None):
         self.info = info
         # definition doc's symbol, None for free-form doc
         self.symbol: Optional[str] = symbol
-        # the sections in textual order
-        self._sections: List[QAPIDoc.Section] = []
+
+        # all sections arranged by kind. This is identical to source
+        # order, with the exception of TODO sections and any generated
+        # sections that are appended post-hoc.
+        self._sections: Dict[QAPIDoc.Kind, List[QAPIDoc.Section]] = {
+            kind: [] for kind in QAPIDoc.Kind
+        }
 
         # The most recently appended section, on which append_line operates
         self._active_section: Optional[QAPIDoc.Section] = None
@@ -767,6 +772,7 @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None):
         # dicts mapping parameter/feature names to their description
         self._args: Dict[str, QAPIDoc.ArgSection] = {}
         self._features: Dict[str, QAPIDoc.ArgSection] = {}
+
         # a command's "Returns" and "Errors" section
         self._returns: Optional[QAPIDoc.Section] = None
         self._errors: Optional[QAPIDoc.Section] = None
@@ -781,11 +787,8 @@ def _append(self, section: 'QAPIDoc.Section') -> None:
         """
         Primary internal interface for adding a new section.
 
-        It is intended to become the sole access point for adding new
-        sections to the QAPIDoc section list, but we are currently in
-        a transitional phase where other methods access the section
-        list directly to do insertions of generated sections at
-        targeted points.
+        All other methods/helpers/etc ultimately use this and only
+        this interface for appending a new section.
         """
         if section.kind == QAPIDoc.Kind.RETURNS:
             if self._returns:
@@ -802,14 +805,15 @@ def _append(self, section: 'QAPIDoc.Section') -> None:
                 raise QAPISemError(
                     section.info, "duplicated '%s' section" % section.kind)
             self.since = section
-        self._sections.append(section)
+        self._sections[section.kind].append(section)
         self._active_section = section
 
     def __iter__(self) -> Iterator['QAPIDoc.Section']:
-        return iter(self._sections)
+        for values in self._sections.values():
+            yield from values
 
     def __len__(self) -> int:
-        return len(self._sections)
+        return sum(len(x) for x in self._sections.values())
 
     def end(self) -> None:
         for section in self:
@@ -870,32 +874,6 @@ def append_line(self, line: str) -> None:
         assert self._active_section is not None
         self._active_section.append_line(line)
 
-    def _insert_near_kind(
-        self,
-        kind: 'QAPIDoc.Kind',
-        new_sect: 'QAPIDoc.Section',
-        after: bool = False,
-    ) -> bool:
-        """Insert or append a new doc section at a specific point."""
-        for idx, sect in enumerate(reversed(list(self))):
-            if sect.kind == kind:
-                pos = len(self) - idx - 1
-                if after:
-                    pos += 1
-                self._sections.insert(pos, new_sect)
-                return True
-        return False
-
-    def append_member_stub(self, stub: 'QAPIDoc.Section') -> None:
-        """
-        Append a stub section after any Member sections.
-        """
-        if self._insert_near_kind(QAPIDoc.Kind.MEMBER, stub, True):
-            return
-
-        # No MEMBER sections present. Insert after INTRO section.
-        self._insert_near_kind(QAPIDoc.Kind.INTRO, stub, after=True)
-
     def connect_member(self, member: 'QAPISchemaMember') -> None:
         if member.name not in self._args:
             assert member.info
@@ -908,7 +886,7 @@ def connect_member(self, member: 'QAPISchemaMember') -> None:
             stub_section = QAPIDoc.ArgSection(
                 self.info, QAPIDoc.Kind.MEMBER, member.name)
             self._args[member.name] = stub_section
-            self.append_member_stub(stub_section)
+            self._append(stub_section)
 
         self._args[member.name].connect(member)
 
@@ -920,30 +898,12 @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
         self._features[feature.name].connect(feature)
 
     def ensure_returns(self, info: QAPISourceInfo) -> None:
-        # This is more complicated than it ought to be.  The doc
-        # parser should already know where a generated RETURNS section
-        # should go.  It currently doesn't.
-        #
-        # TODO: Take advantage of the new strict section ordering and
-        # simplify this algorithm.
-
-        if self._returns:
+        if self._sections[QAPIDoc.Kind.RETURNS]:
             return
 
         # Stub "Returns" section for undocumented returns value
         stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS)
-
-        if any(self._insert_near_kind(kind, stub, after) for kind, after in (
-                # 1. If arguments, right after those.
-                (QAPIDoc.Kind.MEMBER, True),
-                # 2. Elif errors, right *before* those.
-                (QAPIDoc.Kind.ERRORS, False),
-                # 3. Elif features, right *before* those.
-                (QAPIDoc.Kind.FEATURE, False),
-                # 4. Else, right *after* the intro.
-                (QAPIDoc.Kind.INTRO, True),
-        )):
-            return
+        self._append(stub)
 
     def check_expr(self, expr: QAPIExpression) -> None:
         if 'command' in expr:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 7a485e46037..4a0fae6fce2 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -181,8 +181,6 @@ a feature
 another feature
     Details
 .. note:: @arg3 is undocumented
-    Todo
-frobnicate
     Details
 .. admonition:: Notes
 
@@ -207,6 +205,8 @@ Examples::
 
 Note::
     Ceci n'est pas une note
+    Todo
+frobnicate
     Since
 2.10
 doc symbol=cmd-boxed
-- 
2.55.0