[PATCH v2 02/10] qapi: prohibit 'details' sections between tagged sections

John Snow posted 10 patches 3 days, 13 hours ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Kashyap Chamarthy <kchamart@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, John Snow <jsnow@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Eric Blake <eblake@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Jason Wang <jasowang@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Jiri Pirko <jiri@resnulli.us>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Stefan Hajnoczi <stefanha@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Lukas Straub <lukasstraub2@web.de>, Kostiantyn Kostiuk <kkostiuk@redhat.com>
[PATCH v2 02/10] qapi: prohibit 'details' sections between tagged sections
Posted by John Snow 3 days, 13 hours ago
This patch prohibits plain documentation sections from appearing between
(most*) tagged/description sections. The two existing uses of this
pattern (One in tests, one in our actual docs) are patched out.

This is being done for two main reasons:

(1) By limiting the locations where a "details" section may occur, it is
easier to reason about where a details section must be placed when
merging two or more documentation blocks together. This eases the logic
in the forthcoming inliner significantly.

(2) It improves visual consistency in the rendered HTML output. Tagged
sections and descriptions are all presented in a tabular format, so
prohibiting free-form text from interleaving the table looks better.

(*"most": As of this patch, TODO and Since sections may still occur
before, after, or between detail sections. These sections are removed
from the flow of the document when rendered to HTML, so in effect even
though we may have multiple details sections, they will be contiguous in
rendered HTML output.)

Signed-off-by: John Snow <jsnow@redhat.com>

---

[Review note: the output of *.ir files changes slightly, but
intentionally, with this patch; the two distinct changes correlate with
the edits made to the qapi .json files modified explicitly by this
patch. --js]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/devel/qapi-code-gen.rst                 | 15 +++++++++++----
 qapi/qom.json                                |  4 ++--
 scripts/qapi/parser.py                       | 19 +++++++++++++++++++
 tests/qapi-schema/doc-good.json              |  4 ++--
 tests/qapi-schema/doc-good.out               |  4 ++--
 tests/qapi-schema/doc-good.txt               |  8 ++++----
 tests/qapi-schema/doc-misplaced-details.err  |  0
 tests/qapi-schema/doc-misplaced-details.json |  3 +++
 tests/qapi-schema/doc-misplaced-details.out  | 11 +++++++++++
 tests/qapi-schema/meson.build                |  1 +
 10 files changed, 55 insertions(+), 14 deletions(-)
 create mode 100644 tests/qapi-schema/doc-misplaced-details.err
 create mode 100644 tests/qapi-schema/doc-misplaced-details.json
 create mode 100644 tests/qapi-schema/doc-misplaced-details.out

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 3a632b4a648..06ab3547fdc 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -985,10 +985,17 @@ When documentation is required (see pragma_ 'doc-required'), every
 definition must have documentation.
 
 Definition documentation starts with a line naming the definition,
-followed by an optional overview, a description of each argument (for
-commands and events), member (for structs and unions), branch (for
-alternates), or value (for enums), a description of each feature (if
-any), and finally optional tagged sections.
+followed by an optional overview (the "intro"), a description of each
+argument (for commands and events), member (for structs and unions),
+branch (for alternates), or value (for enums), a description of each
+feature (if any), most optional tagged sections, plaintext detail
+paragraphs, and finally the optional "Since" tagged section.
+
+Paragraphs following the optional overview (the "intro") may not appear
+between any description or tagged section as described above. These
+sections are "detail" sections and must appear at the end of the
+documentation block, with the exception of "Since" or "TODO" sections
+which may appear after.
 
 Descriptions start with '\@name:'.  The description text must be
 indented like this::
diff --git a/qapi/qom.json b/qapi/qom.json
index c653248f85d..1b47abd44e9 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -243,12 +243,12 @@
 #
 # @typename: the type name of an object
 #
+# Returns: a list describing object properties
+#
 # .. note:: Objects can create properties at runtime, for example to
 #    describe links between different devices and/or objects.  These
 #    properties are not included in the output of this command.
 #
