| Message ID | 1411165504-18198-14-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > Now that we know every expression is valid with regards to > its keys, we can add further tests that those keys refer to > valid types. With this patch, all references to a type (the > 'data': of command, type, union, and event, and the 'returns': > of command) must resolve to a builtin or another type declared > by the current qapi parse; this includes recursing into each > member of a data dictionary. Dealing with '**' and nested > sub-structs will be done in later patches. > > Update the testsuite to match improved output. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > scripts/qapi.py | 73 ++++++++++++++++++++++++++-- > tests/qapi-schema/data-array-empty.err | 1 + > tests/qapi-schema/data-array-empty.exit | 2 +- > tests/qapi-schema/data-array-empty.json | 2 +- > tests/qapi-schema/data-array-empty.out | 3 -- > tests/qapi-schema/data-array-unknown.err | 1 + > tests/qapi-schema/data-array-unknown.exit | 2 +- > tests/qapi-schema/data-array-unknown.json | 2 +- > tests/qapi-schema/data-array-unknown.out | 3 -- > tests/qapi-schema/data-int.err | 1 + > tests/qapi-schema/data-int.exit | 2 +- > tests/qapi-schema/data-int.json | 2 +- > tests/qapi-schema/data-int.out | 3 -- > tests/qapi-schema/data-member-array-bad.err | 1 + > tests/qapi-schema/data-member-array-bad.exit | 2 +- > tests/qapi-schema/data-member-array-bad.json | 2 +- > tests/qapi-schema/data-member-array-bad.out | 3 -- > tests/qapi-schema/data-member-unknown.err | 1 + > tests/qapi-schema/data-member-unknown.exit | 2 +- > tests/qapi-schema/data-member-unknown.json | 2 +- > tests/qapi-schema/data-member-unknown.out | 3 -- > tests/qapi-schema/data-unknown.err | 1 + > tests/qapi-schema/data-unknown.exit | 2 +- > tests/qapi-schema/data-unknown.json | 2 +- > tests/qapi-schema/data-unknown.out | 3 -- > tests/qapi-schema/returns-array-bad.err | 1 + > tests/qapi-schema/returns-array-bad.exit | 2 +- > tests/qapi-schema/returns-array-bad.json | 2 +- > tests/qapi-schema/returns-array-bad.out | 3 -- > tests/qapi-schema/returns-unknown.err | 1 + > tests/qapi-schema/returns-unknown.exit | 2 +- > tests/qapi-schema/returns-unknown.json | 2 +- > tests/qapi-schema/returns-unknown.out | 3 -- > 33 files changed, 93 insertions(+), 44 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index bf243fa..20c0ce9 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -255,6 +255,52 @@ def discriminator_find_enum_define(expr): > > return find_enum(discriminator_type) > > +def check_type(expr_info, source, data, allow_array = False, > + allowed_names = [], allow_dict = True): > + global all_types > + > + if data is None: > + return > + > + if data == '**': > + return > + > + # Check if array type for data is okay > + if isinstance(data, list): > + if not allow_array: > + raise QAPIExprError(expr_info, > + "%s cannot be an array" % source) > + if len(data) != 1 or not isinstance(data[0], basestring): > + raise QAPIExprError(expr_info, > + "%s: array type must contain single type name" > + % source) > + data = data[0] > + > + # Check if type name for data is okay > + if isinstance(data, basestring): > + if not data in all_types: > + raise QAPIExprError(expr_info, > + "%s references unknown type '%s'" > + % (source, data)) > + if not all_types[data] in allowed_names: > + raise QAPIExprError(expr_info, > + "%s cannot reference %s type '%s'" > + % (source, all_types[data], data)) > + return > + > + # data is a dictionary, check that each member is okay > + if not isinstance(data, OrderedDict): > + raise QAPIExprError(expr_info, > + "%s should be a dictionary" % source) > + if not allow_dict: > + raise QAPIExprError(expr_info, > + "%s should be a type name" % source) > + for (key, value) in data.items(): > + check_type(expr_info, "member '%s' of %s" % (key, source), value, > + allow_array=True, > + allowed_names=['built-in', 'union', 'struct', 'enum'], > + allow_dict=True) Hmm, allowed_names isn't about allowed names, it's about allowed meta-types. Rename? > + > def check_command(expr, expr_info): > global commands > name = expr['command'] > @@ -262,6 +308,15 @@ def check_command(expr, expr_info): > raise QAPIExprError(expr_info, > "command '%s' is already defined" % name) > commands.append(name) > + check_type(expr_info, "'data' for command '%s'" % name, > + expr.get('data'), allow_array=True, > + allowed_names=['union', 'struct']) > + check_type(expr_info, "'base' for command '%s'" % name, > + expr.get('base'), allowed_names=['struct'], > + allow_dict=False) > + check_type(expr_info, "'returns' for command '%s'" % name, > + expr.get('returns'), allow_array=True, > + allowed_names=['built-in', 'union', 'struct', 'enum']) > Nicely done. > def check_event(expr, expr_info): > global events > @@ -271,6 +326,8 @@ def check_event(expr, expr_info): > raise QAPIExprError(expr_info, > "event '%s' is already defined" % name) > events.append(name) > + check_type(expr_info, "'data' for event '%s'" % name, > + expr.get('data'), allowed_names=['union', 'struct']) > if params: > for argname, argentry, optional, structured in parse_args(params): > if structured: > @@ -357,19 +414,27 @@ def check_enum(expr, expr_info): > % (name, member, values[key])) > values[key] = member > > +def check_struct(expr, expr_info): > + name = expr['type'] > + members = expr['data'] > + > + check_type(expr_info, "'data' for type '%s'" % name, members) > + > def check_exprs(schema): > for expr_elem in schema.exprs: > expr = expr_elem['expr'] > info = expr_elem['info'] > > - if expr.has_key('union'): > - check_union(expr, info) > - if expr.has_key('event'): > - check_event(expr, info) > if expr.has_key('enum'): > check_enum(expr, info) > + if expr.has_key('union'): > + check_union(expr, info) > + if expr.has_key('type'): > + check_struct(expr, info) > if expr.has_key('command'): > check_command(expr, info) > + if expr.has_key('event'): > + check_event(expr, info) Not related to this patch: this chain of ifs bothers me. The conditions should be exclusive, because each expression must have a well-defined meta-type. If I remember correctly, you actually enforce this elsewhere in your series. Do we want to make it obvious in the code here? elif instead of if, perhaps? > > def check_keys(expr_elem, meta, required, optional=[]): > expr = expr_elem['expr'] > diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/data-array-empty.err > index e69de29..94e046b 100644 > --- a/tests/qapi-schema/data-array-empty.err > +++ b/tests/qapi-schema/data-array-empty.err > @@ -0,0 +1 @@ > +tests/qapi-schema/data-array-empty.json:2: 'data' for command 'oops': array type must contain single type name > diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/data-array-empty.exit > index 573541a..d00491f 100644 > --- a/tests/qapi-schema/data-array-empty.exit > +++ b/tests/qapi-schema/data-array-empty.exit > @@ -1 +1 @@ > -0 > +1 > diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/data-array-empty.json > index 41b6c1e..fdd5612 100644 > --- a/tests/qapi-schema/data-array-empty.json > +++ b/tests/qapi-schema/data-array-empty.json > @@ -1,2 +1,2 @@ > -# FIXME: we should reject an array for data if it does not contain a known type > +# we reject an array for data if it does not contain a known type > { 'command': 'oops', 'data': [ ] } > diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/data-array-empty.out > index 67802be..e69de29 100644 > --- a/tests/qapi-schema/data-array-empty.out > +++ b/tests/qapi-schema/data-array-empty.out > @@ -1,3 +0,0 @@ > -[OrderedDict([('command', 'oops'), ('data', [])])] > -[] > -[] > diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err > index e69de29..a66c2d6 100644 > --- a/tests/qapi-schema/data-array-unknown.err > +++ b/tests/qapi-schema/data-array-unknown.err > @@ -0,0 +1 @@ > +tests/qapi-schema/data-array-unknown.json:2: 'data' for command 'oops' references unknown type 'NoSuchType' The wording "references type" somehow unduly excites my "reference type" synapses :) Perhaps something like "Type 'NoSuchType' is unknown" would suffice. Entirely up to you; feel free to keep the wording as is. > diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit > index 573541a..d00491f 100644 > --- a/tests/qapi-schema/data-array-unknown.exit > +++ b/tests/qapi-schema/data-array-unknown.exit > @@ -1 +1 @@ > -0 > +1 > diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/data-array-unknown.json > index 434fb5f..9c75b96 100644 > --- a/tests/qapi-schema/data-array-unknown.json > +++ b/tests/qapi-schema/data-array-unknown.json > @@ -1,2 +1,2 @@ > -# FIXME: we should reject an array for data if it does not contain a known type > +# we reject an array for data if it does not contain a known type > { 'command': 'oops', 'data': [ 'NoSuchType' ] } > diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out > index c3bc05e..e69de29 100644 > --- a/tests/qapi-schema/data-array-unknown.out > +++ b/tests/qapi-schema/data-array-unknown.out > @@ -1,3 +0,0 @@ > -[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])] > -[] > -[] > diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err > index e69de29..e1d3ed5 100644 > --- a/tests/qapi-schema/data-int.err > +++ b/tests/qapi-schema/data-int.err > @@ -0,0 +1 @@ > +tests/qapi-schema/data-int.json:2: 'data' for command 'oops' cannot reference built-in type 'int' Perhaps something like "'data' for command 'oops' can't be of built-in type 'int'", or maybe positive instead of negative: "a command's 'data' must be of complex type". Again, entirely up to you. [...]
Markus Armbruster <armbru@redhat.com> writes: > Eric Blake <eblake@redhat.com> writes: > >> Now that we know every expression is valid with regards to >> its keys, we can add further tests that those keys refer to >> valid types. With this patch, all references to a type (the >> 'data': of command, type, union, and event, and the 'returns': >> of command) must resolve to a builtin or another type declared >> by the current qapi parse; this includes recursing into each >> member of a data dictionary. Dealing with '**' and nested >> sub-structs will be done in later patches. >> >> Update the testsuite to match improved output. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> [...] >> @@ -262,6 +308,15 @@ def check_command(expr, expr_info): >> raise QAPIExprError(expr_info, >> "command '%s' is already defined" % name) >> commands.append(name) >> + check_type(expr_info, "'data' for command '%s'" % name, >> + expr.get('data'), allow_array=True, >> + allowed_names=['union', 'struct']) >> + check_type(expr_info, "'base' for command '%s'" % name, >> + expr.get('base'), allowed_names=['struct'], >> + allow_dict=False) >> + check_type(expr_info, "'returns' for command '%s'" % name, >> + expr.get('returns'), allow_array=True, >> + allowed_names=['built-in', 'union', 'struct', 'enum']) >> > > Nicely done. Wait a sec! What's a command's 'base'? [...]
On 09/29/2014 02:27 AM, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > >> Eric Blake <eblake@redhat.com> writes: >> >>> Now that we know every expression is valid with regards to >>> its keys, we can add further tests that those keys refer to >>> valid types. With this patch, all references to a type (the >>> 'data': of command, type, union, and event, and the 'returns': >>> of command) must resolve to a builtin or another type declared >>> by the current qapi parse; this includes recursing into each >>> member of a data dictionary. Dealing with '**' and nested >>> sub-structs will be done in later patches. >>> >>> Update the testsuite to match improved output. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> > [...] >>> @@ -262,6 +308,15 @@ def check_command(expr, expr_info): >>> raise QAPIExprError(expr_info, >>> "command '%s' is already defined" % name) >>> commands.append(name) >>> + check_type(expr_info, "'data' for command '%s'" % name, >>> + expr.get('data'), allow_array=True, >>> + allowed_names=['union', 'struct']) >>> + check_type(expr_info, "'base' for command '%s'" % name, >>> + expr.get('base'), allowed_names=['struct'], >>> + allow_dict=False) >> >> Nicely done. > > Wait a sec! What's a command's 'base'? Blech. You're right - only 'union' and 'struct' have a base. And since an earlier patch already filtered out 'base' as a non-allowed key for 'command, this check_type is dead code. All the more reason for me to spin v5 :)
On 09/26/2014 03:26 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> Now that we know every expression is valid with regards to >> its keys, we can add further tests that those keys refer to >> valid types. With this patch, all references to a type (the >> 'data': of command, type, union, and event, and the 'returns': >> of command) must resolve to a builtin or another type declared >> by the current qapi parse; this includes recursing into each >> member of a data dictionary. Dealing with '**' and nested >> sub-structs will be done in later patches. >> >> + for (key, value) in data.items(): >> + check_type(expr_info, "member '%s' of %s" % (key, source), value, >> + allow_array=True, >> + allowed_names=['built-in', 'union', 'struct', 'enum'], >> + allow_dict=True) > > Hmm, allowed_names isn't about allowed names, it's about allowed > meta-types. Rename? Will do. >> def check_exprs(schema): >> for expr_elem in schema.exprs: >> expr = expr_elem['expr'] >> info = expr_elem['info'] >> >> - if expr.has_key('union'): >> - check_union(expr, info) >> - if expr.has_key('event'): >> - check_event(expr, info) >> if expr.has_key('enum'): >> check_enum(expr, info) >> + if expr.has_key('union'): >> + check_union(expr, info) >> + if expr.has_key('type'): >> + check_struct(expr, info) >> if expr.has_key('command'): >> check_command(expr, info) >> + if expr.has_key('event'): >> + check_event(expr, info) > > Not related to this patch: this chain of ifs bothers me. The conditions > should be exclusive, because each expression must have a well-defined > meta-type. If I remember correctly, you actually enforce this elsewhere > in your series. Do we want to make it obvious in the code here? elif > instead of if, perhaps? Yes, earlier in the series guarantees that by this point, the conditions are now exclusive. It also bothers me that different points in the file are iterating over the 5 key types in different order, I'll clean that up in v5. >> +++ b/tests/qapi-schema/data-array-unknown.err >> @@ -0,0 +1 @@ >> +tests/qapi-schema/data-array-unknown.json:2: 'data' for command 'oops' references unknown type 'NoSuchType' > > The wording "references type" somehow unduly excites my "reference type" > synapses :) > > Perhaps something like "Type 'NoSuchType' is unknown" would suffice. > Entirely up to you; feel free to keep the wording as is. I'll play with it.
diff --git a/scripts/qapi.py b/scripts/qapi.py index bf243fa..20c0ce9 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -255,6 +255,52 @@ def discriminator_find_enum_define(expr): return find_enum(discriminator_type) +def check_type(expr_info, source, data, allow_array = False, + allowed_names = [], allow_dict = True): + global all_types + + if data is None: + return + + if data == '**': + return + + # Check if array type for data is okay + if isinstance(data, list): + if not allow_array: + raise QAPIExprError(expr_info, + "%s cannot be an array" % source) + if len(data) != 1 or not isinstance(data[0], basestring): + raise QAPIExprError(expr_info, + "%s: array type must contain single type name" + % source) + data = data[0] + + # Check if type name for data is okay + if isinstance(data, basestring): + if not data in all_types: + raise QAPIExprError(expr_info, + "%s references unknown type '%s'" + % (source, data)) + if not all_types[data] in allowed_names: + raise QAPIExprError(expr_info, + "%s cannot reference %s type '%s'" + % (source, all_types[data], data)) + return + + # data is a dictionary, check that each member is okay + if not isinstance(data, OrderedDict): + raise QAPIExprError(expr_info, + "%s should be a dictionary" % source) + if not allow_dict: + raise QAPIExprError(expr_info, + "%s should be a type name" % source) + for (key, value) in data.items(): + check_type(expr_info, "member '%s' of %s" % (key, source), value, + allow_array=True, + allowed_names=['built-in', 'union', 'struct', 'enum'], + allow_dict=True) + def check_command(expr, expr_info): global commands name = expr['command'] @@ -262,6 +308,15 @@ def check_command(expr, expr_info): raise QAPIExprError(expr_info, "command '%s' is already defined" % name) commands.append(name) + check_type(expr_info, "'data' for command '%s'" % name, + expr.get('data'), allow_array=True, + allowed_names=['union', 'struct']) + check_type(expr_info, "'base' for command '%s'" % name, + expr.get('base'), allowed_names=['struct'], + allow_dict=False) + check_type(expr_info, "'returns' for command '%s'" % name, + expr.get('returns'), allow_array=True, + allowed_names=['built-in', 'union', 'struct', 'enum']) def check_event(expr, expr_info): global events @@ -271,6 +326,8 @@ def check_event(expr, expr_info): raise QAPIExprError(expr_info, "event '%s' is already defined" % name) events.append(name) + check_type(expr_info, "'data' for event '%s'" % name, + expr.get('data'), allowed_names=['union', 'struct']) if params: for argname, argentry, optional, structured in parse_args(params): if structured: @@ -357,19 +414,27 @@ def check_enum(expr, expr_info): % (name, member, values[key])) values[key] = member +def check_struct(expr, expr_info): + name = expr['type'] + members = expr['data'] + + check_type(expr_info, "'data' for type '%s'" % name, members) + def check_exprs(schema): for expr_elem in schema.exprs: expr = expr_elem['expr'] info = expr_elem['info'] - if expr.has_key('union'): - check_union(expr, info) - if expr.has_key('event'): - check_event(expr, info) if expr.has_key('enum'): check_enum(expr, info) + if expr.has_key('union'): + check_union(expr, info) + if expr.has_key('type'): + check_struct(expr, info) if expr.has_key('command'): check_command(expr, info) + if expr.has_key('event'): + check_event(expr, info) def check_keys(expr_elem, meta, required, optional=[]): expr = expr_elem['expr'] diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/data-array-empty.err index e69de29..94e046b 100644 --- a/tests/qapi-schema/data-array-empty.err +++ b/tests/qapi-schema/data-array-empty.err @@ -0,0 +1 @@ +tests/qapi-schema/data-array-empty.json:2: 'data' for command 'oops': array type must contain single type name diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/data-array-empty.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-array-empty.exit +++ b/tests/qapi-schema/data-array-empty.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/data-array-empty.json index 41b6c1e..fdd5612 100644 --- a/tests/qapi-schema/data-array-empty.json +++ b/tests/qapi-schema/data-array-empty.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an array for data if it does not contain a known type +# we reject an array for data if it does not contain a known type { 'command': 'oops', 'data': [ ] } diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/data-array-empty.out index 67802be..e69de29 100644 --- a/tests/qapi-schema/data-array-empty.out +++ b/tests/qapi-schema/data-array-empty.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', [])])] -[] -[] diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err index e69de29..a66c2d6 100644 --- a/tests/qapi-schema/data-array-unknown.err +++ b/tests/qapi-schema/data-array-unknown.err @@ -0,0 +1 @@ +tests/qapi-schema/data-array-unknown.json:2: 'data' for command 'oops' references unknown type 'NoSuchType' diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-array-unknown.exit +++ b/tests/qapi-schema/data-array-unknown.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/data-array-unknown.json index 434fb5f..9c75b96 100644 --- a/tests/qapi-schema/data-array-unknown.json +++ b/tests/qapi-schema/data-array-unknown.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an array for data if it does not contain a known type +# we reject an array for data if it does not contain a known type { 'command': 'oops', 'data': [ 'NoSuchType' ] } diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out index c3bc05e..e69de29 100644 --- a/tests/qapi-schema/data-array-unknown.out +++ b/tests/qapi-schema/data-array-unknown.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])] -[] -[] diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err index e69de29..e1d3ed5 100644 --- a/tests/qapi-schema/data-int.err +++ b/tests/qapi-schema/data-int.err @@ -0,0 +1 @@ +tests/qapi-schema/data-int.json:2: 'data' for command 'oops' cannot reference built-in type 'int' diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-int.exit +++ b/tests/qapi-schema/data-int.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/data-int.json index 37916e0..a334d92 100644 --- a/tests/qapi-schema/data-int.json +++ b/tests/qapi-schema/data-int.json @@ -1,2 +1,2 @@ -# FIXME: we should reject commands where data is not an array or complex type +# we reject commands where data is not an array or complex type { 'command': 'oops', 'data': 'int' } diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out index e589a4f..e69de29 100644 --- a/tests/qapi-schema/data-int.out +++ b/tests/qapi-schema/data-int.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', 'int')])] -[] -[] diff --git a/tests/qapi-schema/data-member-array-bad.err b/tests/qapi-schema/data-member-array-bad.err index e69de29..ac45442 100644 --- a/tests/qapi-schema/data-member-array-bad.err +++ b/tests/qapi-schema/data-member-array-bad.err @@ -0,0 +1 @@ +tests/qapi-schema/data-member-array-bad.json:2: member 'member' of 'data' for command 'oops': array type must contain single type name diff --git a/tests/qapi-schema/data-member-array-bad.exit b/tests/qapi-schema/data-member-array-bad.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-member-array-bad.exit +++ b/tests/qapi-schema/data-member-array-bad.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-member-array-bad.json b/tests/qapi-schema/data-member-array-bad.json index c954af1..b2ff144 100644 --- a/tests/qapi-schema/data-member-array-bad.json +++ b/tests/qapi-schema/data-member-array-bad.json @@ -1,2 +1,2 @@ -# FIXME: we should reject data if it does not contain a valid array type +# we reject data if it does not contain a valid array type { 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } } diff --git a/tests/qapi-schema/data-member-array-bad.out b/tests/qapi-schema/data-member-array-bad.out index 0e00c41..e69de29 100644 --- a/tests/qapi-schema/data-member-array-bad.out +++ b/tests/qapi-schema/data-member-array-bad.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', [OrderedDict([('nested', 'str')])])]))])] -[] -[] diff --git a/tests/qapi-schema/data-member-unknown.err b/tests/qapi-schema/data-member-unknown.err index e69de29..89dce36 100644 --- a/tests/qapi-schema/data-member-unknown.err +++ b/tests/qapi-schema/data-member-unknown.err @@ -0,0 +1 @@ +tests/qapi-schema/data-member-unknown.json:2: member 'member' of 'data' for command 'oops' references unknown type 'NoSuchType' diff --git a/tests/qapi-schema/data-member-unknown.exit b/tests/qapi-schema/data-member-unknown.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-member-unknown.exit +++ b/tests/qapi-schema/data-member-unknown.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-member-unknown.json b/tests/qapi-schema/data-member-unknown.json index 40e6252..342a41e 100644 --- a/tests/qapi-schema/data-member-unknown.json +++ b/tests/qapi-schema/data-member-unknown.json @@ -1,2 +1,2 @@ -# FIXME: we should reject data if it does not contain a known type +# we reject data if it does not contain a known type { 'command': 'oops', 'data': { 'member': 'NoSuchType' } } diff --git a/tests/qapi-schema/data-member-unknown.out b/tests/qapi-schema/data-member-unknown.out index 36252a5..e69de29 100644 --- a/tests/qapi-schema/data-member-unknown.out +++ b/tests/qapi-schema/data-member-unknown.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 'NoSuchType')]))])] -[] -[] diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err index e69de29..d1a6086 100644 --- a/tests/qapi-schema/data-unknown.err +++ b/tests/qapi-schema/data-unknown.err @@ -0,0 +1 @@ +tests/qapi-schema/data-unknown.json:2: 'data' for command 'oops' references unknown type 'NoSuchType' diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/data-unknown.exit +++ b/tests/qapi-schema/data-unknown.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/data-unknown.json b/tests/qapi-schema/data-unknown.json index 776bd34..32aba43 100644 --- a/tests/qapi-schema/data-unknown.json +++ b/tests/qapi-schema/data-unknown.json @@ -1,2 +1,2 @@ -# FIXME: we should reject data if it does not contain a known type +# we reject data if it does not contain a known type { 'command': 'oops', 'data': 'NoSuchType' } diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out index 2c60726..e69de29 100644 --- a/tests/qapi-schema/data-unknown.out +++ b/tests/qapi-schema/data-unknown.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])] -[] -[] diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err index e69de29..138095c 100644 --- a/tests/qapi-schema/returns-array-bad.err +++ b/tests/qapi-schema/returns-array-bad.err @@ -0,0 +1 @@ +tests/qapi-schema/returns-array-bad.json:2: 'returns' for command 'oops': array type must contain single type name diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/returns-array-bad.exit +++ b/tests/qapi-schema/returns-array-bad.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/returns-array-bad.json b/tests/qapi-schema/returns-array-bad.json index 14882c1..09b0b1f 100644 --- a/tests/qapi-schema/returns-array-bad.json +++ b/tests/qapi-schema/returns-array-bad.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an array return that is not a single type +# we reject an array return that is not a single type { 'command': 'oops', 'returns': [ 'str', 'str' ] } diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out index eccad55..e69de29 100644 --- a/tests/qapi-schema/returns-array-bad.out +++ b/tests/qapi-schema/returns-array-bad.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('returns', ['str', 'str'])])] -[] -[] diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err index e69de29..962f27c 100644 --- a/tests/qapi-schema/returns-unknown.err +++ b/tests/qapi-schema/returns-unknown.err @@ -0,0 +1 @@ +tests/qapi-schema/returns-unknown.json:2: 'returns' for command 'oops' references unknown type 'NoSuchType' diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/returns-unknown.exit +++ b/tests/qapi-schema/returns-unknown.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/returns-unknown.json b/tests/qapi-schema/returns-unknown.json index 61f20eb..25bd498 100644 --- a/tests/qapi-schema/returns-unknown.json +++ b/tests/qapi-schema/returns-unknown.json @@ -1,2 +1,2 @@ -# FIXME: we should reject returns if it does not contain a known type +# we reject returns if it does not contain a known type { 'command': 'oops', 'returns': 'NoSuchType' } diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out index a482c83..e69de29 100644 --- a/tests/qapi-schema/returns-unknown.out +++ b/tests/qapi-schema/returns-unknown.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])] -[] -[]
Now that we know every expression is valid with regards to its keys, we can add further tests that those keys refer to valid types. With this patch, all references to a type (the 'data': of command, type, union, and event, and the 'returns': of command) must resolve to a builtin or another type declared by the current qapi parse; this includes recursing into each member of a data dictionary. Dealing with '**' and nested sub-structs will be done in later patches. Update the testsuite to match improved output. Signed-off-by: Eric Blake <eblake@redhat.com> --- scripts/qapi.py | 73 ++++++++++++++++++++++++++-- tests/qapi-schema/data-array-empty.err | 1 + tests/qapi-schema/data-array-empty.exit | 2 +- tests/qapi-schema/data-array-empty.json | 2 +- tests/qapi-schema/data-array-empty.out | 3 -- tests/qapi-schema/data-array-unknown.err | 1 + tests/qapi-schema/data-array-unknown.exit | 2 +- tests/qapi-schema/data-array-unknown.json | 2 +- tests/qapi-schema/data-array-unknown.out | 3 -- tests/qapi-schema/data-int.err | 1 + tests/qapi-schema/data-int.exit | 2 +- tests/qapi-schema/data-int.json | 2 +- tests/qapi-schema/data-int.out | 3 -- tests/qapi-schema/data-member-array-bad.err | 1 + tests/qapi-schema/data-member-array-bad.exit | 2 +- tests/qapi-schema/data-member-array-bad.json | 2 +- tests/qapi-schema/data-member-array-bad.out | 3 -- tests/qapi-schema/data-member-unknown.err | 1 + tests/qapi-schema/data-member-unknown.exit | 2 +- tests/qapi-schema/data-member-unknown.json | 2 +- tests/qapi-schema/data-member-unknown.out | 3 -- tests/qapi-schema/data-unknown.err | 1 + tests/qapi-schema/data-unknown.exit | 2 +- tests/qapi-schema/data-unknown.json | 2 +- tests/qapi-schema/data-unknown.out | 3 -- tests/qapi-schema/returns-array-bad.err | 1 + tests/qapi-schema/returns-array-bad.exit | 2 +- tests/qapi-schema/returns-array-bad.json | 2 +- tests/qapi-schema/returns-array-bad.out | 3 -- tests/qapi-schema/returns-unknown.err | 1 + tests/qapi-schema/returns-unknown.exit | 2 +- tests/qapi-schema/returns-unknown.json | 2 +- tests/qapi-schema/returns-unknown.out | 3 -- 33 files changed, 93 insertions(+), 44 deletions(-)