diff mbox series

[for-4.0,v7,13/27] qapi: add a dictionary form for TYPE

Message ID 20181208111606.8505-14-marcandre.lureau@redhat.com
State New
Headers show
Series Hi, | expand

Commit Message

Marc-André Lureau Dec. 8, 2018, 11:15 a.m. UTC
Wherever a struct/union/alternate/command/event member with NAME: TYPE
form is accepted, desugar it to a NAME: { 'type': TYPE } form.

This will allow to add new member details, such as 'if' in the
following patch to introduce conditionals, or 'default' for default
values etc.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py                        | 71 ++++++++++++++-----
 tests/Makefile.include                        |  6 ++
 tests/qapi-schema/alternate-invalid-dict.err  |  1 +
 tests/qapi-schema/alternate-invalid-dict.exit |  1 +
 tests/qapi-schema/alternate-invalid-dict.json |  4 ++
 tests/qapi-schema/alternate-invalid-dict.out  |  0
 .../qapi-schema/event-member-invalid-dict.err |  1 +
 .../event-member-invalid-dict.exit            |  1 +
 .../event-member-invalid-dict.json            |  2 +
 .../qapi-schema/event-member-invalid-dict.out |  0
 tests/qapi-schema/event-nest-struct.json      |  2 +-
 .../flat-union-inline-invalid-dict.err        |  1 +
 .../flat-union-inline-invalid-dict.exit       |  1 +
 .../flat-union-inline-invalid-dict.json       | 11 +++
 .../flat-union-inline-invalid-dict.out        |  0
 tests/qapi-schema/flat-union-inline.json      |  2 +-
 .../nested-struct-data-invalid-dict.err       |  1 +
 .../nested-struct-data-invalid-dict.exit      |  1 +
 .../nested-struct-data-invalid-dict.json      |  3 +
 .../nested-struct-data-invalid-dict.out       |  0
 tests/qapi-schema/nested-struct-data.json     |  2 +-
 tests/qapi-schema/qapi-schema-test.json       | 10 +--
 .../struct-member-invalid-dict.err            |  1 +
 .../struct-member-invalid-dict.exit           |  1 +
 .../struct-member-invalid-dict.json           |  3 +
 .../struct-member-invalid-dict.out            |  0
 .../qapi-schema/union-branch-invalid-dict.err |  1 +
 .../union-branch-invalid-dict.exit            |  1 +
 .../union-branch-invalid-dict.json            |  4 ++
 .../qapi-schema/union-branch-invalid-dict.out |  0
 30 files changed, 106 insertions(+), 26 deletions(-)
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.err
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.exit
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.json
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.out
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.exit
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.json
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.out
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.err
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.exit
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.json
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.out
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out

diff --git a/tests/qapi-schema/union-branch-invalid-dict.out b/tests/qapi-schema/union-branch-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2

Comments

Markus Armbruster Dec. 10, 2018, 5:24 p.m. UTC | #1
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Wherever a struct/union/alternate/command/event member with NAME: TYPE
> form is accepted, desugar it to a NAME: { 'type': TYPE } form.
>
> This will allow to add new member details, such as 'if' in the
> following patch to introduce conditionals, or 'default' for default
> values etc.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/common.py                        | 71 ++++++++++++++-----
>  tests/Makefile.include                        |  6 ++
>  tests/qapi-schema/alternate-invalid-dict.err  |  1 +
>  tests/qapi-schema/alternate-invalid-dict.exit |  1 +
>  tests/qapi-schema/alternate-invalid-dict.json |  4 ++
>  tests/qapi-schema/alternate-invalid-dict.out  |  0
>  .../qapi-schema/event-member-invalid-dict.err |  1 +
>  .../event-member-invalid-dict.exit            |  1 +
>  .../event-member-invalid-dict.json            |  2 +
>  .../qapi-schema/event-member-invalid-dict.out |  0
>  tests/qapi-schema/event-nest-struct.json      |  2 +-
>  .../flat-union-inline-invalid-dict.err        |  1 +
>  .../flat-union-inline-invalid-dict.exit       |  1 +
>  .../flat-union-inline-invalid-dict.json       | 11 +++
>  .../flat-union-inline-invalid-dict.out        |  0
>  tests/qapi-schema/flat-union-inline.json      |  2 +-
>  .../nested-struct-data-invalid-dict.err       |  1 +
>  .../nested-struct-data-invalid-dict.exit      |  1 +
>  .../nested-struct-data-invalid-dict.json      |  3 +
>  .../nested-struct-data-invalid-dict.out       |  0
>  tests/qapi-schema/nested-struct-data.json     |  2 +-
>  tests/qapi-schema/qapi-schema-test.json       | 10 +--
>  .../struct-member-invalid-dict.err            |  1 +
>  .../struct-member-invalid-dict.exit           |  1 +
>  .../struct-member-invalid-dict.json           |  3 +
>  .../struct-member-invalid-dict.out            |  0
>  .../qapi-schema/union-branch-invalid-dict.err |  1 +
>  .../union-branch-invalid-dict.exit            |  1 +
>  .../union-branch-invalid-dict.json            |  4 ++
>  .../qapi-schema/union-branch-invalid-dict.out |  0
>  30 files changed, 106 insertions(+), 26 deletions(-)
>  create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
>  create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
>  create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
>  create mode 100644 tests/qapi-schema/event-member-invalid-dict.err
>  create mode 100644 tests/qapi-schema/event-member-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/event-member-invalid-dict.json
>  create mode 100644 tests/qapi-schema/event-member-invalid-dict.out
>  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
>  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.json
>  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.out
>  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.err
>  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.json
>  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.out
>  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
>  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
>  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
>  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
>  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
>  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
>  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 95e55b3f44..4b3ba53dc7 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -588,11 +588,13 @@ def discriminator_find_enum_define(expr):
>      if not base_members:
>          return None
>  
> -    discriminator_type = base_members.get(discriminator)
> -    if not discriminator_type:
> +    discriminator_value = base_members.get(discriminator)
> +    if not discriminator_value:
>          return None
>  
> -    return enum_types.get(discriminator_type)
> +    if isinstance(discriminator_value, dict):
> +        discriminator_value = discriminator_value['type']
> +    return enum_types.get(discriminator_value)

As in PATCH 08, this is slightly more complicated than v6 because @expr
isn't normalized.  I can't say offhand whether normalizing in the place
I suggested in my review of v6 would avoid this complication.

