From ee1e6a1f6c830fd7530243a24c800063bc27a758 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:26 +0200
Subject: qapi: Split .connect_doc(), .check_doc() off .check()

Splitting documentation checking off the .check() methods makes them a
bit more focused, which is welcome, as some of them are pretty big.
It also prepares the ground for the following commits.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-9-armbru@redhat.com>
---
 scripts/qapi/schema.py | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index f7d68a35f4..9b62c8d74d 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -51,6 +51,12 @@ class QAPISchemaEntity(object):
                                            os.path.dirname(schema.fname))
         self._checked = True
 
+    def connect_doc(self):
+        pass
+
+    def check_doc(self):
+        pass
+
     @property
     def ifcond(self):
         assert self._checked
@@ -217,7 +223,10 @@ class QAPISchemaEnumType(QAPISchemaType):
         seen = {}
         for m in self.members:
             m.check_clash(self.info, seen)
-            if self.doc:
+
+    def connect_doc(self):
+        if self.doc:
+            for m in self.members:
                 self.doc.connect_member(m)
 
     def is_implicit(self):
@@ -345,8 +354,6 @@ class QAPISchemaObjectType(QAPISchemaType):
         for m in self.local_members:
             m.check(schema)
             m.check_clash(self.info, seen)
-            if self.doc:
-                self.doc.connect_member(m)
         members = seen.values()
 
         if self.variants:
@@ -358,9 +365,6 @@ class QAPISchemaObjectType(QAPISchemaType):
         for f in self.features:
             f.check_clash(self.info, seen)
 
-        if self.doc:
-            self.doc.check()
-
         self.members = members  # mark completed
 
     # Check that the members of this type do not cause duplicate JSON members,
@@ -372,6 +376,15 @@ class QAPISchemaObjectType(QAPISchemaType):
         for m in self.members:
             m.check_clash(info, seen)
 
+    def connect_doc(self):
+        if self.doc:
+            for m in self.local_members:
+                self.doc.connect_member(m)
+
+    def check_doc(self):
+        if self.doc:
+            self.doc.check()
+
     @property
     def ifcond(self):
         assert self._checked
@@ -639,8 +652,13 @@ class QAPISchemaAlternateType(QAPISchemaType):
                         "%s can't be distinguished from '%s'"
                         % (v.describe(self.info), types_seen[qt]))
                 types_seen[qt] = v.name
-            if self.doc:
+
+    def connect_doc(self):
+        if self.doc:
+            for v in self.variants.variants:
                 self.doc.connect_member(v)
+
+    def check_doc(self):
         if self.doc:
             self.doc.check()
 
@@ -1043,6 +1061,8 @@ class QAPISchema(object):
     def check(self):
         for ent in self._entity_list:
             ent.check(self)
+            ent.connect_doc()
+            ent.check_doc()
 
     def visit(self, visitor):
         visitor.visit_begin(self)
-- 
cgit 1.4.1


From 36a43905ffcac8767947626a6761df63fc020841 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:27 +0200
Subject: qapi: Fix enum doc comment checking

Enumeration type documentation comments are not checked, as
demonstrated by test doc-bad-enum-member.  This is because we neglect
to call self.doc.check() for enumeration types.  Messed up in
816a57cd6e "qapi: Fix detection of bogus member documentation".  Fix
it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-10-armbru@redhat.com>
---
 scripts/qapi/schema.py                     |  4 ++++
 tests/qapi-schema/doc-bad-enum-member.err  |  1 +
 tests/qapi-schema/doc-bad-enum-member.json |  1 -
 tests/qapi-schema/doc-bad-enum-member.out  | 21 ---------------------
 4 files changed, 5 insertions(+), 22 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 9b62c8d74d..0381e3cb40 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -229,6 +229,10 @@ class QAPISchemaEnumType(QAPISchemaType):
             for m in self.members:
                 self.doc.connect_member(m)
 
+    def check_doc(self):
+        if self.doc:
+            self.doc.check()
+
     def is_implicit(self):
         # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
         return self.name.endswith('Kind') or self.name == 'QType'
