This patch changes the qapidoc parser to generate stub Return value
documentation for any command that has a return value but does not have
a "Returns:" doc section.

The stubs include just the type name, which will be rendered with a
cross-reference link in the HTML output.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapidoc.py | 14 ++++++++------
 scripts/qapi/parser.py | 34 ++++++++++++++++++++++++++++++++++
 scripts/qapi/schema.py |  3 +++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 8011ac9efaf..77e28a65cfc 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -255,16 +255,18 @@ def visit_feature(self, section: QAPIDoc.ArgSection) -> None:
     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:
         # FIXME: the formatting for errors may be inconsistent and may
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 949d9e8bff7..fc5174e3357 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -815,6 +815,40 @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
                                % feature.name)
         self.features[feature.name].connect(feature)
 
+    def ensure_returns(self, info: QAPISourceInfo) -> None:
+
+        def _insert_after_kind(
+            kind: QAPIDoc.Kind,
+            new_sect: QAPIDoc.Section
+        ) -> bool:
+            for sect in filter(lambda sect: sect.kind == kind, reversed(
+                    self.all_sections)):
+                idx = self.all_sections.index(sect) + 1
+                self.all_sections.insert(idx, 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_after_kind(kind, stub) for kind in (
+                # 1. If arguments, right after those.
+                QAPIDoc.Kind.MEMBER,
+                # 2. Elif errors, right after those.
+                QAPIDoc.Kind.ERRORS,
+                # 3. Elif features, right after those.
+                QAPIDoc.Kind.FEATURE,
+        )):
+            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 cbe3b5aa91e..3abddea3525 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -1062,6 +1062,9 @@ def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
             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(
-- 
2.48.1

John Snow <jsnow@redhat.com> writes:

> This patch changes the qapidoc parser to generate stub Return value
> documentation for any command that has a return value but does not have
> a "Returns:" doc section.
>
> The stubs include just the type name, which will be rendered with a
> cross-reference link in the HTML output.
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Might want to briefly explain placement of the auto-generated return
value documentation.

The cover letter claims you implemented the placement I suggested:

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 (might require a few more TODO
   hacks to terminate the intro).

This patch adds auto-generated return value documentation where we have
none.

The next patch replaces handwritten by auto-generated return value
documentation where these are at least as good.  Moves the return value
docs in some cases.

Let's check placement.  Skip to the end for a summary of findings.

First the additions (this patch):

* x-debug-query-block-graph

  Title, intro, features, return

  1. doesn't apply, 2. doesn't apply, 3. applies: return should go right
  before features.  It's afterwards.  Unexpected.

* query-tpm

  Title, intro, return, example

  4. applies.  Good.

* query-dirty-rate

  Title, intro, arguments, return, examples

  1. applies.  Good.

* query-vcpu-dirty-limit

  Title, intro, return, example

  4. applies.  Good.

* query-vm-generation-id

  Title, intro, return

  4. applies.  Good.

* query-memory-size-summary

  Title, intro, example, return

  4. applies.  Why is return after example here, and before example
  elsewhere?

  Turns out elsewhere we have Since: between intro and .. qmp-example::
  terminating the intro.  Slighly unusual, Since: more often the last in
  the comment block).

  Except for query-memory-size-summary, which does have Since: at the
  very and, and uses the TODO hack instead to terminate the intro.

  So, the placement algorithm works as intended here.  We just need to
  terminate the intro here as well.

* query-memory-devices

  Title, intro, return, example

  4. applies.  Good.

* query-acpi-ospm-status

  Title, intro, return, example

  4. applies.  Good.

* query-stats-schemas

  Title, intro, arguments, return, note

  1. applies.  Good.

Check placement of moved returns (next patch):

* x-debug-block-dirty-bitmap-sha256

  Title, intro, arguments, features, errors, return

  1. applies: return should go right after arguments.  Its at the end.
  Unexpected.

* query-xen-replication-status

  Title, intro, example, return

  Return after example instead of before.  This is just like
  query-memory-size-summary: placement algorithm works as intended,
  intro needs a terminator.

* query-colo-status

  Title, intro, example, return

  Likewise.

* query-machines

  Title, intro, arguments, return, features, example

  1. applies.  Okay.

* query-balloon

  Title, intro, errors, return, example

  2. applies: return should go right before errors.  It's afterwards.
  Unexpected.

* query-hv-balloon-status-report

  Title, intro, errors, return, example

  Likewise.

* query-yank

  Title, intro, example, return

  Another one just like query-memory-size-summary: placement algorithm
  works as intended, intro needs a terminator.

* query-sev-capabilities

  Title, intro, errors, return, example

  Like query-ballon.

* x-query-virtio-queue-element

  Title, intro, arguments, return, features, example

  1. applies.  Good.


Summary of findings:

1. If we have arguments, return goes right after them.

   Works as intended, except for x-debug-block-dirty-bitmap-sha256.
   Odd.

2. Else if we have errors, return goes right before them.

   You seem to place it afterwards instead.

3. Else if we have features, return goes right before them.

   You seem to place it afterwards instead.

4. Else return goes right after the intro (might require a few more TODO
   hacks to terminate the intro).

   Yes, we need several of them.