In subject, s/dictionnary/dictionary/ Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> Desugar the enum NAME form to { 'name': NAME }. This will allow to add > new enum members, such as 'if' in the following patch. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > scripts/qapi/common.py | 47 ++++++++++++++++--- > tests/Makefile.include | 3 +- > tests/qapi-schema/enum-bad-member.err | 1 + > ...-dict-member.exit => enum-bad-member.exit} | 0 > tests/qapi-schema/enum-bad-member.json | 2 + > ...um-dict-member.out => enum-bad-member.out} | 0 > .../qapi-schema/enum-dict-member-unknown.err | 2 + > .../qapi-schema/enum-dict-member-unknown.exit | 1 + > .../qapi-schema/enum-dict-member-unknown.json | 2 + > .../qapi-schema/enum-dict-member-unknown.out | 0 > tests/qapi-schema/enum-dict-member.err | 1 - > tests/qapi-schema/enum-dict-member.json | 2 - > tests/qapi-schema/enum-missing-data.err | 2 +- > 13 files changed, 51 insertions(+), 12 deletions(-) > create mode 100644 tests/qapi-schema/enum-bad-member.err > rename tests/qapi-schema/{enum-dict-member.exit => enum-bad-member.exit} > (100%) > create mode 100644 tests/qapi-schema/enum-bad-member.json > rename tests/qapi-schema/{enum-dict-member.out => enum-bad-member.out} (100%) > create mode 100644 tests/qapi-schema/enum-dict-member-unknown.err > create mode 100644 tests/qapi-schema/enum-dict-member-unknown.exit > create mode 100644 tests/qapi-schema/enum-dict-member-unknown.json > create mode 100644 tests/qapi-schema/enum-dict-member-unknown.out > delete mode 100644 tests/qapi-schema/enum-dict-member.err > delete mode 100644 tests/qapi-schema/enum-dict-member.json > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index cf8dab2866..e9fb736d46 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -740,6 +740,10 @@ def check_event(expr, info): > allow_metas=meta) > > > +def enum_get_names(expr): > + return [e['name'] for e in expr['data']] > + > + > def check_union(expr, info): > name = expr['union'] > base = expr.get('base') > @@ -799,7 +803,7 @@ def check_union(expr, info): > # If the discriminator names an enum type, then all members > # of 'data' must also be members of the enum type. > if enum_define: > - if key not in enum_define['data']: > + if key not in enum_get_names(enum_define): > raise QAPISemError(info, > "Discriminator value '%s' is not found in > " > "enum '%s'" > @@ -831,10 +835,10 @@ def check_alternate(expr, info): > if qtype == 'QTYPE_QSTRING': > enum_expr = enum_types.get(value) > if enum_expr: > - for v in enum_expr['data']: > + for v in enum_get_names(enum_expr): > if v in ['on', 'off']: > conflicting.add('QTYPE_QBOOL') > - if re.match(r'[-+0-9.]', v): # lazy, could be tightened > + if re.match(r'[-+0-9.]', v): # lazy, could be tightened Unrelated change. > conflicting.add('QTYPE_QNUM') > else: > conflicting.add('QTYPE_QNUM') The helper enum_get_names() is used just twice. The second instance for v in enum_get_names(enum_expr): could just as well iterate over enum_expr for d in enum_expr['data']: v = d['name'] The first instance could simply be inlined then. > @@ -847,19 +851,34 @@ def check_alternate(expr, info): > types_seen[qt] = key > > > -def check_enum(expr, info): > +def normalize_enum(expr, info): > name = expr['enum'] > members = expr.get('data') > - prefix = expr.get('prefix') > > if not isinstance(members, list): > raise QAPISemError(info, > "Enum '%s' requires an array for 'data'" % name) > + > + # translate short member form to dict form > + for i, member in enumerate(members): > + if not isinstance(member, dict): > + member = {'name': member} > + members[i] = member Simpler: members = [m if isinstance(m, dict) else {'name': m} for m in members] > + > + > +def check_enum(expr, info): > + name = expr['enum'] > + members = expr.get('data') Preexisting: this should be expr['data'], because we already checked required keys. > + prefix = expr.get('prefix') > + > if prefix is not None and not isinstance(prefix, str): > raise QAPISemError(info, > "Enum '%s' requires a string for 'prefix'" % name) > + > for member in members: > - check_name(info, "Member of enum '%s'" % name, member, > + source = "dictionary member of enum '%s'" % name > + check_known_keys(info, source, member, ['name'], []) > + check_name(info, "Member of enum '%s'" % name, member['name'], > enum_member=True) > > > @@ -921,6 +940,13 @@ def check_exprs(exprs): > for builtin in builtin_types.keys(): > all_names[builtin] = 'built-in' > > + # Normalize exprs > + for expr_elem in exprs: > + expr = expr_elem['expr'] > + info = expr_elem['info'] > + if 'enum' in expr: > + normalize_enum(expr, info) > + As demonstrated by tests/qapi-schema/enum-missing-data.json below, the error message for {'enum': 'MyEnum'} changes from Key 'data' is missing from enum 'MyEnum' to Enum 'MyEnum' requires an array for 'data' because you normalize before checking keys. I don't think that's a good idea. > # Learn the types and check for valid expression keys > for expr_elem in exprs: > expr = expr_elem['expr'] info = expr_elem['info'] doc = expr_elem.get('doc') if 'include' in expr: continue if not doc and doc_required: raise QAPISemError(info, "Expression missing documentation comment") if 'enum' in expr: meta = 'enum' check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix']) This seems a better place to call normalize_enum(expr, info). enum_types[expr[meta]] = expr elif 'union' in expr: > @@ -1640,7 +1666,14 @@ class QAPISchema(object): > qtype_values, 'QTYPE')) > > def _make_enum_members(self, values): > - return [QAPISchemaMember(v) for v in values] > + enum = [] > + for v in values: > + if isinstance(v, dict): > + name = v['name'] > + else: > + name = v > + enum.append(QAPISchemaMember(name)) > + return enum Despite normalization to the long form, we still deal with both the long and the short form here. Three callers: * _def_predefineds(): passes the short form. * _make_implicit_enum_type(): passes the short form until PATCH 19 changes it to pass the long form. * _def_enum_type(): passes the long form. I believe always passing the long form would be simpler. > > def _make_implicit_enum_type(self, name, info, ifcond, values): > # See also QAPISchemaObjectTypeMember._pretty_owner() > diff --git a/tests/Makefile.include b/tests/Makefile.include > index a49282704e..8e1b122cf2 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -482,10 +482,11 @@ qapi-schema += double-data.json > qapi-schema += double-type.json > qapi-schema += duplicate-key.json > qapi-schema += empty.json > +qapi-schema += enum-bad-member.json > qapi-schema += enum-bad-name.json > qapi-schema += enum-bad-prefix.json > qapi-schema += enum-clash-member.json > -qapi-schema += enum-dict-member.json > +qapi-schema += enum-dict-member-unknown.json > qapi-schema += enum-int-member.json > qapi-schema += enum-member-case.json > qapi-schema += enum-missing-data.json > diff --git a/tests/qapi-schema/enum-bad-member.err > b/tests/qapi-schema/enum-bad-member.err > new file mode 100644 > index 0000000000..211db9e6fc > --- /dev/null > +++ b/tests/qapi-schema/enum-bad-member.err > @@ -0,0 +1 @@ > +tests/qapi-schema/enum-bad-member.json:2: Member of enum 'MyEnum' requires a > string name > diff --git a/tests/qapi-schema/enum-dict-member.exit > b/tests/qapi-schema/enum-bad-member.exit > similarity index 100% > rename from tests/qapi-schema/enum-dict-member.exit > rename to tests/qapi-schema/enum-bad-member.exit > diff --git a/tests/qapi-schema/enum-bad-member.json > b/tests/qapi-schema/enum-bad-member.json > new file mode 100644 > index 0000000000..98da6828b4 > --- /dev/null > +++ b/tests/qapi-schema/enum-bad-member.json > @@ -0,0 +1,2 @@ > +# we reject any enum member that is not a string > +{ 'enum': 'MyEnum', 'data': [ [ ] ] } > diff --git a/tests/qapi-schema/enum-dict-member.out > b/tests/qapi-schema/enum-bad-member.out > similarity index 100% > rename from tests/qapi-schema/enum-dict-member.out > rename to tests/qapi-schema/enum-bad-member.out This replaces enum-dict-member.json. Okay. Took me a while to see this %-/ > diff --git a/tests/qapi-schema/enum-dict-member-unknown.err > b/tests/qapi-schema/enum-dict-member-unknown.err > new file mode 100644 > index 0000000000..76bd0471db > --- /dev/null > +++ b/tests/qapi-schema/enum-dict-member-unknown.err > @@ -0,0 +1,2 @@ > +tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in > dictionary member of enum 'MyEnum' > +Valid keys are 'name'. > diff --git a/tests/qapi-schema/enum-dict-member-unknown.exit > b/tests/qapi-schema/enum-dict-member-unknown.exit > new file mode 100644 > index 0000000000..d00491fd7e > --- /dev/null > +++ b/tests/qapi-schema/enum-dict-member-unknown.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/enum-dict-member-unknown.json > b/tests/qapi-schema/enum-dict-member-unknown.json > new file mode 100644 > index 0000000000..6664c59201 > --- /dev/null > +++ b/tests/qapi-schema/enum-dict-member-unknown.json > @@ -0,0 +1,2 @@ > +# we reject any enum member that is not a string or a dict with 'name' > +{ 'enum': 'MyEnum', 'data': [ { 'name': 'foo', 'bad-key': 'str' } ] } > diff --git a/tests/qapi-schema/enum-dict-member-unknown.out > b/tests/qapi-schema/enum-dict-member-unknown.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/qapi-schema/enum-dict-member.err > b/tests/qapi-schema/enum-dict-member.err > deleted file mode 100644 > index 8ca146ea59..0000000000 > --- a/tests/qapi-schema/enum-dict-member.err > +++ /dev/null > @@ -1 +0,0 @@ > -tests/qapi-schema/enum-dict-member.json:2: Member of enum 'MyEnum' requires > a string name > diff --git a/tests/qapi-schema/enum-dict-member.json > b/tests/qapi-schema/enum-dict-member.json > deleted file mode 100644 > index 79672e0f09..0000000000 > --- a/tests/qapi-schema/enum-dict-member.json > +++ /dev/null > @@ -1,2 +0,0 @@ > -# we reject any enum member that is not a string > -{ 'enum': 'MyEnum', 'data': [ { 'value': 'str' } ] } > diff --git a/tests/qapi-schema/enum-missing-data.err > b/tests/qapi-schema/enum-missing-data.err > index ba4873ae69..b8ccae071b 100644 > --- a/tests/qapi-schema/enum-missing-data.err > +++ b/tests/qapi-schema/enum-missing-data.err > @@ -1 +1 @@ > -tests/qapi-schema/enum-missing-data.json:2: Key 'data' is missing from enum > 'MyEnum' > +tests/qapi-schema/enum-missing-data.json:2: Enum 'MyEnum' requires an array > for 'data'