diff --git a/tests/qapi-schema/doc-bad-enum-member.err b/tests/qapi-schema/doc-bad-enum-member.err
index e69de29bb2..dfa1e786d7 100644
--- a/tests/qapi-schema/doc-bad-enum-member.err
+++ b/tests/qapi-schema/doc-bad-enum-member.err
@@ -0,0 +1 @@
+doc-bad-enum-member.json:3: the following documented members are not in the declaration: a
diff --git a/tests/qapi-schema/doc-bad-enum-member.json b/tests/qapi-schema/doc-bad-enum-member.json
index 9f32fe64b4..9cab35c6e8 100644
--- a/tests/qapi-schema/doc-bad-enum-member.json
+++ b/tests/qapi-schema/doc-bad-enum-member.json
@@ -1,5 +1,4 @@
 # Members listed in the doc comment must exist in the actual schema
-# BUG: nonexistent @a is not rejected
 
 ##
 # @Foo:
diff --git a/tests/qapi-schema/doc-bad-enum-member.out b/tests/qapi-schema/doc-bad-enum-member.out
index 6ca31c1e9b..e69de29bb2 100644
--- a/tests/qapi-schema/doc-bad-enum-member.out
+++ b/tests/qapi-schema/doc-bad-enum-member.out
@@ -1,21 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module doc-bad-enum-member.json
-enum Foo
-    member b
-doc symbol=Foo
-    body=
-
-    arg=a
-a
-    arg=b
-b
-- 
cgit 1.4.1


From 7faefad184201b10c2db4270b2bb93e2a5e9552b Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:28 +0200
Subject: qapi: Clean up doc comment checking for implicit union base

An object type's doc comment describes the type's members, less the
ones defined in a named base type.  Cases:

* Struct: the members are defined in 'data' and inherited from 'base'.
  Since the base type cannot be implicit, the doc comment describes
  just 'data'.

* Simple union: the only member is the implicit tag member @type, and
  the doc comment describes it.

* Flat union with implicit base type: the members are defined in
  'base', and the doc comment describes it.

* Flat union with named base type: the members are inherited from
  'base'.  The doc comment describes no members.

Before we can check a doc comment with .check_doc(), we need
.connect_doc() connect each of its "argument sections" to the member
it documents.

For structs and simple unions, this is straightforward: the members in
question are in .local_members, and .connect_doc() connects them.

For flat unions with a named base type, it's trivial: .local_members
is empty, and .connect_doc() does nothing.

For flat unions with an implicit base type, it's tricky.  We have
QAPISchema._make_implicit_object_type() forward the union's doc
comment to the implicit base type, so that the base type's
.connect_doc() connects the members.  The union's .connect_doc() does
nothing, as .local_members is empty.

Dirt effect: we check the doc comment twice, once for the union type,
and once for the implicit base type.

This is needlessly brittle and hard to understand.  Clean up as
follows.  Make the union's .connect_doc() connect an implicit base's
members itself.  Do not forward the union's doc comment to its
implicit base type.

Requires extending .connect_doc() so it can work with a doc comment
other than self.doc.  Add an optional argument for that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-11-armbru@redhat.com>
---
 scripts/qapi/schema.py | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 0381e3cb40..c16dce1fe0 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -51,7 +51,7 @@ class QAPISchemaEntity(object):
                                            os.path.dirname(schema.fname))
         self._checked = True
 
-    def connect_doc(self):
+    def connect_doc(self, doc=None):
         pass
 
     def check_doc(self):
@@ -224,10 +224,11 @@ class QAPISchemaEnumType(QAPISchemaType):
         for m in self.members:
             m.check_clash(self.info, seen)
 
-    def connect_doc(self):
-        if self.doc:
+    def connect_doc(self, doc=None):
+        doc = doc or self.doc
+        if doc:
             for m in self.members:
-                self.doc.connect_member(m)
+                doc.connect_member(m)
 
     def check_doc(self):
         if self.doc:
@@ -380,10 +381,13 @@ class QAPISchemaObjectType(QAPISchemaType):
         for m in self.members:
             m.check_clash(info, seen)
 
