Browse Source

qapi: Make doc comments optional where we don't need them

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>
Markus Armbruster 8 years ago
parent
commit
bc52d03ff5

+ 30 - 13
docs/qapi-code-gen.txt

@@ -117,10 +117,13 @@ Example:
 
 
 ==== Expression documentation ====
 ==== 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
 documentation block.  Such blocks are called expression documentation
 blocks.
 blocks.
 
 
+When documentation is required (see pragma 'doc-required'), expression
+documentation blocks are mandatory.
+
 The documentation block consists of a first line naming the
 The documentation block consists of a first line naming the
 expression, an optional overview, a description of each argument (for
 expression, an optional overview, a description of each argument (for
 commands and events) or member (for structs, unions and alternates),
 commands and events) or member (for structs, unions and alternates),
@@ -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
 not used by any commands or events in the Client JSON Protocol, for
 the side effect of generated C code used internally.
 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,
 All names must begin with a letter, and contain only ASCII letters,
 digits, hyphen, and underscore.  There are two exceptions: enum values
 digits, hyphen, and underscore.  There are two exceptions: enum values
@@ -282,7 +285,7 @@ The following types are predefined, and map to C as follows:
   QType     QType      JSON string matching enum QType values
   QType     QType      JSON string matching enum QType values
 
 
 
 
-=== Includes ===
+=== Include directives ===
 
 
 Usage: { 'include': STRING }
 Usage: { 'include': STRING }
 
 
@@ -302,6 +305,20 @@ an outer file.  The parser may be made stricter in the future to
 prevent incomplete include files.
 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 ===
 === Struct types ===
 
 
 Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
 Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }

+ 2 - 0
qapi-schema.json

@@ -49,6 +49,8 @@
 #
 #
 ##
 ##
 
 
+{ 'pragma': { 'doc-required': true } }
+
 # QAPI common definitions
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
 { 'include': 'qapi/common.json' }
 
 

+ 2 - 0
qga/qapi-schema.json

@@ -11,6 +11,8 @@
 #
 #
 ##
 ##
 
 
+{ 'pragma': { 'doc-required': true } }
+
 ##
 ##
 # @guest-sync-delimited:
 # @guest-sync-delimited:
 #
 #

+ 23 - 1
scripts/qapi.py

@@ -37,6 +37,9 @@
     'QType':    'QTYPE_QSTRING',
     'QType':    'QTYPE_QSTRING',
 }
 }
 
 
+# Are documentation comments required?
+doc_required = False
+
 # Whitelist of commands allowed to return a non-dictionary
 # Whitelist of commands allowed to return a non-dictionary
 returns_whitelist = [
 returns_whitelist = [
     # From QMP:
     # From QMP:
@@ -277,6 +280,15 @@ def __init__(self, fp, previously_included=[], incl_info=None):
                                        "Value of 'include' must be a string")
                                        "Value of 'include' must be a string")
                 self._include(include, info, os.path.dirname(abs_fname),
                 self._include(include, info, os.path.dirname(abs_fname),
                               previously_included)
                               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:
             else:
                 expr_elem = {'expr': expr,
                 expr_elem = {'expr': expr,
                              'info': info}
                              'info': info}
@@ -308,6 +320,16 @@ def _include(self, include, info, base_dir, previously_included):
         self.exprs.extend(exprs_include.exprs)
         self.exprs.extend(exprs_include.exprs)
         self.docs.extend(exprs_include.docs)
         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):
     def accept(self, skip_comment=True):
         while True:
         while True:
             self.tok = self.src[self.cursor]
             self.tok = self.src[self.cursor]
@@ -863,7 +885,7 @@ def check_exprs(exprs):
         expr = expr_elem['expr']
         expr = expr_elem['expr']
         info = expr_elem['info']
         info = expr_elem['info']
 
 
-        if 'doc' not in expr_elem:
+        if 'doc' not in expr_elem and doc_required:
             raise QAPISemError(info,
             raise QAPISemError(info,
                                "Expression missing documentation comment")
                                "Expression missing documentation comment")
 
 

+ 3 - 0
scripts/qapi2texi.py

