qapi: More rigourous checking of types

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 uses of a type (the 'data':
of command, type, union, alternate, and event; the 'returns':
of command; the 'base': of type and union) must resolve to an
appropriate subset of metatypes  declared by the current qapi
parse; this includes recursing into each member of a data
dictionary.  Dealing with '**' and nested anonymous structs
will be done in later patches.

Update the testsuite to match improved output.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
Eric Blake 2015-05-04 09:05:21 -06:00 committed by Markus Armbruster
parent 0d8b9fb5f2
commit dd883c6f05
52 changed files with 126 additions and 77 deletions

View File

@ -276,6 +276,64 @@ def discriminator_find_enum_define(expr):
return find_enum(discriminator_type) return find_enum(discriminator_type)
def check_type(expr_info, source, value, allow_array = False,
allow_dict = False, allow_metas = []):
global all_names
orig_value = value
if value is None:
return
if value == '**':
return
# Check if array type for value is okay
if isinstance(value, list):
if not allow_array:
raise QAPIExprError(expr_info,
"%s cannot be an array" % source)
if len(value) != 1 or not isinstance(value[0], str):
raise QAPIExprError(expr_info,
"%s: array type must contain single type name"
% source)
value = value[0]
orig_value = "array of %s" %value
# Check if type name for value is okay
if isinstance(value, str):
if not value in all_names:
raise QAPIExprError(expr_info,
"%s uses unknown type '%s'"
% (source, orig_value))
if not all_names[value] in allow_metas:
raise QAPIExprError(expr_info,
"%s cannot use %s type '%s'"
% (source, all_names[value], orig_value))
return
# value is a dictionary, check that each member is okay
if not isinstance(value, 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, arg) in value.items():
check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
allow_array=True, allow_dict=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
def check_command(expr, expr_info):
name = expr['command']
check_type(expr_info, "'data' for command '%s'" % name,
expr.get('data'), allow_dict=True,
allow_metas=['union', 'struct'])
check_type(expr_info, "'returns' for command '%s'" % name,
expr.get('returns'), allow_array=True, allow_dict=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
def check_event(expr, expr_info): def check_event(expr, expr_info):
global events global events
name = expr['event'] name = expr['event']
@ -284,7 +342,9 @@ def check_event(expr, expr_info):
if name.upper() == 'MAX': if name.upper() == 'MAX':
raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created") raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
events.append(name) events.append(name)
check_type(expr_info, "'data' for event '%s'" % name,
expr.get('data'), allow_dict=True,
allow_metas=['union', 'struct'])
if params: if params:
for argname, argentry, optional, structured in parse_args(params): for argname, argentry, optional, structured in parse_args(params):
if structured: if structured:
@ -313,6 +373,7 @@ def check_union(expr, expr_info):
# With no discriminator it is a simple union. # With no discriminator it is a simple union.
if discriminator is None: if discriminator is None:
enum_define = None enum_define = None
allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum']
if base is not None: if base is not None:
raise QAPIExprError(expr_info, raise QAPIExprError(expr_info,
"Simple union '%s' must not have a base" "Simple union '%s' must not have a base"
@ -344,6 +405,7 @@ def check_union(expr, expr_info):
"type '%s'" "type '%s'"
% (discriminator, base)) % (discriminator, base))
enum_define = find_enum(discriminator_type) enum_define = find_enum(discriminator_type)
allow_metas=['struct']
# Do not allow string discriminator # Do not allow string discriminator
if not enum_define: if not enum_define:
raise QAPIExprError(expr_info, raise QAPIExprError(expr_info,
@ -352,6 +414,11 @@ def check_union(expr, expr_info):
# Check every branch # Check every branch
for (key, value) in members.items(): for (key, value) in members.items():
# Each value must name a known type; furthermore, in flat unions,
# branches must be a struct
check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
value, allow_array=True, allow_metas=allow_metas)
# If the discriminator names an enum type, then all members # If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type. # of 'data' must also be members of the enum type.
if enum_define: if enum_define:
@ -387,15 +454,11 @@ def check_alternate(expr, expr_info):
values[c_key] = key values[c_key] = key
# Ensure alternates have no type conflicts. # Ensure alternates have no type conflicts.
if isinstance(value, list): check_type(expr_info, "Member '%s' of alternate '%s'" % (key, name),
raise QAPIExprError(expr_info, value,
"Alternate '%s' member '%s' must " allow_metas=['built-in', 'union', 'struct', 'enum'])
"not be array type" % (name, key))
qtype = find_alternate_member_qtype(value) qtype = find_alternate_member_qtype(value)
if not qtype: assert qtype
raise QAPIExprError(expr_info,
"Alternate '%s' member '%s' has "
"invalid type '%s'" % (name, key, value))
if qtype in types_seen: if qtype in types_seen:
raise QAPIExprError(expr_info, raise QAPIExprError(expr_info,
"Alternate '%s' member '%s' can't " "Alternate '%s' member '%s' can't "
@ -423,6 +486,15 @@ def check_enum(expr, expr_info):
% (name, member, values[key])) % (name, member, values[key]))
values[key] = member 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,
allow_dict=True)
check_type(expr_info, "'base' for type '%s'" % name, expr.get('base'),
allow_metas=['struct'])
def check_exprs(schema): def check_exprs(schema):
for expr_elem in schema.exprs: for expr_elem in schema.exprs:
expr = expr_elem['expr'] expr = expr_elem['expr']
@ -434,8 +506,14 @@ def check_exprs(schema):
check_union(expr, info) check_union(expr, info)
elif expr.has_key('alternate'): elif expr.has_key('alternate'):
check_alternate(expr, info) check_alternate(expr, info)
elif expr.has_key('type'):
check_struct(expr, info)
elif expr.has_key('command'):
check_command(expr, info)
elif expr.has_key('event'): elif expr.has_key('event'):
check_event(expr, info) check_event(expr, info)
else:
assert False, 'unexpected meta type'
def check_keys(expr_elem, meta, required, optional=[]): def check_keys(expr_elem, meta, required, optional=[]):
expr = expr_elem['expr'] expr = expr_elem['expr']

View File

@ -1 +1 @@
tests/qapi-schema/alternate-array.json:5: Alternate 'Alt' member 'two' must not be array type tests/qapi-schema/alternate-array.json:5: Member 'two' of alternate 'Alt' cannot be an array

View File

@ -1 +1 @@
tests/qapi-schema/alternate-nested.json:4: Alternate 'Alt2' member 'nested' has invalid type 'Alt1' tests/qapi-schema/alternate-nested.json:4: Member 'nested' of alternate 'Alt2' cannot use alternate type 'Alt1'

View File

@ -1 +1 @@
tests/qapi-schema/alternate-unknown.json:2: Alternate 'Alt' member 'unknown' has invalid type 'MissingType' tests/qapi-schema/alternate-unknown.json:2: Member 'unknown' of alternate 'Alt' uses unknown type 'MissingType'

View File

@ -0,0 +1 @@
tests/qapi-schema/bad-base.json:3: 'base' for type 'MyType' cannot use union type 'Union'

View File

@ -1 +1 @@
0 1

View File

@ -1,3 +1,3 @@
# FIXME: we should reject a base that is not a struct # we reject a base that is not a struct
{ 'union': 'Union', 'data': { 'a': 'int', 'b': 'str' } } { 'union': 'Union', 'data': { 'a': 'int', 'b': 'str' } }
{ 'type': 'MyType', 'base': 'Union', 'data': { 'c': 'int' } } { 'type': 'MyType', 'base': 'Union', 'data': { 'c': 'int' } }

View File

@ -1,4 +0,0 @@
[OrderedDict([('union', 'Union'), ('data', OrderedDict([('a', 'int'), ('b', 'str')]))]),
OrderedDict([('type', 'MyType'), ('base', 'Union'), ('data', OrderedDict([('c', 'int')]))])]
[{'enum_name': 'UnionKind', 'enum_values': None}]
[OrderedDict([('type', 'MyType'), ('base', 'Union'), ('data', OrderedDict([('c', 'int')]))])]

View File

@ -0,0 +1 @@
tests/qapi-schema/bad-data.json:2: 'data' for command 'oops' cannot be an array

View File

@ -1 +1 @@
0 1

View File

@ -1,2 +1,2 @@
# FIXME: we should ensure 'data' is a dictionary for all but enums # we ensure 'data' is a dictionary for all but enums
{ 'command': 'oops', 'data': [ ] } { 'command': 'oops', 'data': [ ] }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', [])])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/data-array-empty.json:2: Member 'empty' of 'data' for command 'oops': array type must contain single type name

View File

@ -1 +1 @@
0 1

View File

@ -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': { 'empty': [ ] } } { 'command': 'oops', 'data': { 'empty': [ ] } }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', OrderedDict([('empty', [])]))])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/data-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'array of NoSuchType'