-    def connect_doc(self):
-        if self.doc:
+    def connect_doc(self, doc=None):
+        doc = doc or self.doc
+        if doc:
+            if self.base and self.base.is_implicit():
+                self.base.connect_doc(doc)
             for m in self.local_members:
-                self.doc.connect_member(m)
+                doc.connect_member(m)
 
     def check_doc(self):
         if self.doc:
@@ -657,10 +661,11 @@ class QAPISchemaAlternateType(QAPISchemaType):
                         % (v.describe(self.info), types_seen[qt]))
                 types_seen[qt] = v.name
 
-    def connect_doc(self):
-        if self.doc:
+    def connect_doc(self, doc=None):
+        doc = doc or self.doc
+        if doc:
             for v in self.variants.variants:
-                self.doc.connect_member(v)
+                doc.connect_member(v)
 
     def check_doc(self):
         if self.doc:
@@ -974,7 +979,7 @@ class QAPISchema(object):
         tag_member = None
         if isinstance(base, dict):
             base = self._make_implicit_object_type(
-                name, info, doc, ifcond,
+                name, info, None, ifcond,
                 'base', self._make_members(base, info))
         if tag_name:
             variants = [self._make_variant(key, value['type'],
-- 
cgit 1.4.1


From bf83f04e13063bb723fb8b9df789a3613c6d0ceb Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:29 +0200
Subject: qapi: Fix doc comment checking for commands and events

When a command's 'data' is an object, its doc comment describes the
arguments defined there.  When 'data' names a type, the doc comment
does not describe arguments.  Instead, the doc generator inserts a
pointer to the named type.

An event's doc comment works the same.

We don't actually check doc comments for commands and events.
Instead, QAPISchema._def_command() forwards the doc comment to the
implicit argument type, where it gets checked.  Works because the
check only cares for the implicit argument type's members.

Not only is this needlessly hard to understand, it actually falls
apart in two cases:

* When 'data' is empty, there is nothing to forward to, and the doc
  comment remains unchecked.  Demonstrated by test doc-bad-event-arg.

* When 'data' names a type, we can't forward, as the type has its own
  doc comment.  The command or event's doc comment remains unchecked.
  Demonstrated by test doc-bad-boxed-command-arg.

The forwarding goes back to commit 069fb5b250 "qapi: Prepare for
requiring more complete documentation", put to use in commit
816a57cd6e "qapi: Fix detection of bogus member documentation".  That
fix was incomplete.

To fix this, make QAPISchemaCommand and QAPISchemaEvent check doc
comments, and drop the forwarding of doc comments to implicit argument
types.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-12-armbru@redhat.com>
---
 qapi/net.json                                    |  2 --
 scripts/qapi/doc.py                              |  1 +
 scripts/qapi/schema.py                           | 24 ++++++++++++++++++++--
 tests/qapi-schema/doc-bad-boxed-command-arg.err  |  1 +
 tests/qapi-schema/doc-bad-boxed-command-arg.json |  1 -
 tests/qapi-schema/doc-bad-boxed-command-arg.out  | 26 ------------------------
 tests/qapi-schema/doc-bad-event-arg.err          |  1 +
 tests/qapi-schema/doc-bad-event-arg.json         |  1 -
 tests/qapi-schema/doc-bad-event-arg.out          | 19 -----------------
 9 files changed, 25 insertions(+), 51 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/qapi/net.json b/qapi/net.json
index 728990f4fb..4c96137811 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -723,8 +723,6 @@
 # Trigger generation of broadcast RARP frames to update network switches.
 # This can be useful when network bonds fail-over the active slave.
 #
-# @params: AnnounceParameters giving timing and repetition count of announce
-#
 # Example:
 #
 # -> { "execute": "announce-self",
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index c8c4bda153..6f1c17f71f 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -185,6 +185,7 @@ def texi_members(doc, what, base=None, variants=None,
 
 def texi_arguments(doc, boxed_arg_type):
     if boxed_arg_type:
+        assert not doc.args
         return ('\n@b{Arguments:} the members of @code{%s}\n'
                 % boxed_arg_type.name)
     return texi_members(doc, 'Arguments')
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c16dce1fe0..06e37c9c49 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -739,6 +739,16 @@ class QAPISchemaCommand(QAPISchemaEntity):
         for f in self.features:
             f.check_clash(self.info, seen)
 
+    def connect_doc(self, doc=None):
+        doc = doc or self.doc
+        if doc:
+            if self.arg_type and self.arg_type.is_implicit():
+                self.arg_type.connect_doc(doc)
+
+    def check_doc(self):
+        if self.doc:
+            self.doc.check()
+
     def visit(self, visitor):
         QAPISchemaEntity.visit(self, visitor)
         visitor.visit_command(self.name, self.info, self.ifcond,
@@ -775,6 +785,16 @@ class QAPISchemaEvent(QAPISchemaEntity):
                     "event's 'data' can take %s only with 'boxed': true"
                     % self.arg_type.describe())
 
+    def connect_doc(self, doc=None):
+        doc = doc or self.doc
+        if doc:
+            if self.arg_type and self.arg_type.is_implicit():
+                self.arg_type.connect_doc(doc)
+
+    def check_doc(self):
+        if self.doc:
+            self.doc.check()
+
     def visit(self, visitor):
         QAPISchemaEntity.visit(self, visitor)
         visitor.visit_event(self.name, self.info, self.ifcond,
@@ -1026,7 +1046,7 @@ class QAPISchema(object):
         features = expr.get('features', [])
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, doc, ifcond, 'arg', self._make_members(data, info))
+                name, info, None, ifcond, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
@@ -1042,7 +1062,7 @@ class QAPISchema(object):
         ifcond = expr.get('if')
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, doc, ifcond, 'arg', self._make_members(data, info))
+                name, info, None, ifcond, 'arg', self._make_members(data, info))
         self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, data, boxed))
 
     def _def_exprs(self, exprs):
