qapi: Fix undocumented return values by generating something

Generated command documentation lacks information on return value in
several cases, e.g. query-tpm.

The obvious fix would be to require a Returns: section when a command
returns something.

However, note that many existing Returns: sections are pretty useless:
the description is basically the return type, which then gets rendered
like "Return: <Type> – <basically the return type>".  This suggests
that a description is often not really necessary, and requiring one
isn't useful.

Instead, generate the obvious minimal thing when Returns: is absent:
"Return: <Type>".

This auto-generated Return documentation is placed is as follows:

1. If we have arguments, return goes right after them.
2. Else if we have errors, return goes right before them.
3. Else if we have features, return goes right before them.
4. Else return goes right after the intro

To facilitate this algorithm, a "TODO:" hack line is used to separate
the intro from the remainder of the documentation block in cases where
there are no other sections to separate the intro from e.g. examples and
additional detail meant to appear below the key sections of interest.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-ID: <20250711051045.51110-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[_insert_near_kind() code replaced by something simpler, commit
message amended to explain why we're doing this]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index b794cde697166985cd715a082a81c5886c3d7c26..c2f09bac16c9dd593f6870159177955430d3bba6 100644 (file)
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -258,16 +258,18 @@ class Transmogrifier:
     def visit_returns(self, section: QAPIDoc.Section) -> None:
         assert isinstance(self.entity, QAPISchemaCommand)
         rtype = self.entity.ret_type
-        # q_empty can produce None, but we won't be documenting anything
-        # without an explicit return statement in the doc block, and we
-        # should not have any such explicit statements when there is no
-        # return value.
+        # return statements will not be present (and won't be
+        # autogenerated) for any command that doesn't return
+        # *something*, so rtype will always be defined here.
         assert rtype
 
         typ = self.format_type(rtype)
         assert typ
-        assert section.text
-        self.add_field("return", typ, section.text, section.info)
+
+        if section.text:
+            self.add_field("return", typ, section.text, section.info)
+        else:
+            self.add_lines(f":return-nodesc: {typ}", section.info)
 
     def visit_errors(self, section: QAPIDoc.Section) -> None:
         # If the section text does not start with a space, it means text

diff --git a/qapi/machine.json b/qapi/machine.json
index af00a20b1fc833cbf7d06004ac00cb7fcec26124..2d6a137f21e58a454f651e128d1d55764f0ec70b 100644 (file)
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1303,6 +1303,8 @@
 # Return the amount of initially allocated and present hotpluggable
 # (if enabled) memory in bytes.
 #
+# TODO: This line is a hack to separate the example from the body
+#
 # .. qmp-example::
 #
 #     -> { "execute": "query-memory-size-summary" }

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index d43a123cd74fe5e73cf5efb1285abdb3649e5849..2529edf81aa8e4fc2bde87ce675285f775ecd3ce 100644 (file)
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -804,6 +804,43 @@ class QAPIDoc:
                                % feature.name)
         self.features[feature.name].connect(feature)
 
+    def ensure_returns(self, info: QAPISourceInfo) -> None:
+
+        def _insert_near_kind(
+            kind: QAPIDoc.Kind,
+            new_sect: QAPIDoc.Section,
+            after: bool = False,
+        ) -> bool:
+            for idx, sect in enumerate(reversed(self.all_sections)):
+                if sect.kind == kind:
+                    pos = len(self.all_sections) - idx - 1
+                    if after:
+                        pos += 1
+                    self.all_sections.insert(pos, new_sect)
+                    return True
+            return False
+
+        if any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections):
+            return
+
+        # Stub "Returns" section for undocumented returns value
+        stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS)
+
+        if any(_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),
+        )):
+            return
+
+        # Otherwise, it should go right after the intro. The intro
+        # is always the first section and is always present (even
+        # when empty), so we can insert directly at index=1 blindly.
+        self.all_sections.insert(1, stub)
+
     def check_expr(self, expr: QAPIExpression) -> None:
         if 'command' in expr:
             if self.returns and 'returns' not in expr:

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cbe3b5aa91ec1a00e48c2ee5251c573a69f8592e..3abddea35257e8da401baf7cfb9920b1f6d372b2 100644 (file)
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -1062,6 +1062,9 @@ class QAPISchemaCommand(QAPISchemaDefinition):
             if self.arg_type and self.arg_type.is_implicit():
                 self.arg_type.connect_doc(doc)
 
+            if self.ret_type and self.info:
+                doc.ensure_returns(self.info)
+
     def visit(self, visitor: QAPISchemaVisitor) -> None:
         super().visit(visitor)
         visitor.visit_command(