View File

@ -1 +1 @@
0 1

View File

@ -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': { 'array': [ 'NoSuchType' ] } } { 'command': 'oops', 'data': { 'array': [ 'NoSuchType' ] } }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', OrderedDict([('array', ['NoSuchType'])]))])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/data-int.json:2: 'data' for command 'oops' cannot use built-in type 'int'

View File

@ -1 +1 @@
0 1

View File

@ -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' } { 'command': 'oops', 'data': 'int' }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', 'int')])]
[]
[]

View File

@ -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

View File

@ -1 +1 @@
0 1

View File

@ -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' } ] } } { 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', [OrderedDict([('nested', 'str')])])]))])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/data-member-unknown.json:2: Member 'member' of 'data' for command 'oops' uses unknown type 'NoSuchType'

View File

@ -1 +1 @@
0 1

View File

@ -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' } } { 'command': 'oops', 'data': { 'member': 'NoSuchType' } }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 'NoSuchType')]))])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/data-unknown.json:2: 'data' for command 'oops' uses unknown type 'NoSuchType'

View File

@ -1 +1 @@
0 1

View File

@ -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' } { 'command': 'oops', 'data': 'NoSuchType' }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/flat-union-int-branch.json:8: Member 'value1' of union 'TestUnion' cannot use built-in type 'int'

