| Message ID | 20190924132830.15835-19-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: Pay back some frontend technical debt | expand |
On 9/24/19 8:28 AM, Markus Armbruster wrote: > Move check_if() from check_keys() to check_exprs() and call it later, > so its error messages gain an "in definition" line. > > Checking values in a function named check_keys() is unclean anyway. > The original sin was commit 0545f6b887 "qapi: Better error messages > for bad expressions", which checks the value of key 'name'. More > sinning in commit 2cbf09925a "qapi: More rigorous checking for type > safety bypass", commit c818408e44 "qapi: Implement boxed types for > commands/events", and commit 967c885108 "qapi: add 'if' to top-level > expressions". This commit does penance for the latter. The next > commits will do penance for the others. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 7577120f12..fe99e59153 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -920,8 +920,6 @@ def check_keys(expr, info, meta, required, optional=[]): raise QAPISemError(info, "'%s' of %s '%s' should only use true value" % (key, meta, name)) - if key == 'if': - check_if(expr, info) def normalize_enum(expr): @@ -1023,6 +1021,8 @@ def check_exprs(exprs): else: assert False, 'unexpected meta type' + check_if(expr, info) + if doc: doc.check_expr(expr) diff --git a/tests/qapi-schema/bad-if-empty-list.err b/tests/qapi-schema/bad-if-empty-list.err index 75fe6497bc..2218c9279b 100644 --- a/tests/qapi-schema/bad-if-empty-list.err +++ b/tests/qapi-schema/bad-if-empty-list.err @@ -1 +1,2 @@ +tests/qapi-schema/bad-if-empty-list.json: In struct 'TestIfStruct': tests/qapi-schema/bad-if-empty-list.json:2: 'if' condition [] is useless diff --git a/tests/qapi-schema/bad-if-empty.err b/tests/qapi-schema/bad-if-empty.err index 358bdc3e51..a3fdb3009d 100644 --- a/tests/qapi-schema/bad-if-empty.err +++ b/tests/qapi-schema/bad-if-empty.err @@ -1 +1,2 @@ +tests/qapi-schema/bad-if-empty.json: In struct 'TestIfStruct': tests/qapi-schema/bad-if-empty.json:2: 'if' condition '' makes no sense diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err index 53af099083..e03bf0fc3a 100644 --- a/tests/qapi-schema/bad-if-list.err +++ b/tests/qapi-schema/bad-if-list.err @@ -1 +1,2 @@ +tests/qapi-schema/bad-if-list.json: In struct 'TestIfStruct': tests/qapi-schema/bad-if-list.json:2: 'if' condition ' ' makes no sense diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err index c2e3f5f44c..190216c109 100644 --- a/tests/qapi-schema/bad-if.err +++ b/tests/qapi-schema/bad-if.err @@ -1 +1,2 @@ +tests/qapi-schema/bad-if.json: In struct 'TestIfStruct': tests/qapi-schema/bad-if.json:2: 'if' condition must be a string or a list of strings
Move check_if() from check_keys() to check_exprs() and call it later, so its error messages gain an "in definition" line. Checking values in a function named check_keys() is unclean anyway. The original sin was commit 0545f6b887 "qapi: Better error messages for bad expressions", which checks the value of key 'name'. More sinning in commit 2cbf09925a "qapi: More rigorous checking for type safety bypass", commit c818408e44 "qapi: Implement boxed types for commands/events", and commit 967c885108 "qapi: add 'if' to top-level expressions". This commit does penance for the latter. The next commits will do penance for the others. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/common.py | 4 ++-- tests/qapi-schema/bad-if-empty-list.err | 1 + tests/qapi-schema/bad-if-empty.err | 1 + tests/qapi-schema/bad-if-list.err | 1 + tests/qapi-schema/bad-if.err | 1 + 5 files changed, 6 insertions(+), 2 deletions(-)