[PATCH v2 08/10] qapi: enforce doc block section ordering

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 08/10] qapi: enforce doc block section ordering
Posted by John Snow 3 days, 13 hours ago
Ugly hack, gets the job done. Likely many simplifications can be made as
a result, but I didn't make any of them. There are some inconsistencies
with human-readable vs ENUM_NAMES in error messages in this patch, but
it appears to work anyway.

Consider this patch more of a rough idea and not anything approximating
the kind of code you'd want to see from an Enterprise Linux Senior
Software Engineer.

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

---

[Review note: No changes to *.ir files. --js]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/parser.py | 55 ++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index ade26d124df..6e91cd19e58 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -547,22 +547,28 @@ def get_doc(self) -> 'QAPIDoc':
             self.accept(False)
             line = self.get_doc_line()
             have_tagged = False
-            no_more_tags = False
+            last_ordered_section = QAPIDoc.Kind.INTRO
 
             def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
+                nonlocal last_ordered_section
                 if isinstance(this, str):
                     this = QAPIDoc.Kind.from_string(this)
+
                 if this in (QAPIDoc.Kind.TODO, QAPIDoc.Kind.SINCE):
                     return
 
-                if no_more_tags:
+                if this.value < last_ordered_section.value:
                     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)."
+                        f"'{this}' section cannot appear after "
+                        f"'{last_ordered_section}' section, please re-order "
+                        "the sections and adjust phrasing as necessary to "
+                        "ensure consistent documentation flow between the "
+                        "source code and the rendered HTML manual"
                     )
 
+                last_ordered_section = this
+
             while line is not None:
                 # Blank lines
                 while line == '':
@@ -593,7 +599,7 @@ def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
                             self, 'feature descriptions expected')
                     have_tagged = True
                 elif line == 'Details:':
-                    _tag_check("Details")
+                    _tag_check(QAPIDoc.Kind.DETAILS)
                     self.accept(False)
                     line = self.get_doc_line()
                     while line == '':
@@ -602,6 +608,7 @@ def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
                     have_tagged = True
                 elif match := self._match_at_name_colon(line):
                     # description
+                    _tag_check(QAPIDoc.Kind.MEMBER)
                     if have_tagged:
                         raise QAPIParseError(
                             self,
@@ -658,14 +665,16 @@ def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
                     line = self.get_doc_indented(doc)
                     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.
-                    intro = not have_tagged
-                    doc.ensure_untagged_section(self.info, intro)
+                    # Paragraphs appearing before any other sections are
+                    # "intro" paragraphs. Any appearing after are
+                    # "details" paragraphs.
+                    this_section = (
+                        QAPIDoc.Kind.DETAILS
+                        if have_tagged
+                        else QAPIDoc.Kind.INTRO
+                    )
+                    _tag_check(this_section)
+                    doc.ensure_untagged_section(self.info, this_section)
                     doc.append_line(line)
                     line = self.get_doc_paragraph(doc)
         else:
@@ -707,14 +716,17 @@ class QAPIDoc:
     """
 
     class Kind(enum.Enum):
+        # The order here is the order in which sections must appear in
+        # source code; with the exception of 'TODO' which may appear
+        # anywhere but is treated as a comment.
         INTRO = 0
         MEMBER = 1
-        FEATURE = 2
-        RETURNS = 3
-        ERRORS = 4
-        SINCE = 5
+        RETURNS = 2
+        ERRORS = 3
+        FEATURE = 4
+        DETAILS = 5
         TODO = 6
-        DETAILS = 7
+        SINCE = 7
 
         @staticmethod
         def from_string(kind: str) -> 'QAPIDoc.Kind':
@@ -793,9 +805,10 @@ def end(self) -> None:
     def ensure_untagged_section(
         self,
         info: QAPISourceInfo,
-        intro: bool = True,
+        kind: Optional['QAPIDoc.Kind'] = None,
     ) -> None:
-        kind = QAPIDoc.Kind.INTRO if intro else QAPIDoc.Kind.DETAILS
+        if kind is None:
+            kind = QAPIDoc.Kind.INTRO
 
         if self.all_sections and self.all_sections[-1].kind == kind:
             # extend current section
-- 
2.53.0