>  
>  
>  # Names must be letters, numbers, -, and _.  They must start with letter,
> @@ -660,6 +662,15 @@ def check_if(expr, info):
>          check_if_str(ifcond, info)
>  
>  
> +def normalize_members(expr, field):
> +    members = expr.get(field)
> +    if isinstance(members, OrderedDict):
> +        for key, arg in members.items():
> +            if isinstance(arg, dict):
> +                continue
> +            members[key] = {'type': arg}
> +
> +
>  def check_type(info, source, value, allow_array=False,
>                 allow_implicit=False, allow_optional=False,
>                 allow_metas=[]):
> @@ -704,8 +715,14 @@ def check_type(info, source, value, allow_array=False,
>                                 % (source, key))
>          # Todo: allow dictionaries to represent default values of
>          # an optional argument.
> -        check_type(info, "Member '%s' of %s" % (key, source), arg,
> -                   allow_array=True,
> +        if isinstance(arg, dict):
> +            check_known_keys(info, "member '%s' of %s" % (key, source),
> +                             arg, ['type'], [])
> +            typ = arg['type']
> +        else:
> +            typ = arg
> +        check_type(info, "Member '%s' of %s" % (key, source),
> +                   typ, allow_array=True,
>                     allow_metas=['built-in', 'union', 'alternate', 'struct',
>                                  'enum'])
>  

Slightly more complicated than v6, this time because members of the
implicit struct type aren't normalized, yet.

> @@ -776,13 +793,15 @@ def check_union(expr, info):
>          # member of the base struct.
>          check_name(info, "Discriminator of flat union '%s'" % name,
>                     discriminator)
> -        discriminator_type = base_members.get(discriminator)
> -        if not discriminator_type:
> +        discriminator_value = base_members.get(discriminator)
> +        if not discriminator_value:
>              raise QAPISemError(info,
>                                 "Discriminator '%s' is not a member of base "
>                                 "struct '%s'"
>                                 % (discriminator, base))
> -        enum_define = enum_types.get(discriminator_type)
> +        if isinstance(discriminator_value, dict):
> +            discriminator_value = discriminator_value['type']
> +        enum_define = enum_types.get(discriminator_value)
>          allow_metas = ['struct']
>          # Do not allow string discriminator
>          if not enum_define:
> @@ -795,10 +814,16 @@ def check_union(expr, info):
>          raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
>      for (key, value) in members.items():
>          check_name(info, "Member of union '%s'" % name, key)
> +        if isinstance(value, dict):
> +            check_known_keys(info, "member '%s' of union '%s'" % (key, name),
> +                             value, ['type'], [])
> +            typ = value['type']
> +        else:
> +            typ = value

Likewise.

>  
>          # Each value must name a known type
>          check_type(info, "Member '%s' of union '%s'" % (key, name),
> -                   value, allow_array=not base, allow_metas=allow_metas)
> +                   typ, allow_array=not base, allow_metas=allow_metas)
>  
>          # If the discriminator names an enum type, then all members
>          # of 'data' must also be members of the enum type.
> @@ -822,18 +847,24 @@ def check_alternate(expr, info):
>                             "in 'data'" % name)
>      for (key, value) in members.items():
>          check_name(info, "Member of alternate '%s'" % name, key)
> +        if isinstance(value, dict):
> +            check_known_keys(info,
> +                             "member '%s' of alternate '%s'" % (key, name),
> +                             value, ['type'], [])
> +            typ = value['type']
> +        else:
> +            typ = value

Likewise.