diff --git a/tests/qapi-schema/doc-bad-boxed-command-arg.err b/tests/qapi-schema/doc-bad-boxed-command-arg.err
index e69de29bb2..e1101b1667 100644
--- a/tests/qapi-schema/doc-bad-boxed-command-arg.err
+++ b/tests/qapi-schema/doc-bad-boxed-command-arg.err
@@ -0,0 +1 @@
+doc-bad-boxed-command-arg.json:9: the following documented members are not in the declaration: a
diff --git a/tests/qapi-schema/doc-bad-boxed-command-arg.json b/tests/qapi-schema/doc-bad-boxed-command-arg.json
index 2c265d2ca3..bd143241ec 100644
--- a/tests/qapi-schema/doc-bad-boxed-command-arg.json
+++ b/tests/qapi-schema/doc-bad-boxed-command-arg.json
@@ -1,5 +1,4 @@
 # Boxed arguments are not to be documented with the command
-# BUG: not rejected
 
 ##
 # @Args:
diff --git a/tests/qapi-schema/doc-bad-boxed-command-arg.out b/tests/qapi-schema/doc-bad-boxed-command-arg.out
index 4ccd788253..e69de29bb2 100644
--- a/tests/qapi-schema/doc-bad-boxed-command-arg.out
+++ b/tests/qapi-schema/doc-bad-boxed-command-arg.out
@@ -1,26 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module doc-bad-boxed-command-arg.json
-object Args
-    member a: int optional=False
-command cmd-boxed Args -> None
-    gen=True success_response=True boxed=True oob=False preconfig=False
-doc symbol=Args
-    body=
-
-    arg=a
-an argument
-doc symbol=cmd-boxed
-    body=
-
-    arg=a
-bogus
diff --git a/tests/qapi-schema/doc-bad-event-arg.err b/tests/qapi-schema/doc-bad-event-arg.err
index e69de29bb2..114ff4a3c7 100644
--- a/tests/qapi-schema/doc-bad-event-arg.err
+++ b/tests/qapi-schema/doc-bad-event-arg.err
@@ -0,0 +1 @@
+doc-bad-event-arg.json:3: the following documented members are not in the declaration: a
diff --git a/tests/qapi-schema/doc-bad-event-arg.json b/tests/qapi-schema/doc-bad-event-arg.json
index 80d4e1240b..23c83cc81f 100644
--- a/tests/qapi-schema/doc-bad-event-arg.json
+++ b/tests/qapi-schema/doc-bad-event-arg.json
@@ -1,5 +1,4 @@
 # Arguments listed in the doc comment must exist in the actual schema
