Skip to content

Commit

Permalink
qapi: Make doc comments optional where we don't need them
Browse files Browse the repository at this point in the history
Since we added the documentation generator in commit 3313b61, doc
comments are mandatory.  That's a very good idea for a schema that
needs to be documented, but has proven to be annoying for testing.

Make doc comments optional again, but add a new directive

    { 'pragma': { 'doc-required': true } }

to let a QAPI schema require them.

Add test cases for the new pragma directive.  While there, plug a
minor hole in includ directive test coverage.

Require documentation in the schemas we actually want documented:
qapi-schema.json and qga/qapi-schema.json.

We could probably make qapi2texi.py cope with incomplete
documentation, but for now, simply make it refuse to run unless the
schema has 'doc-required': true.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1489582656-31133-3-git-send-email-armbru@redhat.com>
[qapi-code-gen.txt wording tweaked]
Reviewed-by: Eric Blake <eblake@redhat.com>
  • Loading branch information
Markus Armbruster committed Mar 16, 2017
1 parent e04dea8 commit bc52d03
Show file tree
Hide file tree
Showing 26 changed files with 92 additions and 14 deletions.
43 changes: 30 additions & 13 deletions docs/qapi-code-gen.txt
Expand Up @@ -117,10 +117,13 @@ Example:

==== Expression documentation ====

Each expression that isn't an include directive must be preceded by a
Each expression that isn't an include directive may be preceded by a
documentation block. Such blocks are called expression documentation
blocks.

When documentation is required (see pragma 'doc-required'), expression
documentation blocks are mandatory.

The documentation block consists of a first line naming the
expression, an optional overview, a description of each argument (for
commands and events) or member (for structs, unions and alternates),
Expand Down Expand Up @@ -204,17 +207,17 @@ once. It is permissible for the schema to contain additional types
not used by any commands or events in the Client JSON Protocol, for
the side effect of generated C code used internally.

There are seven top-level expressions recognized by the parser:
'include', 'command', 'struct', 'enum', 'union', 'alternate', and
'event'. There are several groups of types: simple types (a number of
built-in types, such as 'int' and 'str'; as well as enumerations),
complex types (structs and two flavors of unions), and alternate types
(a choice between other types). The 'command' and 'event' expressions
can refer to existing types by name, or list an anonymous type as a
dictionary. Listing a type name inside an array refers to a
single-dimension array of that type; multi-dimension arrays are not
directly supported (although an array of a complex struct that
contains an array member is possible).
There are eight top-level expressions recognized by the parser:
'include', 'pragma', 'command', 'struct', 'enum', 'union',
'alternate', and 'event'. There are several groups of types: simple
types (a number of built-in types, such as 'int' and 'str'; as well as
enumerations), complex types (structs and two flavors of unions), and
alternate types (a choice between other types). The 'command' and
'event' expressions can refer to existing types by name, or list an
anonymous type as a dictionary. Listing a type name inside an array
refers to a single-dimension array of that type; multi-dimension
arrays are not directly supported (although an array of a complex
struct that contains an array member is possible).

All names must begin with a letter, and contain only ASCII letters,
digits, hyphen, and underscore. There are two exceptions: enum values
Expand Down Expand Up @@ -282,7 +285,7 @@ The following types are predefined, and map to C as follows:
QType QType JSON string matching enum QType values


=== Includes ===
=== Include directives ===

Usage: { 'include': STRING }

Expand All @@ -302,6 +305,20 @@ an outer file. The parser may be made stricter in the future to
prevent incomplete include files.


=== Pragma directives ===

Usage: { 'pragma': DICT }

The pragma directive lets you control optional generator behavior.
The dictionary's entries are pragma names and values.

Pragma's scope is currently the complete schema. Setting the same
pragma to different values in parts of the schema doesn't work.

Pragma 'doc-required' takes a boolean value. If true, documentation
is required. Default is false.


=== Struct types ===

Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
Expand Down
2 changes: 2 additions & 0 deletions qapi-schema.json
Expand Up @@ -49,6 +49,8 @@
#
##

{ 'pragma': { 'doc-required': true } }

# QAPI common definitions
{ 'include': 'qapi/common.json' }

Expand Down
2 changes: 2 additions & 0 deletions qga/qapi-schema.json
Expand Up @@ -11,6 +11,8 @@
#
##

{ 'pragma': { 'doc-required': true } }

##
# @guest-sync-delimited:
#
Expand Down
24 changes: 23 additions & 1 deletion scripts/qapi.py
Expand Up @@ -37,6 +37,9 @@
'QType': 'QTYPE_QSTRING',
}

# Are documentation comments required?
doc_required = False

# Whitelist of commands allowed to return a non-dictionary
returns_whitelist = [
# From QMP:
Expand Down Expand Up @@ -277,6 +280,15 @@ def __init__(self, fp, previously_included=[], incl_info=None):
"Value of 'include' must be a string")
self._include(include, info, os.path.dirname(abs_fname),
previously_included)
elif "pragma" in expr:
if len(expr) != 1:
raise QAPISemError(info, "Invalid 'pragma' directive")
pragma = expr['pragma']
if not isinstance(pragma, dict):
raise QAPISemError(
info, "Value of 'pragma' must be a dictionary")
for name, value in pragma.iteritems():
self._pragma(name, value, info)
else:
expr_elem = {'expr': expr,
'info': info}
Expand Down Expand Up @@ -308,6 +320,16 @@ def _include(self, include, info, base_dir, previously_included):
self.exprs.extend(exprs_include.exprs)
self.docs.extend(exprs_include.docs)

