[Qemu-devel] [PATCH for-2.9 37/47] qapi: Fix detection of bogus member documentation

Markus Armbruster posted 47 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for-2.9 37/47] qapi: Fix detection of bogus member documentation
Posted by Markus Armbruster 8 years, 7 months ago
check_definition_doc() checks for member documentation without a
matching member.  It laboriously second-guesses what members
QAPISchema._def_exprs() will create.  That's a stupid game.

Move the check into QAPISchema.check(), where the members are known.
Delegate the actual checking to new QAPIDoc.check().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                             | 38 ++++++++++-------------------
 tests/qapi-schema/doc-bad-union-member.err  |  1 +
 tests/qapi-schema/doc-bad-union-member.exit |  2 +-
 tests/qapi-schema/doc-bad-union-member.out  | 11 ---------
 4 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e53701a..0b0ba59 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -249,6 +249,15 @@ class QAPIDoc(object):
             self.args[member.name] = QAPIDoc.ArgSection(member.name)
         self.args[member.name].connect(member)
 
+    def check(self):
+        bogus = [name for name, section in self.args.iteritems()
+                 if not section.member]
+        if bogus:
+            raise QAPISemError(
+                self.info,
+                "The following documented members are not in "
+                "the declaration: %s" % ", ".join(bogus))
+
 
 class QAPISchemaParser(object):
 
@@ -990,34 +999,9 @@ def check_exprs(exprs):
 
 
 def check_definition_doc(doc, expr, info):
-    for i in ('enum', 'union', 'alternate', 'struct', 'command', 'event'):
-        if i in expr:
-            meta = i
-            break
-
     if doc.has_section('Returns') and 'command' not in expr:
         raise QAPISemError(info, "'Returns:' is only valid for commands")
 
-    if meta == 'union':
-        args = expr.get('base', [])
-    else:
-        args = expr.get('data', [])
-    if isinstance(args, str):
-        return
-    if isinstance(args, dict):
-        args = args.keys()
-    assert isinstance(args, list)
-
-    if (meta == 'alternate'
-            or (meta == 'union' and not expr.get('discriminator'))):
-        args.append('type')
-
-    doc_args = set(doc.args.keys())
-    args = set([name.strip('*') for name in args])
-    if not doc_args.issubset(args):
-        raise QAPISemError(info, "The following documented members are not in "
-                           "the declaration: %s" % ', '.join(doc_args - args))
-
 
 def check_docs(docs):
     for doc in docs:
@@ -1263,6 +1247,8 @@ class QAPISchemaObjectType(QAPISchemaType):
             self.variants.check(schema, seen)
             assert self.variants.tag_member in self.members
             self.variants.check_clash(schema, self.info, seen)
+        if self.doc:
+            self.doc.check()
 
     # Check that the members of this type do not cause duplicate JSON members,
     # and update seen to track the members seen so far. Report any errors
@@ -1432,6 +1418,8 @@ class QAPISchemaAlternateType(QAPISchemaType):
             v.check_clash(self.info, seen)
             if self.doc:
                 self.doc.connect_member(v)
+        if self.doc:
+            self.doc.check()
 
     def c_type(self):
         return c_name(self.name) + pointer_suffix
diff --git a/tests/qapi-schema/doc-bad-union-member.err b/tests/qapi-schema/doc-bad-union-member.err
index e69de29..4b016df 100644
--- a/tests/qapi-schema/doc-bad-union-member.err
+++ b/tests/qapi-schema/doc-bad-union-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-union-member.json:3: The following documented members are not in the declaration: a, b
diff --git a/tests/qapi-schema/doc-bad-union-member.exit b/tests/qapi-schema/doc-bad-union-member.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/doc-bad-union-member.exit
+++ b/tests/qapi-schema/doc-bad-union-member.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/doc-bad-union-member.out b/tests/qapi-schema/doc-bad-union-member.out
index 2576ecd..e69de29 100644
--- a/tests/qapi-schema/doc-bad-union-member.out
+++ b/tests/qapi-schema/doc-bad-union-member.out
@@ -1,11 +0,0 @@
-object Base
-    member type: T optional=False
-object Empty
-object Frob
-    base Base
-    tag type
-    case nothing: Empty
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
-    prefix QTYPE
-enum T ['nothing']
-object q_empty
-- 
2.7.4


Re: [Qemu-devel] [PATCH for-2.9 37/47] qapi: Fix detection of bogus member documentation
Posted by Eric Blake 8 years, 7 months ago
On 03/13/2017 01:18 AM, Markus Armbruster wrote:
> check_definition_doc() checks for member documentation without a
> matching member.  It laboriously second-guesses what members
> QAPISchema._def_exprs() will create.  That's a stupid game.
> 
> Move the check into QAPISchema.check(), where the members are known.
> Delegate the actual checking to new QAPIDoc.check().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                             | 38 ++++++++++-------------------
>  tests/qapi-schema/doc-bad-union-member.err  |  1 +
>  tests/qapi-schema/doc-bad-union-member.exit |  2 +-
>  tests/qapi-schema/doc-bad-union-member.out  | 11 ---------
>  4 files changed, 15 insertions(+), 37 deletions(-)

Nice diffstat.


> +++ b/tests/qapi-schema/doc-bad-union-member.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/doc-bad-union-member.json:3: The following documented members are not in the declaration: a, b

Nice that you're able to report all problems within the doc, rather than
stopping at the first.  (Wish we could do the same about the overall
.json file, but that's harder, and out of scope for this series)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org