On 2/24/21 5:39 AM, Markus Armbruster wrote:
John Snow <js...@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 <js...@redhat.com>
Reviewed-by: Eduardo Habkost <ehabk...@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