-# BUG: nonexistent @a is not rejected
 
 ##
 # @FOO:
diff --git a/tests/qapi-schema/doc-bad-event-arg.out b/tests/qapi-schema/doc-bad-event-arg.out
index ad0367cd45..e69de29bb2 100644
--- a/tests/qapi-schema/doc-bad-event-arg.out
+++ b/tests/qapi-schema/doc-bad-event-arg.out
@@ -1,19 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module doc-bad-event-arg.json
-event FOO None
-    boxed=False
-doc symbol=FOO
-    body=
-
-    arg=a
-a
-- 
cgit 1.4.1


From a710e1c8c3f1c0dda45608927d6f9cedd898c0d2 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:30 +0200
Subject: qapi: Simplify ._make_implicit_object_type()

All callers now pass doc=None.  Drop the argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-13-armbru@redhat.com>
---
 scripts/qapi/schema.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 06e37c9c49..27da4e0f7d 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -920,8 +920,7 @@ class QAPISchema(object):
             self._def_entity(QAPISchemaArrayType(name, info, element_type))
         return name
 
-    def _make_implicit_object_type(self, name, info, doc, ifcond,
-                                   role, members):
+    def _make_implicit_object_type(self, name, info, ifcond, role, members):
         if not members:
             return None
         # See also QAPISchemaObjectTypeMember.describe()
@@ -939,7 +938,7 @@ class QAPISchema(object):
             # TODO kill simple unions or implement the disjunction
             assert (ifcond or []) == typ._ifcond # pylint: disable=protected-access
         else:
-            self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond,
+            self._def_entity(QAPISchemaObjectType(name, info, None, ifcond,
                                                   None, members, None, []))
         return name
 
@@ -986,7 +985,7 @@ class QAPISchema(object):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
         typ = self._make_implicit_object_type(
-            typ, info, None, self.lookup_type(typ),
+            typ, info, self.lookup_type(typ),
             'wrapper', [self._make_member('data', typ, None, info)])
         return QAPISchemaObjectTypeVariant(case, info, typ, ifcond)
 
@@ -999,7 +998,7 @@ class QAPISchema(object):
         tag_member = None
         if isinstance(base, dict):
             base = self._make_implicit_object_type(
-                name, info, None, ifcond,
+                name, info, ifcond,
                 'base', self._make_members(base, info))
         if tag_name:
             variants = [self._make_variant(key, value['type'],
@@ -1046,7 +1045,7 @@ class QAPISchema(object):
         features = expr.get('features', [])
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, None, ifcond, 'arg', self._make_members(data, info))
+                name, info, ifcond, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
@@ -1062,7 +1061,7 @@ class QAPISchema(object):
         ifcond = expr.get('if')
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, None, ifcond, 'arg', self._make_members(data, info))
+                name, info, ifcond, 'arg', self._make_members(data, info))
         self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, data, boxed))
 
     def _def_exprs(self, exprs):
-- 
cgit 1.4.1


From 1192a4862b02b01c77415c3c04b5931e2b0b42bf Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:31 +0200
Subject: qapi: Eliminate .check_doc() overrides

All sub-classes of QAPISchemaEntity now override .check_doc() the same
way, except for QAPISchemaType and and QAPISchemaArrayType.

Put the overrides' code in QAPISchemaEntity.check_doc(), and drop the
overrides.  QAPISchemaType doesn't care because it's abstract.
QAPISchemaArrayType doesn't care because its .doc is always None.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-14-armbru@redhat.com>
---
 scripts/qapi/schema.py | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 27da4e0f7d..ee510f129b 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -55,7 +55,8 @@ class QAPISchemaEntity(object):
         pass
 
     def check_doc(self):
-        pass
+        if self.doc:
+            self.doc.check()
 
     @property
     def ifcond(self):
@@ -230,10 +231,6 @@ class QAPISchemaEnumType(QAPISchemaType):
             for m in self.members:
                 doc.connect_member(m)
 