def _pragma(self, name, value, info):
global doc_required
if name == 'doc-required':
if not isinstance(value, bool):
raise QAPISemError(info,
"Pragma 'doc-required' must be boolean")
doc_required = value
else:
raise QAPISemError(info, "Unknown pragma '%s'" % name)

def accept(self, skip_comment=True):
while True:
self.tok = self.src[self.cursor]
Expand Down Expand Up @@ -863,7 +885,7 @@ def check_exprs(exprs):
expr = expr_elem['expr']
info = expr_elem['info']

if 'doc' not in expr_elem:
if 'doc' not in expr_elem and doc_required:
raise QAPISemError(info,
"Expression missing documentation comment")

Expand Down
3 changes: 3 additions & 0 deletions scripts/qapi2texi.py
Expand Up @@ -254,6 +254,9 @@ def main(argv):
sys.exit(1)

schema = qapi.QAPISchema(argv[1])
if not qapi.doc_required:
print >>sys.stderr, ("%s: need pragma 'doc-required' "
"to generate documentation" % argv[0])
print texi(schema.docs)


Expand Down
5 changes: 5 additions & 0 deletions tests/Makefile.include
Expand Up @@ -381,6 +381,7 @@ qapi-schema += doc-invalid-end2.json
qapi-schema += doc-invalid-return.json
qapi-schema += doc-invalid-section.json
qapi-schema += doc-invalid-start.json
qapi-schema += doc-missing.json
qapi-schema += doc-missing-colon.json
qapi-schema += doc-missing-expr.json
qapi-schema += doc-missing-space.json
Expand Down Expand Up @@ -422,6 +423,7 @@ qapi-schema += funny-char.json
qapi-schema += ident-with-escape.json
qapi-schema += include-before-err.json
qapi-schema += include-cycle.json
qapi-schema += include-extra-junk.json
qapi-schema += include-format-err.json
qapi-schema += include-nested-err.json
qapi-schema += include-no-file.json
Expand All @@ -439,6 +441,9 @@ qapi-schema += missing-comma-object.json
qapi-schema += missing-type.json
qapi-schema += nested-struct-data.json
qapi-schema += non-objects.json
qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-non-dict.json
qapi-schema += qapi-schema-test.json
qapi-schema += quoted-structural-chars.json
qapi-schema += redefined-builtin.json
Expand Down
1 change: 1 addition & 0 deletions tests/qapi-schema/doc-missing.err
@@ -0,0 +1 @@
tests/qapi-schema/doc-missing.json:5: Expression missing documentation comment
1 change: 1 addition & 0 deletions tests/qapi-schema/doc-missing.exit
@@ -0,0 +1 @@
1
5 changes: 5 additions & 0 deletions tests/qapi-schema/doc-missing.json
@@ -0,0 +1,5 @@
# Expression documentation required

{ 'pragma': { 'doc-required': true } }

{ 'command': 'undocumented' }
Empty file.
1 change: 1 addition & 0 deletions tests/qapi-schema/include-extra-junk.err
@@ -0,0 +1 @@
tests/qapi-schema/include-extra-junk.json:3: Invalid 'include' directive
1 change: 1 addition & 0 deletions tests/qapi-schema/include-extra-junk.exit
@@ -0,0 +1 @@
1
3 changes: 3 additions & 0 deletions tests/qapi-schema/include-extra-junk.json
@@ -0,0 +1,3 @@
# 'include' must be the sole member

{ 'include': 'comments.json', 'junk': true }
Empty file.
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-doc-required-crap.err
@@ -0,0 +1 @@
tests/qapi-schema/pragma-doc-required-crap.json:3: Pragma 'doc-required' must be boolean
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-doc-required-crap.exit
@@ -0,0 +1 @@
1
3 changes: 3 additions & 0 deletions tests/qapi-schema/pragma-doc-required-crap.json
@@ -0,0 +1,3 @@
# 'doc-required' must be bool

{ 'pragma': { 'doc-required': {} } }
Empty file.
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-extra-junk.err
@@ -0,0 +1 @@
tests/qapi-schema/pragma-extra-junk.json:3: Invalid 'pragma' directive
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-extra-junk.exit
@@ -0,0 +1 @@
1
3 changes: 3 additions & 0 deletions tests/qapi-schema/pragma-extra-junk.json
@@ -0,0 +1,3 @@
# 'pragma' must be the sole member

{ 'pragma': { 'doc-required': true }, 'junk': true }
Empty file.
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-non-dict.err
@@ -0,0 +1 @@
tests/qapi-schema/pragma-non-dict.json:3: Value of 'pragma' must be a dictionary
1 change: 1 addition & 0 deletions tests/qapi-schema/pragma-non-dict.exit
@@ -0,0 +1 @@
1
3 changes: 3 additions & 0 deletions tests/qapi-schema/pragma-non-dict.json
@@ -0,0 +1,3 @@
# Value of 'pragma' must be a dictionary

{ 'pragma': [] }
Empty file.

0 comments on commit bc52d03

Please sign in to comment.