[PULL 12/12] qapi: Add documentation format validation

Markus Armbruster posted 12 patches 1 week, 4 days ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, "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>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eric Blake <eblake@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, Gerd Hoffmann <kraxel@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Eduardo Habkost <eduardo@habkost.net>, 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>, Kostiantyn Kostiuk <kkostiuk@redhat.com>
[PULL 12/12] qapi: Add documentation format validation
Posted by Markus Armbruster 1 week, 4 days ago
From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Add explicit validation for QAPI documentation formatting rules:

1. Lines must not exceed 70 columns in width (including '# ' prefix)
2. Sentences must be separated by two spaces

Example sections and literal :: blocks (seldom case) are excluded, we
don't require them to be <= 70, that would be too restrictive. Anyway,
they share common 80-columns recommendations (not requirements).

Add two simple tests, illustrating the change.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20251031183129.246814-1-vsementsov@yandex-team.ru>

The detection of example and literal blocks isn't quite correct, but
it works well enough, and we can improve on top.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Comments, error messages, and test file names tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/parser.py                        | 50 ++++++++++++++++++-
 .../doc-bad-space-between-sentences.err       |  2 +
 .../doc-bad-space-between-sentences.json      |  6 +++
 .../doc-bad-space-between-sentences.out       |  0
 tests/qapi-schema/doc-long-line.err           |  1 +
 tests/qapi-schema/doc-long-line.json          |  6 +++
 tests/qapi-schema/doc-long-line.out           |  0
 tests/qapi-schema/meson.build                 |  2 +
 8 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 tests/qapi-schema/doc-bad-space-between-sentences.err
 create mode 100644 tests/qapi-schema/doc-bad-space-between-sentences.json
 create mode 100644 tests/qapi-schema/doc-bad-space-between-sentences.out
 create mode 100644 tests/qapi-schema/doc-long-line.err
 create mode 100644 tests/qapi-schema/doc-long-line.json
 create mode 100644 tests/qapi-schema/doc-long-line.out

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 9fbf80a541..1bb1af7051 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -108,6 +108,11 @@ def __init__(self,
         self.exprs: List[QAPIExpression] = []
         self.docs: List[QAPIDoc] = []
 
+        # State for tracking qmp-example blocks and simple
+        # :: literal blocks.
+        self._literal_mode = False
+        self._literal_mode_indent = 0
+
         # Showtime!
         self._parse()
 
@@ -423,12 +428,55 @@ def get_doc_line(self) -> Optional[str]:
             if self.val != '##':
                 raise QAPIParseError(
                     self, "junk after '##' at end of documentation comment")
+            self._literal_mode = False
             return None
         if self.val == '#':
             return ''
         if self.val[1] != ' ':
             raise QAPIParseError(self, "missing space after #")
-        return self.val[2:].rstrip()
+
+        line = self.val[2:].rstrip()
+
+        if re.match(r'(\.\. +qmp-example)? *::$', line):
+            self._literal_mode = True
+            self._literal_mode_indent = 0
+        elif self._literal_mode and line:
+            indent = re.match(r'^ *', line).end()
+            if self._literal_mode_indent == 0:
+                self._literal_mode_indent = indent
+            elif indent < self._literal_mode_indent:
+                # ReST directives stop at decreasing indentation
+                self._literal_mode = False
+
+        if not self._literal_mode:
+            self._validate_doc_line_format(line)
+
+        return line
+
+    def _validate_doc_line_format(self, line: str) -> None:
+        """
+        Validate documentation format rules for a single line:
+        1. Lines should not exceed 70 characters
+        2. Sentences should be separated by two spaces
+        """
+        full_line_length = len(line) + 2  # "# " = 2 characters
+        if full_line_length > 70:
+            # Skip URL lines - they can't be broken
+            if re.match(r' *(https?|ftp)://[^ ]*$', line):
+                pass
+            else:
+                raise QAPIParseError(
+                    self, "documentation line longer than 70 characters")
+
+        single_space_pattern = r'(\be\.g\.|^ *\d\.|([.!?])) [A-Z0-9(]'
+        for m in list(re.finditer(single_space_pattern, line)):
+            if not m.group(2):
+                continue
+            # HACK so the error message points to the offending spot
+            self.pos = self.line_pos + 2 + m.start(2) + 1
+            raise QAPIParseError(
+                self, "Use two spaces between sentences\n"
+                "If this not the end of a sentence, please report a bug.")
 
     @staticmethod
     def _match_at_name_colon(string: str) -> Optional[Match[str]]:
diff --git a/tests/qapi-schema/doc-bad-space-between-sentences.err b/tests/qapi-schema/doc-bad-space-between-sentences.err
new file mode 100644
index 0000000000..479982ce22
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-space-between-sentences.err
@@ -0,0 +1,2 @@
+doc-bad-space-between-sentences.json:4:47: Use two spaces between sentences
+If this not the end of a sentence, please report a bug.
diff --git a/tests/qapi-schema/doc-bad-space-between-sentences.json b/tests/qapi-schema/doc-bad-space-between-sentences.json
new file mode 100644
index 0000000000..87b6f6eb4e
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-space-between-sentences.json
@@ -0,0 +1,6 @@
+##
+# @foo:
+#
+# Sentences should be separated by two spaces. But here is only one.
+##
+{ 'command': 'foo' }
diff --git a/tests/qapi-schema/doc-bad-space-between-sentences.out b/tests/qapi-schema/doc-bad-space-between-sentences.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/doc-long-line.err b/tests/qapi-schema/doc-long-line.err
new file mode 100644
index 0000000000..7baa5297cf
--- /dev/null
+++ b/tests/qapi-schema/doc-long-line.err
@@ -0,0 +1 @@
+doc-long-line.json:4:1: documentation line longer than 70 characters
diff --git a/tests/qapi-schema/doc-long-line.json b/tests/qapi-schema/doc-long-line.json
new file mode 100644
index 0000000000..c10153b0d5
--- /dev/null
+++ b/tests/qapi-schema/doc-long-line.json
@@ -0,0 +1,6 @@
+##
+# @foo:
+#
+# This line has exactly 71 chars, including the leading hash and space.
+##
+{ 'command': 'foo' }
diff --git a/tests/qapi-schema/doc-long-line.out b/tests/qapi-schema/doc-long-line.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index c47025d16d..debff633ac 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -61,6 +61,7 @@ schemas = [
   'doc-bad-event-arg.json',
   'doc-bad-feature.json',
   'doc-bad-indent.json',
+  'doc-bad-space-between-sentences.json',
   'doc-bad-symbol.json',
   'doc-bad-union-member.json',
   'doc-before-include.json',
@@ -81,6 +82,7 @@ schemas = [
   'doc-invalid-return2.json',
   'doc-invalid-section.json',
   'doc-invalid-start.json',
+  'doc-long-line.json',
   'doc-missing-colon.json',
   'doc-missing-expr.json',
   'doc-missing-space.json',
-- 
2.49.0
Re: [PULL 12/12] qapi: Add documentation format validation
Posted by Richard Henderson 1 week, 3 days ago
On 11/4/25 14:21, Markus Armbruster wrote:
> @@ -423,12 +428,55 @@ def get_doc_line(self) -> Optional[str]:
>               if self.val != '##':
>                   raise QAPIParseError(
>                       self, "junk after '##' at end of documentation comment")
> +            self._literal_mode = False
>               return None
>           if self.val == '#':
>               return ''
>           if self.val[1] != ' ':
>               raise QAPIParseError(self, "missing space after #")
> -        return self.val[2:].rstrip()
> +
> +        line = self.val[2:].rstrip()
> +
> +        if re.match(r'(\.\. +qmp-example)? *::$', line):
> +            self._literal_mode = True
> +            self._literal_mode_indent = 0
> +        elif self._literal_mode and line:
> +            indent = re.match(r'^ *', line).end()

Another failure from my incomplete testing last night:

https://gitlab.com/qemu-project/qemu/-/jobs/11982687207#L127

../scripts/qapi/parser.py:444: error: Item "None" of "Optional[Match[str]]" has no 
attribute "end"  [union-attr]


r~
Re: [PULL 12/12] qapi: Add documentation format validation
Posted by Markus Armbruster 1 week, 3 days ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 11/4/25 14:21, Markus Armbruster wrote:
>> @@ -423,12 +428,55 @@ def get_doc_line(self) -> Optional[str]:
>>               if self.val != '##':
>>                   raise QAPIParseError(
>>                       self, "junk after '##' at end of documentation comment")
>> +            self._literal_mode = False
>>               return None
>>           if self.val == '#':
>>               return ''
>>           if self.val[1] != ' ':
>>               raise QAPIParseError(self, "missing space after #")
>> -        return self.val[2:].rstrip()
>> +
>> +        line = self.val[2:].rstrip()
>> +
>> +        if re.match(r'(\.\. +qmp-example)? *::$', line):
>> +            self._literal_mode = True
>> +            self._literal_mode_indent = 0
>> +        elif self._literal_mode and line:
>> +            indent = re.match(r'^ *', line).end()
>
> Another failure from my incomplete testing last night:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/11982687207#L127
>
> ../scripts/qapi/parser.py:444: error: Item "None" of "Optional[Match[str]]" has no attribute "end"  [union-attr]

Missed in review, sorry.  I'll post a fix a.s.a.p.  Thanks!