@@ -254,6 +254,9 @@ def main(argv):
         sys.exit(1)
         sys.exit(1)
 
 
     schema = qapi.QAPISchema(argv[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)
     print texi(schema.docs)
 
 
 
 

+ 5 - 0
tests/Makefile.include

@@ -381,6 +381,7 @@ qapi-schema += doc-invalid-end2.json
 qapi-schema += doc-invalid-return.json
 qapi-schema += doc-invalid-return.json
 qapi-schema += doc-invalid-section.json
 qapi-schema += doc-invalid-section.json
 qapi-schema += doc-invalid-start.json
 qapi-schema += doc-invalid-start.json
+qapi-schema += doc-missing.json
 qapi-schema += doc-missing-colon.json
 qapi-schema += doc-missing-colon.json
 qapi-schema += doc-missing-expr.json
 qapi-schema += doc-missing-expr.json
 qapi-schema += doc-missing-space.json
 qapi-schema += doc-missing-space.json
@@ -422,6 +423,7 @@ qapi-schema += funny-char.json
 qapi-schema += ident-with-escape.json
 qapi-schema += ident-with-escape.json
 qapi-schema += include-before-err.json
 qapi-schema += include-before-err.json
 qapi-schema += include-cycle.json
 qapi-schema += include-cycle.json
+qapi-schema += include-extra-junk.json
 qapi-schema += include-format-err.json
 qapi-schema += include-format-err.json
 qapi-schema += include-nested-err.json
 qapi-schema += include-nested-err.json
 qapi-schema += include-no-file.json
 qapi-schema += include-no-file.json
@@ -439,6 +441,9 @@ qapi-schema += missing-comma-object.json
 qapi-schema += missing-type.json
 qapi-schema += missing-type.json
 qapi-schema += nested-struct-data.json
 qapi-schema += nested-struct-data.json
 qapi-schema += non-objects.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 += qapi-schema-test.json
 qapi-schema += quoted-structural-chars.json
 qapi-schema += quoted-structural-chars.json
 qapi-schema += redefined-builtin.json
 qapi-schema += redefined-builtin.json

+ 1 - 0
tests/qapi-schema/doc-missing.err

@@ -0,0 +1 @@
+tests/qapi-schema/doc-missing.json:5: Expression missing documentation comment

+ 1 - 0
tests/qapi-schema/doc-missing.exit

@@ -0,0 +1 @@
+1

+ 5 - 0
tests/qapi-schema/doc-missing.json

@@ -0,0 +1,5 @@
+# Expression documentation required
+
+{ 'pragma': { 'doc-required': true } }
+
+{ 'command': 'undocumented' }

+ 0 - 0
tests/qapi-schema/doc-missing.out


+ 1 - 0
tests/qapi-schema/include-extra-junk.err

@@ -0,0 +1 @@
+tests/qapi-schema/include-extra-junk.json:3: Invalid 'include' directive

+ 1 - 0
tests/qapi-schema/include-extra-junk.exit

@@ -0,0 +1 @@
+1

+ 3 - 0
tests/qapi-schema/include-extra-junk.json

@@ -0,0 +1,3 @@
+# 'include' must be the sole member
+
+{ 'include': 'comments.json', 'junk': true }

+ 0 - 0
tests/qapi-schema/include-extra-junk.out


+ 1 - 0
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 - 0
tests/qapi-schema/pragma-doc-required-crap.exit

@@ -0,0 +1 @@
+1

+ 3 - 0
tests/qapi-schema/pragma-doc-required-crap.json

@@ -0,0 +1,3 @@
+# 'doc-required' must be bool
+
+{ 'pragma': { 'doc-required': {} } }

+ 0 - 0
tests/qapi-schema/pragma-doc-required-crap.out


+ 1 - 0
tests/qapi-schema/pragma-extra-junk.err

@@ -0,0 +1 @@
+tests/qapi-schema/pragma-extra-junk.json:3: Invalid 'pragma' directive

+ 1 - 0
tests/qapi-schema/pragma-extra-junk.exit

@@ -0,0 +1 @@
+1

+ 3 - 0
tests/qapi-schema/pragma-extra-junk.json

@@ -0,0 +1,3 @@
+# 'pragma' must be the sole member
+
+{ 'pragma': { 'doc-required': true }, 'junk': true }

+ 0 - 0
tests/qapi-schema/pragma-extra-junk.out


+ 1 - 0
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 - 0
tests/qapi-schema/pragma-non-dict.exit

@@ -0,0 +1 @@
+1

+ 3 - 0
tests/qapi-schema/pragma-non-dict.json

@@ -0,0 +1,3 @@
+# Value of 'pragma' must be a dictionary
+
+{ 'pragma': [] }

+ 0 - 0
tests/qapi-schema/pragma-non-dict.out