diff mbox series

[v8,06/22] qapi: add a dictionary form with 'name' key for enum members

Message ID 20181213123724.4866-7-marcandre.lureau@redhat.com
State New
Headers show
Series qapi: add #if pre-processor conditions to generated code (part 2) | expand

Commit Message

Marc-André Lureau Dec. 13, 2018, 12:37 p.m. UTC
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.lureau@redhat.com>
---
 scripts/qapi/common.py                        | 38 +++++++++++++++----
 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 -
 12 files changed, 40 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

Comments

Markus Armbruster Dec. 13, 2018, 1:23 p.m. UTC | #1
Marc-André Lureau <marcandre.lureau@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.lureau@redhat.com>

With the next two patches squashed in:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Happy to squash when I apply.  I forgot to do it in my qapi-if-2 branch.
Markus Armbruster Dec. 13, 2018, 2:32 p.m. UTC | #2
One little thing...

Marc-André Lureau <marcandre.lureau@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.lureau@redhat.com>
> ---
[...]
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index f205805751..bb928d2f7d 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
[...]
> @@ -847,19 +851,29 @@ def check_alternate(expr, info):
>              types_seen[qt] = key
>  
>  
> +def normalize_enum(expr):
> +    if isinstance(expr['data'], list):
> +        expr['data'] = [m if isinstance(m, dict) else {'name': m}
> +                        for m in expr['data']]
> +
> +
>  def check_enum(expr, info):
>      name = expr['enum']
> -    members = expr.get('data')
> +    members = expr['data']
>      prefix = expr.get('prefix')
>  
> -    if not isinstance(members, list):
> -        raise QAPISemError(info,
> -                           "Enum '%s' requires an array for 'data'" % name)
>      if prefix is not None and not isinstance(prefix, str):
>          raise QAPISemError(info,
>                             "Enum '%s' requires a string for 'prefix'" % name)
> +
> +    if not isinstance(members, list):
> +        raise QAPISemError(info,
> +                           "Enum '%s' requires an array for 'data'" % name)
> +

The move of the "requires an array" check looks accidental.  I'll back
it out unless you tell me it's needed.

>      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)
>  
>  
[...]
Marc-André Lureau Dec. 13, 2018, 2:35 p.m. UTC | #3
On Thu, Dec 13, 2018 at 6:32 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> One little thing...
>
> Marc-André Lureau <marcandre.lureau@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.lureau@redhat.com>
> > ---
> [...]
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index f205805751..bb928d2f7d 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> [...]
> > @@ -847,19 +851,29 @@ def check_alternate(expr, info):
> >              types_seen[qt] = key
> >
> >
> > +def normalize_enum(expr):
> > +    if isinstance(expr['data'], list):
> > +        expr['data'] = [m if isinstance(m, dict) else {'name': m}
> > +                        for m in expr['data']]
> > +
> > +
> >  def check_enum(expr, info):
> >      name = expr['enum']
> > -    members = expr.get('data')
> > +    members = expr['data']
> >      prefix = expr.get('prefix')
> >
> > -    if not isinstance(members, list):
> > -        raise QAPISemError(info,
> > -                           "Enum '%s' requires an array for 'data'" % name)
> >      if prefix is not None and not isinstance(prefix, str):
> >          raise QAPISemError(info,
> >                             "Enum '%s' requires a string for 'prefix'" % name)
> > +
> > +    if not isinstance(members, list):
> > +        raise QAPISemError(info,
> > +                           "Enum '%s' requires an array for 'data'" % name)
> > +
>
> The move of the "requires an array" check looks accidental.  I'll back
> it out unless you tell me it's needed.

ack

>
> >      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)
> >
> >
> [...]
diff mbox series

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f205805751..bb928d2f7d 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,7 +835,7 @@  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
@@ -847,19 +851,29 @@  def check_alternate(expr, info):
             types_seen[qt] = key
 
 
+def normalize_enum(expr):
+    if isinstance(expr['data'], list):
+        expr['data'] = [m if isinstance(m, dict) else {'name': m}
+                        for m in expr['data']]
+
+
 def check_enum(expr, info):
     name = expr['enum']
-    members = expr.get('data')
+    members = expr['data']
     prefix = expr.get('prefix')
 
-    if not isinstance(members, list):
-        raise QAPISemError(info,
-                           "Enum '%s' requires an array for 'data'" % name)
     if prefix is not None and not isinstance(prefix, str):
         raise QAPISemError(info,
                            "Enum '%s' requires a string for 'prefix'" % name)
+
+    if not isinstance(members, list):
+        raise QAPISemError(info,
+                           "Enum '%s' requires an array for 'data'" % 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)
 
 
@@ -937,6 +951,7 @@  def check_exprs(exprs):
         if 'enum' in expr:
             meta = 'enum'
             check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix'])
+            normalize_enum(expr)
             enum_types[expr[meta]] = expr
         elif 'union' in expr:
             meta = 'union'
@@ -1640,7 +1655,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
 
     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 fb0b449c02..2e894c1037 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -379,10 +379,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
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' } ] }