>  
>          # Ensure alternates have no type conflicts.
> -        check_type(info, "Member '%s' of alternate '%s'" % (key, name),
> -                   value,
> +        check_type(info, "Member '%s' of alternate '%s'" % (key, name), typ,
>                     allow_metas=['built-in', 'union', 'struct', 'enum'])
> -        qtype = find_alternate_member_qtype(value)
> +        qtype = find_alternate_member_qtype(typ)
>          if not qtype:
>              raise QAPISemError(info, "Alternate '%s' member '%s' cannot use "
> -                               "type '%s'" % (name, key, value))
> +                               "type '%s'" % (name, key, typ))
>          conflicting = set([qtype])
>          if qtype == 'QTYPE_QSTRING':
> -            enum_expr = enum_types.get(value)
> +            enum_expr = enum_types.get(typ)
>              if enum_expr:
>                  for v in enum_get_names(enum_expr):
>                      if v in ['on', 'off']:
> @@ -1035,6 +1066,10 @@ def normalize_exprs(exprs):
>          expr = expr_elem['expr']
>          if 'enum' in expr:
>              normalize_enum(expr)
> +        elif 'union' in expr:
> +            normalize_members(expr, 'base')
> +        if {'union', 'alternate', 'struct', 'command', 'event'} & set(expr):
> +            normalize_members(expr, 'data')
>  
>      return exprs
>  
> @@ -1738,7 +1773,7 @@ class QAPISchema(object):
>          return QAPISchemaObjectTypeMember(name, typ, optional)
>  
>      def _make_members(self, data, info):
> -        return [self._make_member(key, value, info)
> +        return [self._make_member(key, value['type'], info)
>                  for (key, value) in data.items()]
>  
>      def _def_struct_type(self, expr, info, doc):
> @@ -1774,11 +1809,11 @@ class QAPISchema(object):
>                  name, info, doc, ifcond,
>                  'base', self._make_members(base, info))
>          if tag_name:
> -            variants = [self._make_variant(key, value)
> +            variants = [self._make_variant(key, value['type'])
>                          for (key, value) in data.items()]
>              members = []
>          else:
> -            variants = [self._make_simple_variant(key, value, info)
> +            variants = [self._make_simple_variant(key, value['type'], info)
>                          for (key, value) in data.items()]
>              typ = self._make_implicit_enum_type(name, info, ifcond,
>                                                  [v.name for v in variants])
> @@ -1794,7 +1829,7 @@ class QAPISchema(object):
>          name = expr['alternate']
>          data = expr['data']
>          ifcond = expr.get('if')
> -        variants = [self._make_variant(key, value)
> +        variants = [self._make_variant(key, value['type'])
>                      for (key, value) in data.items()]
>          tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
>          self._def_entity(
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 3c9eea27fd..ea5d1e8787 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -318,6 +318,7 @@ qapi-schema += alternate-conflict-string.json
>  qapi-schema += alternate-conflict-bool-string.json
>  qapi-schema += alternate-conflict-num-string.json
>  qapi-schema += alternate-empty.json
> +qapi-schema += alternate-invalid-dict.json
>  qapi-schema += alternate-nested.json
>  qapi-schema += alternate-unknown.json
>  qapi-schema += args-alternate.json
> @@ -394,6 +395,7 @@ qapi-schema += escape-too-big.json
>  qapi-schema += escape-too-short.json
>  qapi-schema += event-boxed-empty.json
>  qapi-schema += event-case.json
> +qapi-schema += event-member-invalid-dict.json
>  qapi-schema += event-nest-struct.json
>  qapi-schema += flat-union-array-branch.json
>  qapi-schema += flat-union-bad-base.json
> @@ -403,6 +405,7 @@ qapi-schema += flat-union-base-union.json
>  qapi-schema += flat-union-clash-member.json
>  qapi-schema += flat-union-empty.json
>  qapi-schema += flat-union-inline.json
> +qapi-schema += flat-union-inline-invalid-dict.json
>  qapi-schema += flat-union-int-branch.json
>  qapi-schema += flat-union-invalid-branch-key.json
>  qapi-schema += flat-union-invalid-discriminator.json
> @@ -430,6 +433,7 @@ qapi-schema += missing-comma-list.json
>  qapi-schema += missing-comma-object.json
>  qapi-schema += missing-type.json
>  qapi-schema += nested-struct-data.json
> +qapi-schema += nested-struct-data-invalid-dict.json
>  qapi-schema += non-objects.json
>  qapi-schema += oob-test.json
>  qapi-schema += allow-preconfig-test.json
> @@ -460,6 +464,7 @@ qapi-schema += returns-whitelist.json
>  qapi-schema += struct-base-clash-deep.json
>  qapi-schema += struct-base-clash.json
>  qapi-schema += struct-data-invalid.json
> +qapi-schema += struct-member-invalid-dict.json
>  qapi-schema += struct-member-invalid.json
>  qapi-schema += trailing-comma-list.json
>  qapi-schema += trailing-comma-object.json
> @@ -471,6 +476,7 @@ qapi-schema += unicode-str.json
>  qapi-schema += union-base-empty.json
>  qapi-schema += union-base-no-discriminator.json
>  qapi-schema += union-branch-case.json
> +qapi-schema += union-branch-invalid-dict.json
>  qapi-schema += union-clash-branches.json
>  qapi-schema += union-empty.json
>  qapi-schema += union-invalid-base.json
> diff --git a/tests/qapi-schema/alternate-invalid-dict.err b/tests/qapi-schema/alternate-invalid-dict.err
> new file mode 100644
> index 0000000000..631d46628e
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/alternate-invalid-dict.json:2: Key 'type' is missing from member 'two' of alternate 'Alt'
> diff --git a/tests/qapi-schema/alternate-invalid-dict.exit b/tests/qapi-schema/alternate-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/alternate-invalid-dict.json b/tests/qapi-schema/alternate-invalid-dict.json
> new file mode 100644
> index 0000000000..8e0b2ac287
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-invalid-dict.json
> @@ -0,0 +1,4 @@
> +# exploded member form must have a 'type'
> +{ 'alternate': 'Alt',
> +  'data': { 'one': 'str',
> +            'two': { 'if': 'foo' } } }
> diff --git a/tests/qapi-schema/alternate-invalid-dict.out b/tests/qapi-schema/alternate-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err
> new file mode 100644
> index 0000000000..1a57fa29b0
> --- /dev/null
> +++ b/tests/qapi-schema/event-member-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/event-member-invalid-dict.json:1: Key 'type' is missing from member 'a' of 'data' for event 'EVENT_A'
> diff --git a/tests/qapi-schema/event-member-invalid-dict.exit b/tests/qapi-schema/event-member-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/event-member-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json
> new file mode 100644
> index 0000000000..ee6f3ecb6f
> --- /dev/null
> +++ b/tests/qapi-schema/event-member-invalid-dict.json
> @@ -0,0 +1,2 @@
> +{ 'event': 'EVENT_A',
> +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> diff --git a/tests/qapi-schema/event-member-invalid-dict.out b/tests/qapi-schema/event-member-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/event-nest-struct.json b/tests/qapi-schema/event-nest-struct.json
> index ee6f3ecb6f..355ddaeff1 100644
> --- a/tests/qapi-schema/event-nest-struct.json
> +++ b/tests/qapi-schema/event-nest-struct.json
> @@ -1,2 +1,2 @@
>  { 'event': 'EVENT_A',
> -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> +  'data': { 'a' : { 'type' : { 'integer': 'int' } }, 'b' : 'str' } }
> diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.err b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> new file mode 100644
> index 0000000000..9c4c45b7f0
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/flat-union-inline-invalid-dict.json:7: Key 'type' is missing from member 'value1' of union 'TestUnion'
> diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.exit b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> new file mode 100644
> index 0000000000..62c7cda617
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> @@ -0,0 +1,11 @@
> +# we require branches to be a struct name
> +# TODO: should we allow anonymous inline branch types?
> +{ 'enum': 'TestEnum',
> +  'data': [ 'value1', 'value2' ] }
> +{ 'struct': 'Base',
> +  'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
> +{ 'union': 'TestUnion',
> +  'base': 'Base',
> +  'discriminator': 'enum1',
> +  'data': { 'value1': { 'string': 'str' },
> +            'value2': { 'integer': 'int' } } }
> diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.out b/tests/qapi-schema/flat-union-inline-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
> index 62c7cda617..a9b3ce3f0d 100644
> --- a/tests/qapi-schema/flat-union-inline.json
> +++ b/tests/qapi-schema/flat-union-inline.json
> @@ -7,5 +7,5 @@
>  { 'union': 'TestUnion',
>    'base': 'Base',
>    'discriminator': 'enum1',
> -  'data': { 'value1': { 'string': 'str' },
> +  'data': { 'value1': { 'type': {} },
>              'value2': { 'integer': 'int' } } }
> diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> new file mode 100644
> index 0000000000..5bd364e8d9
> --- /dev/null
> +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/nested-struct-data-invalid-dict.json:2: Key 'type' is missing from member 'a' of 'data' for command 'foo'
> diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.exit b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> new file mode 100644
> index 0000000000..efbe773ded
> --- /dev/null
> +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> @@ -0,0 +1,3 @@
> +# inline subtypes collide with our desired future use of defaults
> +{ 'command': 'foo',
> +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.out b/tests/qapi-schema/nested-struct-data-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
> index efbe773ded..5b8a40cca3 100644
> --- a/tests/qapi-schema/nested-struct-data.json
> +++ b/tests/qapi-schema/nested-struct-data.json
> @@ -1,3 +1,3 @@
>  # inline subtypes collide with our desired future use of defaults
>  { 'command': 'foo',
> -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> +  'data': { 'a' : { 'type': {} }, 'b' : 'str' } }
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 22d9044a89..94570154c9 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -11,7 +11,7 @@
>          'guest-sync' ] } }
>  
>  { 'struct': 'TestStruct',
> -  'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
> +  'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' } }
>  
>  # for testing enums
>  { 'struct': 'NestedEnumsOne',
> @@ -77,7 +77,7 @@
>  { 'union': 'UserDefFlatUnion',
>    'base': 'UserDefUnionBase',   # intentional forward reference
>    'discriminator': 'enum1',
> -  'data': { 'value1' : 'UserDefA',
> +  'data': { 'value1' : {'type': 'UserDefA'},
>              'value2' : 'UserDefB',
>              'value3' : 'UserDefB'
>              # 'value4' defaults to empty
> @@ -98,7 +98,7 @@
>  { 'struct': 'WrapAlternate',
>    'data': { 'alt': 'UserDefAlternate' } }
>  { 'alternate': 'UserDefAlternate',
> -  'data': { 'udfu': 'UserDefFlatUnion', 'e': 'EnumOne', 'i': 'int',
> +  'data': { 'udfu': {'type': 'UserDefFlatUnion'}, 'e': 'EnumOne', 'i': 'int',
>              'n': 'null' } }
>  
>  { 'struct': 'UserDefC',
> @@ -134,7 +134,7 @@
>  { 'command': 'user_def_cmd', 'data': {} }
>  { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
>  { 'command': 'user_def_cmd2',
> -  'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
> +  'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'},
>    'returns': 'UserDefTwo' }
>  
>  { 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
> @@ -166,7 +166,7 @@
>  
>  # testing event
>  { 'struct': 'EventStructOne',
> -  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
> +  'data': { 'struct1': {'type': 'UserDefOne'}, 'string': 'str', '*enum2': 'EnumOne' } }
>  
>  { 'event': 'EVENT_A' }
>  { 'event': 'EVENT_B',
> diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err
> new file mode 100644
> index 0000000000..6a765bc668
> --- /dev/null
> +++ b/tests/qapi-schema/struct-member-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/struct-member-invalid-dict.json:2: Key 'type' is missing from member '*a' of 'data' for struct 'foo'
> diff --git a/tests/qapi-schema/struct-member-invalid-dict.exit b/tests/qapi-schema/struct-member-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/struct-member-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json
> new file mode 100644
> index 0000000000..9fe0d455a9
> --- /dev/null
> +++ b/tests/qapi-schema/struct-member-invalid-dict.json
> @@ -0,0 +1,3 @@
> +# Long form of member must have a value member 'type'
> +{ 'struct': 'foo',
> +  'data': { '*a': { 'case': 'foo' } } }
> diff --git a/tests/qapi-schema/struct-member-invalid-dict.out b/tests/qapi-schema/struct-member-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/union-branch-invalid-dict.err b/tests/qapi-schema/union-branch-invalid-dict.err
> new file mode 100644
> index 0000000000..89f9b36791
> --- /dev/null
> +++ b/tests/qapi-schema/union-branch-invalid-dict.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/union-branch-invalid-dict.json:2: Key 'type' is missing from member 'integer' of union 'UnionInvalidBranch'
> diff --git a/tests/qapi-schema/union-branch-invalid-dict.exit b/tests/qapi-schema/union-branch-invalid-dict.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/union-branch-invalid-dict.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/union-branch-invalid-dict.json b/tests/qapi-schema/union-branch-invalid-dict.json
> new file mode 100644
> index 0000000000..9778598dbd
> --- /dev/null
> +++ b/tests/qapi-schema/union-branch-invalid-dict.json
> @@ -0,0 +1,4 @@
> +# Long form of member must have a value member 'type'
> +{ 'union': 'UnionInvalidBranch',
> +  'data': { 'integer': { 'if': 'foo'},
> +            's8': 'int8' } }
> diff --git a/tests/qapi-schema/union-branch-invalid-dict.out b/tests/qapi-schema/union-branch-invalid-dict.out
> new file mode 100644
> index 0000000000..e69de29bb2

Looks good, but I'd like to investigate whether my suggested placement
of the normalization would simplify things before I give my R-by.
Marc-André Lureau Dec. 11, 2018, 12:11 p.m. UTC | #2
Hi

On Mon, Dec 10, 2018 at 9:26 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Wherever a struct/union/alternate/command/event member with NAME: TYPE
> > form is accepted, desugar it to a NAME: { 'type': TYPE } form.
> >
> > This will allow to add new member details, such as 'if' in the
> > following patch to introduce conditionals, or 'default' for default
> > values etc.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/qapi/common.py                        | 71 ++++++++++++++-----
> >  tests/Makefile.include                        |  6 ++
> >  tests/qapi-schema/alternate-invalid-dict.err  |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.exit |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.json |  4 ++
> >  tests/qapi-schema/alternate-invalid-dict.out  |  0
> >  .../qapi-schema/event-member-invalid-dict.err |  1 +
> >  .../event-member-invalid-dict.exit            |  1 +
> >  .../event-member-invalid-dict.json            |  2 +
> >  .../qapi-schema/event-member-invalid-dict.out |  0
> >  tests/qapi-schema/event-nest-struct.json      |  2 +-
> >  .../flat-union-inline-invalid-dict.err        |  1 +
> >  .../flat-union-inline-invalid-dict.exit       |  1 +
> >  .../flat-union-inline-invalid-dict.json       | 11 +++
> >  .../flat-union-inline-invalid-dict.out        |  0
> >  tests/qapi-schema/flat-union-inline.json      |  2 +-
> >  .../nested-struct-data-invalid-dict.err       |  1 +
> >  .../nested-struct-data-invalid-dict.exit      |  1 +
> >  .../nested-struct-data-invalid-dict.json      |  3 +
> >  .../nested-struct-data-invalid-dict.out       |  0
> >  tests/qapi-schema/nested-struct-data.json     |  2 +-
> >  tests/qapi-schema/qapi-schema-test.json       | 10 +--
> >  .../struct-member-invalid-dict.err            |  1 +
> >  .../struct-member-invalid-dict.exit           |  1 +
> >  .../struct-member-invalid-dict.json           |  3 +
> >  .../struct-member-invalid-dict.out            |  0
> >  .../qapi-schema/union-branch-invalid-dict.err |  1 +
> >  .../union-branch-invalid-dict.exit            |  1 +
> >  .../union-branch-invalid-dict.json            |  4 ++
> >  .../qapi-schema/union-branch-invalid-dict.out |  0
> >  30 files changed, 106 insertions(+), 26 deletions(-)
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 95e55b3f44..4b3ba53dc7 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -588,11 +588,13 @@ def discriminator_find_enum_define(expr):
> >      if not base_members:
> >          return None
> >
> > -    discriminator_type = base_members.get(discriminator)
> > -    if not discriminator_type:
> > +    discriminator_value = base_members.get(discriminator)
> > +    if not discriminator_value:
> >          return None
> >
> > -    return enum_types.get(discriminator_type)
> > +    if isinstance(discriminator_value, dict):
> > +        discriminator_value = discriminator_value['type']
> > +    return enum_types.get(discriminator_value)
>
> As in PATCH 08, this is slightly more complicated than v6 because @expr
> isn't normalized.  I can't say offhand whether normalizing in the place
> I suggested in my review of v6 would avoid this complication.
>
> >
> >
> >  # Names must be letters, numbers, -, and _.  They must start with letter,
> > @@ -660,6 +662,15 @@ def check_if(expr, info):
> >          check_if_str(ifcond, info)
> >
> >
> > +def normalize_members(expr, field):
> > +    members = expr.get(field)
> > +    if isinstance(members, OrderedDict):
> > +        for key, arg in members.items():
> > +            if isinstance(arg, dict):
> > +                continue
> > +            members[key] = {'type': arg}
> > +
> > +
> >  def check_type(info, source, value, allow_array=False,
> >                 allow_implicit=False, allow_optional=False,
> >                 allow_metas=[]):
> > @@ -704,8 +715,14 @@ def check_type(info, source, value, allow_array=False,
> >                                 % (source, key))
> >          # Todo: allow dictionaries to represent default values of
> >          # an optional argument.
> > -        check_type(info, "Member '%s' of %s" % (key, source), arg,
> > -                   allow_array=True,
> > +        if isinstance(arg, dict):
> > +            check_known_keys(info, "member '%s' of %s" % (key, source),
> > +                             arg, ['type'], [])
> > +            typ = arg['type']
> > +        else:
> > +            typ = arg
> > +        check_type(info, "Member '%s' of %s" % (key, source),
> > +                   typ, allow_array=True,
> >                     allow_metas=['built-in', 'union', 'alternate', 'struct',
> >                                  'enum'])
> >
>
> Slightly more complicated than v6, this time because members of the
> implicit struct type aren't normalized, yet.
>
> > @@ -776,13 +793,15 @@ def check_union(expr, info):
> >          # member of the base struct.
> >          check_name(info, "Discriminator of flat union '%s'" % name,
> >                     discriminator)
> > -        discriminator_type = base_members.get(discriminator)
> > -        if not discriminator_type:
> > +        discriminator_value = base_members.get(discriminator)
> > +        if not discriminator_value:
> >              raise QAPISemError(info,
> >                                 "Discriminator '%s' is not a member of base "
> >                                 "struct '%s'"
> >                                 % (discriminator, base))
> > -        enum_define = enum_types.get(discriminator_type)
> > +        if isinstance(discriminator_value, dict):
> > +            discriminator_value = discriminator_value['type']
> > +        enum_define = enum_types.get(discriminator_value)
> >          allow_metas = ['struct']
> >          # Do not allow string discriminator
> >          if not enum_define:
> > @@ -795,10 +814,16 @@ def check_union(expr, info):
> >          raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
> >      for (key, value) in members.items():
> >          check_name(info, "Member of union '%s'" % name, key)
> > +        if isinstance(value, dict):
> > +            check_known_keys(info, "member '%s' of union '%s'" % (key, name),
> > +                             value, ['type'], [])
> > +            typ = value['type']
> > +        else:
> > +            typ = value
>
> Likewise.
>
> >
> >          # Each value must name a known type
> >          check_type(info, "Member '%s' of union '%s'" % (key, name),
> > -                   value, allow_array=not base, allow_metas=allow_metas)
> > +                   typ, allow_array=not base, allow_metas=allow_metas)
> >
> >          # If the discriminator names an enum type, then all members
> >          # of 'data' must also be members of the enum type.
> > @@ -822,18 +847,24 @@ def check_alternate(expr, info):
> >                             "in 'data'" % name)
> >      for (key, value) in members.items():
> >          check_name(info, "Member of alternate '%s'" % name, key)
> > +        if isinstance(value, dict):
> > +            check_known_keys(info,
> > +                             "member '%s' of alternate '%s'" % (key, name),
> > +                             value, ['type'], [])
> > +            typ = value['type']
> > +        else:
> > +            typ = value
>
> Likewise.
>
> >
> >          # Ensure alternates have no type conflicts.
> > -        check_type(info, "Member '%s' of alternate '%s'" % (key, name),
> > -                   value,
> > +        check_type(info, "Member '%s' of alternate '%s'" % (key, name), typ,
> >                     allow_metas=['built-in', 'union', 'struct', 'enum'])
> > -        qtype = find_alternate_member_qtype(value)
> > +        qtype = find_alternate_member_qtype(typ)
> >          if not qtype:
> >              raise QAPISemError(info, "Alternate '%s' member '%s' cannot use "
> > -                               "type '%s'" % (name, key, value))
> > +                               "type '%s'" % (name, key, typ))
> >          conflicting = set([qtype])
> >          if qtype == 'QTYPE_QSTRING':
> > -            enum_expr = enum_types.get(value)
> > +            enum_expr = enum_types.get(typ)
> >              if enum_expr:
> >                  for v in enum_get_names(enum_expr):
> >                      if v in ['on', 'off']:
> > @@ -1035,6 +1066,10 @@ def normalize_exprs(exprs):
> >          expr = expr_elem['expr']
> >          if 'enum' in expr:
> >              normalize_enum(expr)
> > +        elif 'union' in expr:
> > +            normalize_members(expr, 'base')
> > +        if {'union', 'alternate', 'struct', 'command', 'event'} & set(expr):
> > +            normalize_members(expr, 'data')
> >
> >      return exprs
> >
> > @@ -1738,7 +1773,7 @@ class QAPISchema(object):
> >          return QAPISchemaObjectTypeMember(name, typ, optional)
> >
> >      def _make_members(self, data, info):
> > -        return [self._make_member(key, value, info)
> > +        return [self._make_member(key, value['type'], info)
> >                  for (key, value) in data.items()]
> >
> >      def _def_struct_type(self, expr, info, doc):
> > @@ -1774,11 +1809,11 @@ class QAPISchema(object):
> >                  name, info, doc, ifcond,
> >                  'base', self._make_members(base, info))
> >          if tag_name:
> > -            variants = [self._make_variant(key, value)
> > +            variants = [self._make_variant(key, value['type'])
> >                          for (key, value) in data.items()]
> >              members = []
> >          else:
> > -            variants = [self._make_simple_variant(key, value, info)
> > +            variants = [self._make_simple_variant(key, value['type'], info)
> >                          for (key, value) in data.items()]
> >              typ = self._make_implicit_enum_type(name, info, ifcond,
> >                                                  [v.name for v in variants])
> > @@ -1794,7 +1829,7 @@ class QAPISchema(object):
> >          name = expr['alternate']
> >          data = expr['data']
> >          ifcond = expr.get('if')
> > -        variants = [self._make_variant(key, value)
> > +        variants = [self._make_variant(key, value['type'])
> >                      for (key, value) in data.items()]
> >          tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
> >          self._def_entity(
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 3c9eea27fd..ea5d1e8787 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -318,6 +318,7 @@ qapi-schema += alternate-conflict-string.json
> >  qapi-schema += alternate-conflict-bool-string.json
> >  qapi-schema += alternate-conflict-num-string.json
> >  qapi-schema += alternate-empty.json
> > +qapi-schema += alternate-invalid-dict.json
> >  qapi-schema += alternate-nested.json
> >  qapi-schema += alternate-unknown.json
> >  qapi-schema += args-alternate.json
> > @@ -394,6 +395,7 @@ qapi-schema += escape-too-big.json
> >  qapi-schema += escape-too-short.json
> >  qapi-schema += event-boxed-empty.json
> >  qapi-schema += event-case.json
> > +qapi-schema += event-member-invalid-dict.json
> >  qapi-schema += event-nest-struct.json
> >  qapi-schema += flat-union-array-branch.json
> >  qapi-schema += flat-union-bad-base.json
> > @@ -403,6 +405,7 @@ qapi-schema += flat-union-base-union.json
> >  qapi-schema += flat-union-clash-member.json
> >  qapi-schema += flat-union-empty.json
> >  qapi-schema += flat-union-inline.json
> > +qapi-schema += flat-union-inline-invalid-dict.json
> >  qapi-schema += flat-union-int-branch.json
> >  qapi-schema += flat-union-invalid-branch-key.json
> >  qapi-schema += flat-union-invalid-discriminator.json
> > @@ -430,6 +433,7 @@ qapi-schema += missing-comma-list.json
> >  qapi-schema += missing-comma-object.json
> >  qapi-schema += missing-type.json
> >  qapi-schema += nested-struct-data.json
> > +qapi-schema += nested-struct-data-invalid-dict.json
> >  qapi-schema += non-objects.json
> >  qapi-schema += oob-test.json
> >  qapi-schema += allow-preconfig-test.json
> > @@ -460,6 +464,7 @@ qapi-schema += returns-whitelist.json
> >  qapi-schema += struct-base-clash-deep.json
> >  qapi-schema += struct-base-clash.json
> >  qapi-schema += struct-data-invalid.json
> > +qapi-schema += struct-member-invalid-dict.json
> >  qapi-schema += struct-member-invalid.json
> >  qapi-schema += trailing-comma-list.json
> >  qapi-schema += trailing-comma-object.json
> > @@ -471,6 +476,7 @@ qapi-schema += unicode-str.json
> >  qapi-schema += union-base-empty.json
> >  qapi-schema += union-base-no-discriminator.json
> >  qapi-schema += union-branch-case.json
> > +qapi-schema += union-branch-invalid-dict.json
> >  qapi-schema += union-clash-branches.json
> >  qapi-schema += union-empty.json
> >  qapi-schema += union-invalid-base.json
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.err b/tests/qapi-schema/alternate-invalid-dict.err
> > new file mode 100644
> > index 0000000000..631d46628e
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/alternate-invalid-dict.json:2: Key 'type' is missing from member 'two' of alternate 'Alt'
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.exit b/tests/qapi-schema/alternate-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.json b/tests/qapi-schema/alternate-invalid-dict.json
> > new file mode 100644
> > index 0000000000..8e0b2ac287
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.json
> > @@ -0,0 +1,4 @@
> > +# exploded member form must have a 'type'
> > +{ 'alternate': 'Alt',
> > +  'data': { 'one': 'str',
> > +            'two': { 'if': 'foo' } } }
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.out b/tests/qapi-schema/alternate-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err
> > new file mode 100644
> > index 0000000000..1a57fa29b0
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/event-member-invalid-dict.json:1: Key 'type' is missing from member 'a' of 'data' for event 'EVENT_A'
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.exit b/tests/qapi-schema/event-member-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json
> > new file mode 100644
> > index 0000000000..ee6f3ecb6f
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.json
> > @@ -0,0 +1,2 @@
> > +{ 'event': 'EVENT_A',
> > +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.out b/tests/qapi-schema/event-member-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/event-nest-struct.json b/tests/qapi-schema/event-nest-struct.json
> > index ee6f3ecb6f..355ddaeff1 100644
> > --- a/tests/qapi-schema/event-nest-struct.json
> > +++ b/tests/qapi-schema/event-nest-struct.json
> > @@ -1,2 +1,2 @@
> >  { 'event': 'EVENT_A',
> > -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > +  'data': { 'a' : { 'type' : { 'integer': 'int' } }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.err b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> > new file mode 100644
> > index 0000000000..9c4c45b7f0
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/flat-union-inline-invalid-dict.json:7: Key 'type' is missing from member 'value1' of union 'TestUnion'
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.exit b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> > new file mode 100644
> > index 0000000000..62c7cda617
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> > @@ -0,0 +1,11 @@
> > +# we require branches to be a struct name
> > +# TODO: should we allow anonymous inline branch types?
> > +{ 'enum': 'TestEnum',
> > +  'data': [ 'value1', 'value2' ] }
> > +{ 'struct': 'Base',
> > +  'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
> > +{ 'union': 'TestUnion',
> > +  'base': 'Base',
> > +  'discriminator': 'enum1',
> > +  'data': { 'value1': { 'string': 'str' },
> > +            'value2': { 'integer': 'int' } } }
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.out b/tests/qapi-schema/flat-union-inline-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
> > index 62c7cda617..a9b3ce3f0d 100644
> > --- a/tests/qapi-schema/flat-union-inline.json
> > +++ b/tests/qapi-schema/flat-union-inline.json
> > @@ -7,5 +7,5 @@
> >  { 'union': 'TestUnion',
> >    'base': 'Base',
> >    'discriminator': 'enum1',
> > -  'data': { 'value1': { 'string': 'str' },
> > +  'data': { 'value1': { 'type': {} },
> >              'value2': { 'integer': 'int' } } }
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> > new file mode 100644
> > index 0000000000..5bd364e8d9
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/nested-struct-data-invalid-dict.json:2: Key 'type' is missing from member 'a' of 'data' for command 'foo'
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.exit b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> > new file mode 100644
> > index 0000000000..efbe773ded
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> > @@ -0,0 +1,3 @@
> > +# inline subtypes collide with our desired future use of defaults
> > +{ 'command': 'foo',
> > +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.out b/tests/qapi-schema/nested-struct-data-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
> > index efbe773ded..5b8a40cca3 100644
> > --- a/tests/qapi-schema/nested-struct-data.json
> > +++ b/tests/qapi-schema/nested-struct-data.json
> > @@ -1,3 +1,3 @@
> >  # inline subtypes collide with our desired future use of defaults
> >  { 'command': 'foo',
> > -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > +  'data': { 'a' : { 'type': {} }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> > index 22d9044a89..94570154c9 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -11,7 +11,7 @@
> >          'guest-sync' ] } }
> >
> >  { 'struct': 'TestStruct',
> > -  'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
> > +  'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' } }
> >
> >  # for testing enums
> >  { 'struct': 'NestedEnumsOne',
> > @@ -77,7 +77,7 @@
> >  { 'union': 'UserDefFlatUnion',
> >    'base': 'UserDefUnionBase',   # intentional forward reference
> >    'discriminator': 'enum1',
> > -  'data': { 'value1' : 'UserDefA',
> > +  'data': { 'value1' : {'type': 'UserDefA'},
> >              'value2' : 'UserDefB',
> >              'value3' : 'UserDefB'
> >              # 'value4' defaults to empty
> > @@ -98,7 +98,7 @@
> >  { 'struct': 'WrapAlternate',
> >    'data': { 'alt': 'UserDefAlternate' } }
> >  { 'alternate': 'UserDefAlternate',
> > -  'data': { 'udfu': 'UserDefFlatUnion', 'e': 'EnumOne', 'i': 'int',
> > +  'data': { 'udfu': {'type': 'UserDefFlatUnion'}, 'e': 'EnumOne', 'i': 'int',
> >              'n': 'null' } }
> >
> >  { 'struct': 'UserDefC',
> > @@ -134,7 +134,7 @@
> >  { 'command': 'user_def_cmd', 'data': {} }
> >  { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
> >  { 'command': 'user_def_cmd2',
> > -  'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
> > +  'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'},
> >    'returns': 'UserDefTwo' }
> >
> >  { 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
> > @@ -166,7 +166,7 @@
> >
> >  # testing event
> >  { 'struct': 'EventStructOne',
> > -  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
> > +  'data': { 'struct1': {'type': 'UserDefOne'}, 'string': 'str', '*enum2': 'EnumOne' } }
> >
> >  { 'event': 'EVENT_A' }
> >  { 'event': 'EVENT_B',
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err
> > new file mode 100644
> > index 0000000000..6a765bc668
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/struct-member-invalid-dict.json:2: Key 'type' is missing from member '*a' of 'data' for struct 'foo'
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.exit b/tests/qapi-schema/struct-member-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json
> > new file mode 100644
> > index 0000000000..9fe0d455a9
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.json
> > @@ -0,0 +1,3 @@
> > +# Long form of member must have a value member 'type'
> > +{ 'struct': 'foo',
> > +  'data': { '*a': { 'case': 'foo' } } }
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.out b/tests/qapi-schema/struct-member-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.err b/tests/qapi-schema/union-branch-invalid-dict.err
> > new file mode 100644
> > index 0000000000..89f9b36791
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/union-branch-invalid-dict.json:2: Key 'type' is missing from member 'integer' of union 'UnionInvalidBranch'
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.exit b/tests/qapi-schema/union-branch-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.json b/tests/qapi-schema/union-branch-invalid-dict.json
> > new file mode 100644
> > index 0000000000..9778598dbd
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.json
> > @@ -0,0 +1,4 @@
> > +# Long form of member must have a value member 'type'
> > +{ 'union': 'UnionInvalidBranch',
> > +  'data': { 'integer': { 'if': 'foo'},
> > +            's8': 'int8' } }
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.out b/tests/qapi-schema/union-branch-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
>
> Looks good, but I'd like to investigate whether my suggested placement
> of the normalization would simplify things before I give my R-by.
>

I am not sure I follow, you suggest to have normalization before
check? Then how can we produce an error that reflects the input
accurately? Should we carry additional information (original form etc)
on the source during normalization?
Markus Armbruster Dec. 11, 2018, 3:18 p.m. UTC | #3
New pylint gripes:

scripts/qapi/common.py:674:0: R0912: Too many branches (13/12) (too-many-branches)
scripts/qapi/common.py:764:0: R0912: Too many branches (13/12) (too-many-branches)
scripts/qapi/common.py:838:0: R0912: Too many branches (13/12) (too-many-branches)

That's check_type(), check_union(), check_alternate().  Let's ignore
these for now.
Markus Armbruster Dec. 11, 2018, 3:35 p.m. UTC | #4
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Mon, Dec 10, 2018 at 9:26 PM Markus Armbruster <armbru@redhat.com> wrote:
[...]
>> Looks good, but I'd like to investigate whether my suggested placement
>> of the normalization would simplify things before I give my R-by.
>>
>
> I am not sure I follow, you suggest to have normalization before
> check? Then how can we produce an error that reflects the input
> accurately? Should we carry additional information (original form etc)
> on the source during normalization?

Let me play with it.  If my idea works out, showing it to you will be
easier than explaining it.  If it doesn't, ditching it quietly will also
be easier than explaining it :)
diff mbox series

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 95e55b3f44..4b3ba53dc7 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -588,11 +588,13 @@  def discriminator_find_enum_define(expr):
     if not base_members:
         return None
 
-    discriminator_type = base_members.get(discriminator)
-    if not discriminator_type:
+    discriminator_value = base_members.get(discriminator)
+    if not discriminator_value:
         return None
 
-    return enum_types.get(discriminator_type)
+    if isinstance(discriminator_value, dict):
+        discriminator_value = discriminator_value['type']
+    return enum_types.get(discriminator_value)
 
 
 # Names must be letters, numbers, -, and _.  They must start with letter,
@@ -660,6 +662,15 @@  def check_if(expr, info):
         check_if_str(ifcond, info)
 
 
+def normalize_members(expr, field):
+    members = expr.get(field)
+    if isinstance(members, OrderedDict):
+        for key, arg in members.items():
+            if isinstance(arg, dict):
+                continue
+            members[key] = {'type': arg}
+
+
 def check_type(info, source, value, allow_array=False,
                allow_implicit=False, allow_optional=False,
                allow_metas=[]):
@@ -704,8 +715,14 @@  def check_type(info, source, value, allow_array=False,
                                % (source, key))
         # Todo: allow dictionaries to represent default values of
         # an optional argument.
-        check_type(info, "Member '%s' of %s" % (key, source), arg,
-                   allow_array=True,
+        if isinstance(arg, dict):
+            check_known_keys(info, "member '%s' of %s" % (key, source),
+                             arg, ['type'], [])
+            typ = arg['type']
+        else:
+            typ = arg
+        check_type(info, "Member '%s' of %s" % (key, source),
+                   typ, allow_array=True,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])
 
@@ -776,13 +793,15 @@  def check_union(expr, info):
         # member of the base struct.
         check_name(info, "Discriminator of flat union '%s'" % name,
                    discriminator)
-        discriminator_type = base_members.get(discriminator)
-        if not discriminator_type:
+        discriminator_value = base_members.get(discriminator)
+        if not discriminator_value:
             raise QAPISemError(info,
                                "Discriminator '%s' is not a member of base "
                                "struct '%s'"
                                % (discriminator, base))
-        enum_define = enum_types.get(discriminator_type)
+        if isinstance(discriminator_value, dict):
+            discriminator_value = discriminator_value['type']
+        enum_define = enum_types.get(discriminator_value)
         allow_metas = ['struct']
         # Do not allow string discriminator
         if not enum_define:
@@ -795,10 +814,16 @@  def check_union(expr, info):
         raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
     for (key, value) in members.items():
         check_name(info, "Member of union '%s'" % name, key)
+        if isinstance(value, dict):
+            check_known_keys(info, "member '%s' of union '%s'" % (key, name),
+                             value, ['type'], [])
+            typ = value['type']
+        else:
+            typ = value
 
         # Each value must name a known type
         check_type(info, "Member '%s' of union '%s'" % (key, name),
-                   value, allow_array=not base, allow_metas=allow_metas)
+                   typ, allow_array=not base, allow_metas=allow_metas)
 
         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
@@ -822,18 +847,24 @@  def check_alternate(expr, info):
                            "in 'data'" % name)
     for (key, value) in members.items():
         check_name(info, "Member of alternate '%s'" % name, key)
+        if isinstance(value, dict):
+            check_known_keys(info,
+                             "member '%s' of alternate '%s'" % (key, name),
+                             value, ['type'], [])
+            typ = value['type']
+        else:
+            typ = value
 
         # Ensure alternates have no type conflicts.
-        check_type(info, "Member '%s' of alternate '%s'" % (key, name),
-                   value,
+        check_type(info, "Member '%s' of alternate '%s'" % (key, name), typ,
                    allow_metas=['built-in', 'union', 'struct', 'enum'])
-        qtype = find_alternate_member_qtype(value)
+        qtype = find_alternate_member_qtype(typ)
         if not qtype:
             raise QAPISemError(info, "Alternate '%s' member '%s' cannot use "
-                               "type '%s'" % (name, key, value))
+                               "type '%s'" % (name, key, typ))
         conflicting = set([qtype])
         if qtype == 'QTYPE_QSTRING':
-            enum_expr = enum_types.get(value)
+            enum_expr = enum_types.get(typ)
             if enum_expr:
                 for v in enum_get_names(enum_expr):
                     if v in ['on', 'off']:
@@ -1035,6 +1066,10 @@  def normalize_exprs(exprs):
         expr = expr_elem['expr']
         if 'enum' in expr:
             normalize_enum(expr)
+        elif 'union' in expr:
+            normalize_members(expr, 'base')
+        if {'union', 'alternate', 'struct', 'command', 'event'} & set(expr):
+            normalize_members(expr, 'data')
 
     return exprs
 
@@ -1738,7 +1773,7 @@  class QAPISchema(object):
         return QAPISchemaObjectTypeMember(name, typ, optional)
 
     def _make_members(self, data, info):
-        return [self._make_member(key, value, info)
+        return [self._make_member(key, value['type'], info)
                 for (key, value) in data.items()]
 
     def _def_struct_type(self, expr, info, doc):
@@ -1774,11 +1809,11 @@  class QAPISchema(object):
                 name, info, doc, ifcond,
                 'base', self._make_members(base, info))
         if tag_name:
-            variants = [self._make_variant(key, value)
+            variants = [self._make_variant(key, value['type'])
                         for (key, value) in data.items()]
             members = []
         else:
-            variants = [self._make_simple_variant(key, value, info)
+            variants = [self._make_simple_variant(key, value['type'], info)
                         for (key, value) in data.items()]
             typ = self._make_implicit_enum_type(name, info, ifcond,
                                                 [v.name for v in variants])
@@ -1794,7 +1829,7 @@  class QAPISchema(object):
         name = expr['alternate']
         data = expr['data']
         ifcond = expr.get('if')
-        variants = [self._make_variant(key, value)
+        variants = [self._make_variant(key, value['type'])
                     for (key, value) in data.items()]
         tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
         self._def_entity(
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3c9eea27fd..ea5d1e8787 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -318,6 +318,7 @@  qapi-schema += alternate-conflict-string.json
 qapi-schema += alternate-conflict-bool-string.json
 qapi-schema += alternate-conflict-num-string.json
 qapi-schema += alternate-empty.json
+qapi-schema += alternate-invalid-dict.json
 qapi-schema += alternate-nested.json
 qapi-schema += alternate-unknown.json
 qapi-schema += args-alternate.json
@@ -394,6 +395,7 @@  qapi-schema += escape-too-big.json
 qapi-schema += escape-too-short.json
 qapi-schema += event-boxed-empty.json
 qapi-schema += event-case.json
+qapi-schema += event-member-invalid-dict.json
 qapi-schema += event-nest-struct.json
 qapi-schema += flat-union-array-branch.json
 qapi-schema += flat-union-bad-base.json
@@ -403,6 +405,7 @@  qapi-schema += flat-union-base-union.json
 qapi-schema += flat-union-clash-member.json
 qapi-schema += flat-union-empty.json
 qapi-schema += flat-union-inline.json
+qapi-schema += flat-union-inline-invalid-dict.json
 qapi-schema += flat-union-int-branch.json
 qapi-schema += flat-union-invalid-branch-key.json
 qapi-schema += flat-union-invalid-discriminator.json
@@ -430,6 +433,7 @@  qapi-schema += missing-comma-list.json
 qapi-schema += missing-comma-object.json
 qapi-schema += missing-type.json
 qapi-schema += nested-struct-data.json
+qapi-schema += nested-struct-data-invalid-dict.json
 qapi-schema += non-objects.json
 qapi-schema += oob-test.json
 qapi-schema += allow-preconfig-test.json
@@ -460,6 +464,7 @@  qapi-schema += returns-whitelist.json
 qapi-schema += struct-base-clash-deep.json
 qapi-schema += struct-base-clash.json
 qapi-schema += struct-data-invalid.json
+qapi-schema += struct-member-invalid-dict.json
 qapi-schema += struct-member-invalid.json
 qapi-schema += trailing-comma-list.json
 qapi-schema += trailing-comma-object.json
@@ -471,6 +476,7 @@  qapi-schema += unicode-str.json
 qapi-schema += union-base-empty.json
 qapi-schema += union-base-no-discriminator.json
 qapi-schema += union-branch-case.json
+qapi-schema += union-branch-invalid-dict.json
 qapi-schema += union-clash-branches.json
 qapi-schema += union-empty.json
 qapi-schema += union-invalid-base.json
diff --git a/tests/qapi-schema/alternate-invalid-dict.err b/tests/qapi-schema/alternate-invalid-dict.err
new file mode 100644
index 0000000000..631d46628e
--- /dev/null
+++ b/tests/qapi-schema/alternate-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/alternate-invalid-dict.json:2: Key 'type' is missing from member 'two' of alternate 'Alt'
diff --git a/tests/qapi-schema/alternate-invalid-dict.exit b/tests/qapi-schema/alternate-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/alternate-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/alternate-invalid-dict.json b/tests/qapi-schema/alternate-invalid-dict.json
new file mode 100644
index 0000000000..8e0b2ac287
--- /dev/null
+++ b/tests/qapi-schema/alternate-invalid-dict.json
@@ -0,0 +1,4 @@ 
+# exploded member form must have a 'type'
+{ 'alternate': 'Alt',
+  'data': { 'one': 'str',
+            'two': { 'if': 'foo' } } }
diff --git a/tests/qapi-schema/alternate-invalid-dict.out b/tests/qapi-schema/alternate-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err
new file mode 100644
index 0000000000..1a57fa29b0
--- /dev/null
+++ b/tests/qapi-schema/event-member-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/event-member-invalid-dict.json:1: Key 'type' is missing from member 'a' of 'data' for event 'EVENT_A'
diff --git a/tests/qapi-schema/event-member-invalid-dict.exit b/tests/qapi-schema/event-member-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/event-member-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json
new file mode 100644
index 0000000000..ee6f3ecb6f
--- /dev/null
+++ b/tests/qapi-schema/event-member-invalid-dict.json
@@ -0,0 +1,2 @@ 
+{ 'event': 'EVENT_A',
+  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/event-member-invalid-dict.out b/tests/qapi-schema/event-member-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/event-nest-struct.json b/tests/qapi-schema/event-nest-struct.json
index ee6f3ecb6f..355ddaeff1 100644
--- a/tests/qapi-schema/event-nest-struct.json
+++ b/tests/qapi-schema/event-nest-struct.json
@@ -1,2 +1,2 @@ 
 { 'event': 'EVENT_A',
-  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
+  'data': { 'a' : { 'type' : { 'integer': 'int' } }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.err b/tests/qapi-schema/flat-union-inline-invalid-dict.err
new file mode 100644
index 0000000000..9c4c45b7f0
--- /dev/null
+++ b/tests/qapi-schema/flat-union-inline-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/flat-union-inline-invalid-dict.json:7: Key 'type' is missing from member 'value1' of union 'TestUnion'
diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.exit b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json
new file mode 100644
index 0000000000..62c7cda617
--- /dev/null
+++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json
@@ -0,0 +1,11 @@ 
+# we require branches to be a struct name
+# TODO: should we allow anonymous inline branch types?
+{ 'enum': 'TestEnum',
+  'data': [ 'value1', 'value2' ] }
+{ 'struct': 'Base',
+  'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
+{ 'union': 'TestUnion',
+  'base': 'Base',
+  'discriminator': 'enum1',
+  'data': { 'value1': { 'string': 'str' },
+            'value2': { 'integer': 'int' } } }
diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.out b/tests/qapi-schema/flat-union-inline-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
index 62c7cda617..a9b3ce3f0d 100644
--- a/tests/qapi-schema/flat-union-inline.json
+++ b/tests/qapi-schema/flat-union-inline.json
@@ -7,5 +7,5 @@ 
 { 'union': 'TestUnion',
   'base': 'Base',
   'discriminator': 'enum1',
-  'data': { 'value1': { 'string': 'str' },
+  'data': { 'value1': { 'type': {} },
             'value2': { 'integer': 'int' } } }
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err
new file mode 100644
index 0000000000..5bd364e8d9
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/nested-struct-data-invalid-dict.json:2: Key 'type' is missing from member 'a' of 'data' for command 'foo'
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.exit b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json
new file mode 100644
index 0000000000..efbe773ded
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json
@@ -0,0 +1,3 @@ 
+# inline subtypes collide with our desired future use of defaults
+{ 'command': 'foo',
+  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.out b/tests/qapi-schema/nested-struct-data-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
index efbe773ded..5b8a40cca3 100644
--- a/tests/qapi-schema/nested-struct-data.json
+++ b/tests/qapi-schema/nested-struct-data.json
@@ -1,3 +1,3 @@ 
 # inline subtypes collide with our desired future use of defaults
 { 'command': 'foo',
-  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
+  'data': { 'a' : { 'type': {} }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 22d9044a89..94570154c9 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -11,7 +11,7 @@ 
         'guest-sync' ] } }
 
 { 'struct': 'TestStruct',
-  'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
+  'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' } }
 
 # for testing enums
 { 'struct': 'NestedEnumsOne',
@@ -77,7 +77,7 @@ 
 { 'union': 'UserDefFlatUnion',
   'base': 'UserDefUnionBase',   # intentional forward reference
   'discriminator': 'enum1',
-  'data': { 'value1' : 'UserDefA',
+  'data': { 'value1' : {'type': 'UserDefA'},
             'value2' : 'UserDefB',
             'value3' : 'UserDefB'
             # 'value4' defaults to empty
@@ -98,7 +98,7 @@ 
 { 'struct': 'WrapAlternate',
   'data': { 'alt': 'UserDefAlternate' } }
 { 'alternate': 'UserDefAlternate',
-  'data': { 'udfu': 'UserDefFlatUnion', 'e': 'EnumOne', 'i': 'int',
+  'data': { 'udfu': {'type': 'UserDefFlatUnion'}, 'e': 'EnumOne', 'i': 'int',
             'n': 'null' } }
 
 { 'struct': 'UserDefC',
@@ -134,7 +134,7 @@ 
 { 'command': 'user_def_cmd', 'data': {} }
 { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
 { 'command': 'user_def_cmd2',
-  'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
+  'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'},
   'returns': 'UserDefTwo' }
 
 { 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
@@ -166,7 +166,7 @@ 
 
 # testing event
 { 'struct': 'EventStructOne',
-  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
+  'data': { 'struct1': {'type': 'UserDefOne'}, 'string': 'str', '*enum2': 'EnumOne' } }
 
 { 'event': 'EVENT_A' }
 { 'event': 'EVENT_B',
diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err
new file mode 100644
index 0000000000..6a765bc668
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/struct-member-invalid-dict.json:2: Key 'type' is missing from member '*a' of 'data' for struct 'foo'
diff --git a/tests/qapi-schema/struct-member-invalid-dict.exit b/tests/qapi-schema/struct-member-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json
new file mode 100644
index 0000000000..9fe0d455a9
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid-dict.json
@@ -0,0 +1,3 @@ 
+# Long form of member must have a value member 'type'
+{ 'struct': 'foo',
+  'data': { '*a': { 'case': 'foo' } } }
diff --git a/tests/qapi-schema/struct-member-invalid-dict.out b/tests/qapi-schema/struct-member-invalid-dict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/union-branch-invalid-dict.err b/tests/qapi-schema/union-branch-invalid-dict.err
new file mode 100644
index 0000000000..89f9b36791
--- /dev/null
+++ b/tests/qapi-schema/union-branch-invalid-dict.err
@@ -0,0 +1 @@ 
+tests/qapi-schema/union-branch-invalid-dict.json:2: Key 'type' is missing from member 'integer' of union 'UnionInvalidBranch'
diff --git a/tests/qapi-schema/union-branch-invalid-dict.exit b/tests/qapi-schema/union-branch-invalid-dict.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/union-branch-invalid-dict.exit
@@ -0,0 +1 @@ 
+1
diff --git a/tests/qapi-schema/union-branch-invalid-dict.json b/tests/qapi-schema/union-branch-invalid-dict.json
new file mode 100644
index 0000000000..9778598dbd
--- /dev/null
+++ b/tests/qapi-schema/union-branch-invalid-dict.json
@@ -0,0 +1,4 @@ 
+# Long form of member must have a value member 'type'
+{ 'union': 'UnionInvalidBranch',
+  'data': { 'integer': { 'if': 'foo'},
+            's8': 'int8' } }