-# Returns: a list describing object properties
-#
 # Since: 2.12
 ##
 { 'command': 'qom-list-properties',
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index da0ac32ad89..8a21e9e8b56 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -545,6 +545,21 @@ def get_doc(self) -> 'QAPIDoc':
             self.accept(False)
             line = self.get_doc_line()
             have_tagged = False
+            no_more_tags = False
+
+            def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
+                if isinstance(this, str):
+                    this = QAPIDoc.Kind.from_string(this)
+                if this in (QAPIDoc.Kind.TODO, QAPIDoc.Kind.SINCE):
+                    return
+
+                if no_more_tags:
+                    raise QAPIParseError(
+                        self,
+                        f"'{this}' section cannot appear after plain "
+                        "paragraphs that follow other tagged sections\n"
+                        "Move this section up above the plain paragraph(s)."
+                    )
 
             while line is not None:
                 # Blank lines
@@ -558,6 +573,7 @@ def get_doc(self) -> 'QAPIDoc':
                     if doc.features:
                         raise QAPIParseError(
                             self, "duplicated 'Features:' line")
+                    _tag_check(QAPIDoc.Kind.FEATURE)
                     self.accept(False)
                     line = self.get_doc_line()
                     while line == '':
@@ -621,6 +637,7 @@ def get_doc(self) -> 'QAPIDoc':
                         )
                         raise QAPIParseError(self, emsg)
 
+                    _tag_check(match.group(1))
                     doc.new_tagged_section(
                         self.info,
                         QAPIDoc.Kind.from_string(match.group(1))
@@ -632,6 +649,8 @@ def get_doc(self) -> 'QAPIDoc':
                     have_tagged = True
                 else:
                     # plain paragraph
+                    if have_tagged:
+                        no_more_tags = True
 
                     # Paragraphs before tagged sections are "intro" paragraphs.
                     # Any appearing after are "detail" paragraphs.
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index fac13425b72..9103fed472e 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -165,12 +165,12 @@
 # @cmd-feat1: a feature
 # @cmd-feat2: another feature
 #
-# .. note:: @arg3 is undocumented
-#
 # Returns: @Object
 #
 # Errors: some
 #
+# .. note:: @arg3 is undocumented
+#
 # TODO: frobnicate
 #
 # .. admonition:: Notes
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 04e29e8d50f..6a0167ad580 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -175,12 +175,12 @@ description starts on the same line
 a feature
     feature=cmd-feat2
 another feature
-    section=Details
-.. note:: @arg3 is undocumented
     section=Returns
 @Object
     section=Errors
 some
+    section=Details
+.. note:: @arg3 is undocumented
     section=Todo
 frobnicate
     section=Details
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 74b73681d32..ded699dd596 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -120,16 +120,16 @@ Command cmd (Since: 2.10)
 
       * **cmd-feat2** -- another feature
 
-   Note:
-
-     "arg3" is undocumented
-
    Return:
       "Object" -- "Object"
 
    Errors:
       some
 
+   Note:
+
+     "arg3" is undocumented
+
    Notes:
 
    * Lorem ipsum dolor sit amet
diff --git a/tests/qapi-schema/doc-misplaced-details.err b/tests/qapi-schema/doc-misplaced-details.err
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tests/qapi-schema/doc-misplaced-details.json b/tests/qapi-schema/doc-misplaced-details.json
new file mode 100644
index 00000000000..de593ab0f69
--- /dev/null
+++ b/tests/qapi-schema/doc-misplaced-details.json
@@ -0,0 +1,3 @@
+# FIXME / TODO
+# This test should test for the error message when we misplace details
+# sections interleaved between descriptions/tagged sections.
diff --git a/tests/qapi-schema/doc-misplaced-details.out b/tests/qapi-schema/doc-misplaced-details.out
new file mode 100644
index 00000000000..3c602d2592c
--- /dev/null
+++ b/tests/qapi-schema/doc-misplaced-details.out
@@ -0,0 +1,11 @@
+module ./builtin
+object q_empty
+enum QType
+    member none
+    member qnull
+    member qnum
+    member qstring
+    member qdict
+    member qlist
+    member qbool
+module doc-misplaced-details.json
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index debff633ac1..c233d77ab78 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -83,6 +83,7 @@ schemas = [
   'doc-invalid-section.json',
   'doc-invalid-start.json',
   'doc-long-line.json',
+  'doc-misplaced-details.json',
   'doc-missing-colon.json',
   'doc-missing-expr.json',
   'doc-missing-space.json',
-- 
2.53.0