On Tue, Sep 22, 2020 at 05:13:13PM -0400, John Snow wrote: > This enforces a type signature against all of the top-level expression > check routines without necessarily needing to create some > overcomplicated class hierarchy for them. > > Signed-off-by: John Snow <js...@redhat.com> > --- > scripts/qapi/expr.py | 69 ++++++++++++++++++++++---------------------- > 1 file changed, 35 insertions(+), 34 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 74b2681ef8..cfd342aa04 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -31,8 +31,11 @@ > structures and contextual semantic validation. > """ > > +from enum import Enum > import re > from typing import ( > + Callable, > + Dict, > Iterable, > List, > MutableMapping, > @@ -494,6 +497,26 @@ def check_event(expr: Expression, info: QAPISourceInfo) > -> None: > check_type(args, info, "'data'", allow_dict=not boxed) > > > +class ExpressionType(str, Enum): > + INCLUDE = 'include' > + ENUM = 'enum' > + UNION = 'union' > + ALTERNATE = 'alternate' > + STRUCT = 'struct' > + COMMAND = 'command' > + EVENT = 'event' > + > + > +_CHECK_FN: Dict[str, Callable[[Expression, QAPISourceInfo], None]] = { > + 'enum': check_enum, > + 'union': check_union, > + 'alternate': check_alternate, > + 'struct': check_struct, > + 'command': check_command, > + 'event': check_event, > +} > + > + > def check_exprs(exprs: List[_JSObject]) -> List[_JSObject]: > """ > Validate and normalize a list of parsed QAPI schema expressions. [RW] > @@ -519,28 +542,20 @@ def check_exprs(exprs: List[_JSObject]) -> > List[_JSObject]: > assert tmp is None or isinstance(tmp, QAPIDoc) > doc: Optional[QAPIDoc] = tmp > > - if 'include' in expr: > - continue > - > - if 'enum' in expr: > - meta = 'enum' > - elif 'union' in expr: > - meta = 'union' > - elif 'alternate' in expr: > - meta = 'alternate' > - elif 'struct' in expr: > - meta = 'struct' > - elif 'command' in expr: > - meta = 'command' > - elif 'event' in expr: > - meta = 'event' > + for kind in ExpressionType: > + if kind in expr: > + meta = kind
I see lots of meta.value expressions below. Maybe setting meta = kind.value will make the code more readable? I don't think this should block an obvious improvement to the code, so: Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> > + break > else: > raise QAPISemError(info, "expression is missing metatype") > > + if meta == ExpressionType.INCLUDE: > + continue > + > name = cast(str, expr[meta]) # asserted right below: > - check_name_is_str(name, info, "'%s'" % meta) > - info.set_defn(meta, name) > - check_defn_name_str(name, info, meta) > + check_name_is_str(name, info, "'%s'" % meta.value) > + info.set_defn(meta.value, name) > + check_defn_name_str(name, info, meta.value) > > if doc: > if doc.symbol != name: > @@ -551,22 +566,8 @@ def check_exprs(exprs: List[_JSObject]) -> > List[_JSObject]: > raise QAPISemError(info, > "documentation comment required") > > - if meta == 'enum': > - check_enum(expr, info) > - elif meta == 'union': > - check_union(expr, info) > - elif meta == 'alternate': > - check_alternate(expr, info) > - elif meta == 'struct': > - check_struct(expr, info) > - elif meta == 'command': > - check_command(expr, info) > - elif meta == 'event': > - check_event(expr, info) > - else: > - assert False, 'unexpected meta type' > - > - check_if(expr, info, meta) > + _CHECK_FN[meta](expr, info) > + check_if(expr, info, meta.value) > check_features(expr.get('features'), info) > check_flags(expr, info) > > -- > 2.26.2 > -- Eduardo