-    def check_doc(self):
-        if self.doc:
-            self.doc.check()
-
     def is_implicit(self):
         # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
         return self.name.endswith('Kind') or self.name == 'QType'
@@ -389,10 +386,6 @@ class QAPISchemaObjectType(QAPISchemaType):
             for m in self.local_members:
                 doc.connect_member(m)
 
-    def check_doc(self):
-        if self.doc:
-            self.doc.check()
-
     @property
     def ifcond(self):
         assert self._checked
@@ -667,10 +660,6 @@ class QAPISchemaAlternateType(QAPISchemaType):
             for v in self.variants.variants:
                 doc.connect_member(v)
 
-    def check_doc(self):
-        if self.doc:
-            self.doc.check()
-
     def c_type(self):
         return c_name(self.name) + pointer_suffix
 
@@ -745,10 +734,6 @@ class QAPISchemaCommand(QAPISchemaEntity):
             if self.arg_type and self.arg_type.is_implicit():
                 self.arg_type.connect_doc(doc)
 
-    def check_doc(self):
-        if self.doc:
-            self.doc.check()
-
     def visit(self, visitor):
         QAPISchemaEntity.visit(self, visitor)
         visitor.visit_command(self.name, self.info, self.ifcond,
@@ -791,10 +776,6 @@ class QAPISchemaEvent(QAPISchemaEntity):
             if self.arg_type and self.arg_type.is_implicit():
                 self.arg_type.connect_doc(doc)
 
-    def check_doc(self):
-        if self.doc:
-            self.doc.check()
-
     def visit(self, visitor):
         QAPISchemaEntity.visit(self, visitor)
         visitor.visit_event(self.name, self.info, self.ifcond,
-- 
cgit 1.4.1


From b3cdff10e5e82ba7b99c59ab3089883f6bb85ed8 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:35 +0200
Subject: qapi: Lift features into QAPISchemaEntity

Commit 6a8c0b5102 "qapi: Add feature flags to struct types" added
features to QAPISchemaObjectType.  Commit a95daa5093 "qapi: Add
feature flags to commands in qapi" added them to QAPISchemaCommand,
duplicating the code.  Tolerable, but the duplication will only get
worse as we add features to more definitions.

To de-duplicate, lift features from QAPISchemaObjectType and
QAPISchemaCommand into QAPISchemaEntity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-18-armbru@redhat.com>
---
 scripts/qapi/schema.py | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index ee510f129b..bdea9482fc 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -27,8 +27,11 @@ from qapi.parser import QAPISchemaParser
 class QAPISchemaEntity(object):
     meta = None
 
-    def __init__(self, name, info, doc, ifcond=None):
+    def __init__(self, name, info, doc, ifcond=None, features=None):
         assert name is None or isinstance(name, str)
+        for f in features or []:
+            assert isinstance(f, QAPISchemaFeature)
+            f.set_defined_in(name)
         self.name = name
         self._module = None
         # For explicitly defined entities, info points to the (explicit)
@@ -39,6 +42,7 @@ class QAPISchemaEntity(object):
         self.info = info
         self.doc = doc
         self._ifcond = ifcond or []
+        self.features = features or []
         self._checked = False
 
     def c_name(self):
@@ -49,6 +53,10 @@ class QAPISchemaEntity(object):
         if self.info:
             self._module = os.path.relpath(self.info.fname,
                                            os.path.dirname(schema.fname))
+        seen = {}
+        for f in self.features:
+            f.check_clash(self.info, seen)
+
         self._checked = True
 
     def connect_doc(self, doc=None):
@@ -307,7 +315,7 @@ class QAPISchemaObjectType(QAPISchemaType):
         # struct has local_members, optional base, and no variants
         # flat union has base, variants, and no local_members
         # simple union has local_members, variants, and no base
-        QAPISchemaType.__init__(self, name, info, doc, ifcond)
+        QAPISchemaType.__init__(self, name, info, doc, ifcond, features)
         self.meta = 'union' if variants else 'struct'
         assert base is None or isinstance(base, str)
         for m in local_members:
@@ -316,15 +324,11 @@ class QAPISchemaObjectType(QAPISchemaType):
         if variants is not None:
             assert isinstance(variants, QAPISchemaObjectTypeVariants)
             variants.set_defined_in(name)
-        for f in features:
-            assert isinstance(f, QAPISchemaFeature)
-            f.set_defined_in(name)
         self._base_name = base
         self.base = None
         self.local_members = local_members
         self.variants = variants
         self.members = None
-        self.features = features
 
     def check(self, schema):
         # This calls another type T's .check() exactly when the C
@@ -362,11 +366,6 @@ class QAPISchemaObjectType(QAPISchemaType):
             self.variants.check(schema, seen)
             self.variants.check_clash(self.info, seen)
 
-        # Features are in a name space separate from members
-        seen = {}
-        for f in self.features:
-            f.check_clash(self.info, seen)
-
         self.members = members  # mark completed
 
     # Check that the members of this type do not cause duplicate JSON members,
@@ -678,12 +677,9 @@ class QAPISchemaCommand(QAPISchemaEntity):
     def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
                  gen, success_response, boxed, allow_oob, allow_preconfig,
                  features):
-        QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
+        QAPISchemaEntity.__init__(self, name, info, doc, ifcond, features)
         assert not arg_type or isinstance(arg_type, str)
         assert not ret_type or isinstance(ret_type, str)
-        for f in features:
-            assert isinstance(f, QAPISchemaFeature)
-            f.set_defined_in(name)
         self._arg_type_name = arg_type
         self.arg_type = None
         self._ret_type_name = ret_type
@@ -693,7 +689,6 @@ class QAPISchemaCommand(QAPISchemaEntity):
         self.boxed = boxed
         self.allow_oob = allow_oob
         self.allow_preconfig = allow_preconfig
-        self.features = features
 
     def check(self, schema):
         QAPISchemaEntity.check(self, schema)
@@ -723,11 +718,6 @@ class QAPISchemaCommand(QAPISchemaEntity):
                         "command's 'returns' cannot take %s"
                         % self.ret_type.describe())
 
