| Message ID | 20200922211313.4082880-7-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt2 | expand |
On Tue, Sep 22, 2020 at 05:13:03PM -0400, John Snow wrote: > Iterating over the members of data isn't going to work if it's not some > form of dict anyway, but for type safety, formalize it. > > Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> -- Eduardo
On Tue, Sep 22, 2020 at 05:13:03PM -0400, John Snow wrote: > Iterating over the members of data isn't going to work if it's not some > form of dict anyway, but for type safety, formalize it. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/expr.py | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 3f5af5f5e4..633f9b9172 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -247,6 +247,9 @@ def check_union(expr, info): > raise QAPISemError(info, "'discriminator' requires 'base'") > check_name_is_str(discriminator, info, "'discriminator'") > > + if not isinstance(members, dict): > + raise QAPISemError(info, "'data' must be an object") > + Don't you mean "must be a dict" ? > for (key, value) in members.items(): > source = "'data' member '%s'" % key > check_name_str(key, info, source) > @@ -260,6 +263,10 @@ def check_alternate(expr, info): > > if not members: > raise QAPISemError(info, "'data' must not be empty") > + > + if not isinstance(members, dict): > + raise QAPISemError(info, "'data' must be an object") > + Same here? - Cleber. > for (key, value) in members.items(): > source = "'data' member '%s'" % key > check_name_str(key, info, source) > -- > 2.26.2 >
On 9/24/20 8:31 PM, Cleber Rosa wrote: > On Tue, Sep 22, 2020 at 05:13:03PM -0400, John Snow wrote: >> Iterating over the members of data isn't going to work if it's not some >> form of dict anyway, but for type safety, formalize it. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi/expr.py | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >> index 3f5af5f5e4..633f9b9172 100644 >> --- a/scripts/qapi/expr.py >> +++ b/scripts/qapi/expr.py >> @@ -247,6 +247,9 @@ def check_union(expr, info): >> raise QAPISemError(info, "'discriminator' requires 'base'") >> check_name_is_str(discriminator, info, "'discriminator'") >> >> + if not isinstance(members, dict): >> + raise QAPISemError(info, "'data' must be an object") >> + > > Don't you mean "must be a dict" ? > This error is speaking JSON-ese! json objects become python dicts, so if we didn't get a python dict here, we didn't get a json object.
On Thu, Sep 24, 2020 at 08:50:27PM -0400, John Snow wrote: > On 9/24/20 8:31 PM, Cleber Rosa wrote: > > On Tue, Sep 22, 2020 at 05:13:03PM -0400, John Snow wrote: > > > Iterating over the members of data isn't going to work if it's not some > > > form of dict anyway, but for type safety, formalize it. > > > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > > --- > > > scripts/qapi/expr.py | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > > > index 3f5af5f5e4..633f9b9172 100644 > > > --- a/scripts/qapi/expr.py > > > +++ b/scripts/qapi/expr.py > > > @@ -247,6 +247,9 @@ def check_union(expr, info): > > > raise QAPISemError(info, "'discriminator' requires 'base'") > > > check_name_is_str(discriminator, info, "'discriminator'") > > > + if not isinstance(members, dict): > > > + raise QAPISemError(info, "'data' must be an object") > > > + > > > > Don't you mean "must be a dict" ? > > > > This error is speaking JSON-ese! json objects become python dicts, so if we > didn't get a python dict here, we didn't get a json object. Right! Thanks for the explanation. - Cleber.
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 3f5af5f5e4..633f9b9172 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -247,6 +247,9 @@ def check_union(expr, info): raise QAPISemError(info, "'discriminator' requires 'base'") check_name_is_str(discriminator, info, "'discriminator'") + if not isinstance(members, dict): + raise QAPISemError(info, "'data' must be an object") + for (key, value) in members.items(): source = "'data' member '%s'" % key check_name_str(key, info, source) @@ -260,6 +263,10 @@ def check_alternate(expr, info): if not members: raise QAPISemError(info, "'data' must not be empty") + + if not isinstance(members, dict): + raise QAPISemError(info, "'data' must be an object") + for (key, value) in members.items(): source = "'data' member '%s'" % key check_name_str(key, info, source)
Iterating over the members of data isn't going to work if it's not some form of dict anyway, but for type safety, formalize it. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/expr.py | 7 +++++++ 1 file changed, 7 insertions(+)