| Message ID | 1489385927-6735-38-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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> writes: > 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) Nice to have, but whether it would be worth the error recovery complications is doubtful. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
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
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(-)