-        # Features are in a name space separate from members
-        seen = {}
-        for f in self.features:
-            f.check_clash(self.info, seen)
-
     def connect_doc(self, doc=None):
         doc = doc or self.doc
         if doc:
-- 
cgit 1.4.1


From e151941d1b691402f7914750e025209b7839a1c0 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 24 Oct 2019 13:02:37 +0200
Subject: qapi: Check feature documentation against the schema

Commit f3ed93d545 "qapi: Allow documentation for features" neglected
to check documentation against the schema.  Fix that: check them the
same way we check arguments.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-20-armbru@redhat.com>
---
 scripts/qapi/parser.py                   | 31 ++++++++++++++++++++++---------
 scripts/qapi/schema.py                   |  2 ++
 tests/qapi-schema/doc-bad-feature.err    |  1 +
 tests/qapi-schema/doc-bad-feature.json   |  1 -
 tests/qapi-schema/doc-bad-feature.out    | 19 -------------------
 tests/qapi-schema/doc-undoc-feature.err  |  2 ++
 tests/qapi-schema/doc-undoc-feature.json |  1 -
 tests/qapi-schema/doc-undoc-feature.out  | 21 ---------------------
 8 files changed, 27 insertions(+), 51 deletions(-)

(limited to 'scripts/qapi/schema.py')

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 6c45a00cf4..342792e410 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -555,18 +555,31 @@ class QAPIDoc(object):
             self.args[member.name] = QAPIDoc.ArgSection(member.name)
         self.args[member.name].connect(member)
 
+    def connect_feature(self, feature):
+        if feature.name not in self.features:
+            raise QAPISemError(feature.info,
+                               "feature '%s' lacks documentation"
+                               % feature.name)
+            self.features[feature.name] = QAPIDoc.ArgSection(feature.name)
+        self.features[feature.name].connect(feature)
+
     def check_expr(self, expr):
         if self.has_section('Returns') and 'command' not in expr:
             raise QAPISemError(self.info,
                                "'Returns:' is only valid for commands")
 
     def check(self):
