| Message ID | 20201026213637.47087-5-jsnow@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qapi: static typing conversion, pt3 | expand |
John Snow <jsnow@redhat.com> writes: > mypy isn't fond of allowing you to check for bool membership in a > collection of str elements. Guard this lookup for precisely when we were > given a name. Spoilsport. Peeking at the patch... aha, it's about check_type()'s parameter @allow_dict. @allow_dict tells us whether an anonymous type is allowed, and also whether its member names may violate the naming rules. * a str: allow anonymous type, waive member naming rules if @allow_dict is in .name_case_whitelist. Used for checking struct's 'data' and union's 'base'. * True: allow anonymous type, enforce member naming rules Used for checking 'data' of commands and events. Waiving the naming rules is simply not implemented there. * False (default): do not allow anonymous type Perhaps the "is in .name_case_whitelist" check should be lifted into the two callers that pass a str. We could then turn the parameter into an enum. Meh. Perhaps a separate @permit_upper parameter, only valid with allow_dict=True. Meh again. Splitting check_type() into multiple functions feels more promising. Not now. > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qapi/expr.py | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index f7c7f91326ef..2c4c341d5243 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -173,7 +173,9 @@ def check_type(value, info, source, > raise QAPISemError(info, > "%s should be an object or type name" % source) > > - permit_upper = allow_dict in info.pragma.name_case_whitelist > + permit_upper = False > + if isinstance(allow_dict, str): > + permit_upper = allow_dict in info.pragma.name_case_whitelist Slightly more compact: permit_upper = (isinstance(allow_dict, str) and allow_dict in info.pragma.name_case_whitelist) Matter of taste. > > # value is a dictionary, check that each member is okay > for (key, arg) in value.items():
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index f7c7f91326ef..2c4c341d5243 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -173,7 +173,9 @@ def check_type(value, info, source, raise QAPISemError(info, "%s should be an object or type name" % source) - permit_upper = allow_dict in info.pragma.name_case_whitelist + permit_upper = False + if isinstance(allow_dict, str): + permit_upper = allow_dict in info.pragma.name_case_whitelist # value is a dictionary, check that each member is okay for (key, arg) in value.items():