|
From: | John Snow |
Subject: | Re: [PATCH v3 06/16] qapi/expr.py: Check type of 'data' member |
Date: | Wed, 24 Feb 2021 17:06:12 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
On 2/24/21 5:39 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:Iterating over the members of data isn't going to work if it's not some form of dict anyway, but for the sake of mypy, formalize it. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@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 c97e8ce8a4d..afa6bd07769 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -254,6 +254,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) @@ -267,6 +270,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)All errors require a negative test. If an error is unreachable, it should be an assertion instead. If these new errors are reachable, the commit might be a bug fix.
You can, yes: Traceback (most recent call last): File "/home/jsnow/src/qemu/scripts/qapi-gen.py", line 19, in <module> sys.exit(main.main()) File "/home/jsnow/src/qemu/scripts/qapi/main.py", line 89, in main generate(args.schema, File "/home/jsnow/src/qemu/scripts/qapi/main.py", line 51, in generate schema = QAPISchema(schema_file) File "/home/jsnow/src/qemu/scripts/qapi/schema.py", line 853, in __init__ exprs = check_exprs(parser.exprs)File "/home/jsnow/src/qemu/scripts/qapi/expr.py", line 337, in check_exprs
check_union(expr, info)File "/home/jsnow/src/qemu/scripts/qapi/expr.py", line 248, in check_union
for (key, value) in members.items(): AttributeError: 'list' object has no attribute 'items' from this beauty: ## # @Foo: # # @id: identifier of the backend # # @driver: the backend driver to use # # @timer-period: timer period (in microseconds, 0: use lowest possible) # # Since: 4.0 ## { 'union': 'Foo', 'base': { 'id': 'str', 'driver': 'AudiodevDriver', '*timer-period': 'uint32' }, 'discriminator': 'driver', 'data': ['hello', 'world'] }I'll add some new regression tests for you. Do you want them squished in with this commit, or would you like it done kind of independently, at the beginning of this series instead?
--js
[Prev in Thread] | Current Thread | [Next in Thread] |