-        bogus = [name for name, section in self.args.items()
-                 if not section.member]
-        if bogus:
-            raise QAPISemError(
-                self.info,
-                "documented member%s '%s' %s not exist"
-                % ("s" if len(bogus) > 1 else "",
-                   "', '".join(bogus),
-                   "do" if len(bogus) > 1 else "does"))
+
+        def check_args_section(args, info, what):
+            bogus = [name for name, section in args.items()
+                     if not section.member]
+            if bogus:
+                raise QAPISemError(
+                    self.info,
+                    "documented member%s '%s' %s not exist"
+                    % ("s" if len(bogus) > 1 else "",
+                       "', '".join(bogus),
+                       "do" if len(bogus) > 1 else "does"))
+
+        check_args_section(self.args, self.info, 'members')
+        check_args_section(self.features, self.info, 'features')
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index bdea9482fc..cf0045f34e 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -56,6 +56,8 @@ class QAPISchemaEntity(object):
         seen = {}
         for f in self.features:
             f.check_clash(self.info, seen)
+            if self.doc:
+                self.doc.connect_feature(f)
 
         self._checked = True
 
diff --git a/tests/qapi-schema/doc-bad-feature.err b/tests/qapi-schema/doc-bad-feature.err
index e69de29bb2..e4c62adfa3 100644
--- a/tests/qapi-schema/doc-bad-feature.err
+++ b/tests/qapi-schema/doc-bad-feature.err
@@ -0,0 +1 @@
+doc-bad-feature.json:3: documented member 'a' does not exist
diff --git a/tests/qapi-schema/doc-bad-feature.json b/tests/qapi-schema/doc-bad-feature.json
index 2a78e3b1db..3d49b8e607 100644
--- a/tests/qapi-schema/doc-bad-feature.json
+++ b/tests/qapi-schema/doc-bad-feature.json
@@ -1,5 +1,4 @@
 # Features listed in the doc comment must exist in the actual schema
-# BUG: nonexistent @a is not rejected
 
 ##
 # @foo:
diff --git a/tests/qapi-schema/doc-bad-feature.out b/tests/qapi-schema/doc-bad-feature.out
index fef4a3e400..e69de29bb2 100644
--- a/tests/qapi-schema/doc-bad-feature.out
+++ b/tests/qapi-schema/doc-bad-feature.out
@@ -1,19 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module doc-bad-feature.json
-command foo None -> None
-    gen=True success_response=True boxed=False oob=False preconfig=False
-doc symbol=foo
-    body=
-
-    feature=a
-a
diff --git a/tests/qapi-schema/doc-undoc-feature.err b/tests/qapi-schema/doc-undoc-feature.err
index e69de29bb2..62fc82d2b9 100644
--- a/tests/qapi-schema/doc-undoc-feature.err
+++ b/tests/qapi-schema/doc-undoc-feature.err
@@ -0,0 +1,2 @@
+doc-undoc-feature.json: In command 'foo':
+doc-undoc-feature.json:9: feature 'undoc' lacks documentation
diff --git a/tests/qapi-schema/doc-undoc-feature.json b/tests/qapi-schema/doc-undoc-feature.json
index c7650d9974..c52f88e2cd 100644
--- a/tests/qapi-schema/doc-undoc-feature.json
+++ b/tests/qapi-schema/doc-undoc-feature.json
@@ -1,5 +1,4 @@
 # Doc comment must cover all features
-# BUG: missing documentation for @undoc not caught
 
 ##
 # @foo:
diff --git a/tests/qapi-schema/doc-undoc-feature.out b/tests/qapi-schema/doc-undoc-feature.out
index cdb097361f..e69de29bb2 100644
--- a/tests/qapi-schema/doc-undoc-feature.out
+++ b/tests/qapi-schema/doc-undoc-feature.out
@@ -1,21 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module doc-undoc-feature.json
-command foo None -> None
-    gen=True success_response=True boxed=False oob=False preconfig=False
-    feature undoc
-    feature doc
-doc symbol=foo
-    body=
-
-    feature=doc
-documented feature
-- 
cgit 1.4.1