View File

@ -1 +1 @@
0 1

View File

@ -1,4 +1,4 @@
# FIXME: we should require flat union branches to be a complex type # we require flat union branches to be a complex type
{ 'enum': 'TestEnum', { 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] } 'data': [ 'value1', 'value2' ] }
{ 'type': 'Base', { 'type': 'Base',

View File

@ -1,7 +0,0 @@
[OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
OrderedDict([('type', 'Base'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'TestUnion'), ('base', 'Base'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'int'), ('value2', 'TestTypeB')]))])]
[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}]
[OrderedDict([('type', 'Base'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]

View File

@ -0,0 +1 @@
tests/qapi-schema/returns-array-bad.json:2: 'returns' for command 'oops': array type must contain single type name

View File

@ -1 +1 @@
0 1

View File

@ -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' ] } { 'command': 'oops', 'returns': [ 'str', 'str' ] }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('returns', ['str', 'str'])])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/returns-unknown.json:2: 'returns' for command 'oops' uses unknown type 'NoSuchType'

View File

@ -1 +1 @@
0 1

View File

@ -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' } { 'command': 'oops', 'returns': 'NoSuchType' }

View File

@ -1,3 +0,0 @@
[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])]
[]
[]

View File

@ -0,0 +1 @@
tests/qapi-schema/union-unknown.json:2: Member 'unknown' of union 'Union' uses unknown type 'MissingType'

View File

@ -1 +1 @@
0 1

View File

@ -1,3 +1,3 @@
# FIXME: we should reject a union with unknown type in branch # we reject a union with unknown type in branch
{ 'union': 'Union', { 'union': 'Union',
'data': { 'unknown': 'MissingType' } } 'data': { 'unknown': 'MissingType' } }

View File

@ -1,3 +0,0 @@
[OrderedDict([('union', 'Union'), ('data', OrderedDict([('unknown', 'MissingType')]))])]
[{'enum_name': 'UnionKind', 'enum_values': None}]
[]