diff mbox series

[v2,22/26] qapi: Improve reporting of invalid 'if' further

Message ID 20190927134639.4284-23-armbru@redhat.com
State New
Headers show
Series qapi: Pay back some frontend technical debt | expand

Commit Message

Markus Armbruster Sept. 27, 2019, 1:46 p.m. UTC 
check_if()'s errors don't point to the offending part of the
expression.  For instance:

    tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes no sense

Other check_FOO() do, with the help of a @source argument.  Make
check_if() do that, too.  The example above improves to:

    tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' makes no sense

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py                        | 27 +++++++++++--------
 .../alternate-branch-if-invalid.err           |  2 +-
 tests/qapi-schema/bad-if-empty-list.err       |  2 +-
 tests/qapi-schema/bad-if-empty.err            |  2 +-
 tests/qapi-schema/bad-if-list.err             |  2 +-
 tests/qapi-schema/bad-if.err                  |  2 +-
 tests/qapi-schema/enum-if-invalid.err         |  2 +-
 tests/qapi-schema/features-if-invalid.err     |  2 +-
 .../qapi-schema/struct-member-if-invalid.err  |  2 +-
 tests/qapi-schema/union-branch-if-invalid.err |  2 +-
 10 files changed, 25 insertions(+), 20 deletions(-)

Comments

Eric Blake Sept. 27, 2019, 4:08 p.m. UTC | #1 
On 9/27/19 8:46 AM, Markus Armbruster wrote:
> check_if()'s errors don't point to the offending part of the
> expression.  For instance:
> 
>      tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes no sense
> 
> Other check_FOO() do, with the help of a @source argument.  Make
> check_if() do that, too.  The example above improves to:
> 
>      tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' makes no sense
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 81c217cd60..4bc8c807aa 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -692,22 +692,27 @@  def check_defn_name_str(name, info, meta):
             info, "%s name should not end in '%s'" % (meta, name[-4:]))
 
 
-def check_if(expr, info):
+def check_if(expr, info, source):
 
     def check_if_str(ifcond, info):
         if not isinstance(ifcond, str):
             raise QAPISemError(
-                info, "'if' condition must be a string or a list of strings")
+                info,
+                "'if' condition of %s must be a string or a list of strings"
+                % source)
         if ifcond.strip() == '':
-            raise QAPISemError(info, "'if' condition '%s' makes no sense"
-                               % ifcond)
+            raise QAPISemError(
+                info,
+                "'if' condition '%s' of %s makes no sense"
+                % (ifcond, source))
 
     ifcond = expr.get('if')
     if ifcond is None:
         return
     if isinstance(ifcond, list):
         if ifcond == []:
-            raise QAPISemError(info, "'if' condition [] is useless")
+            raise QAPISemError(
+                info, "'if' condition [] of %s is useless" % source)
         for elt in ifcond:
             check_if_str(elt, info)
     else:
@@ -752,7 +757,7 @@  def check_type(value, info, source,
         if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
             raise QAPISemError(info, "%s uses reserved name" % key_source)
         check_known_keys(arg, info, key_source, ['type'], ['if'])
-        check_if(arg, info)
+        check_if(arg, info, key_source)
         normalize_if(arg)
         check_type(arg['type'], info, key_source, allow_array=True)
 
@@ -796,7 +801,7 @@  def check_union(expr, info):
         source = "'data' member '%s'" % key
         check_name_str(key, info, source)
         check_known_keys(value, info, source, ['type'], ['if'])
-        check_if(value, info)
+        check_if(value, info, source)
         normalize_if(value)
         check_type(value['type'], info, source, allow_array=not base)
 
@@ -810,7 +815,7 @@  def check_alternate(expr, info):
         source = "'data' member '%s'" % key
         check_name_str(key, info, source)
         check_known_keys(value, info, source, ['type'], ['if'])
-        check_if(value, info)
+        check_if(value, info, source)
         normalize_if(value)
         check_type(value['type'], info, source)
 
@@ -834,7 +839,7 @@  def check_enum(expr, info):
         source = "%s '%s'" % (source, member['name'])
         check_name_str(member['name'], info, source,
                        enum_member=True, permit_upper=permit_upper)
-        check_if(member, info)
+        check_if(member, info, source)
         normalize_if(member)
 
 
@@ -856,7 +861,7 @@  def check_struct(expr, info):
             check_name_is_str(f['name'], info, source)
             source = "%s '%s'" % (source, f['name'])
             check_name_str(f['name'], info, source)
-            check_if(f, info)
+            check_if(f, info, source)
             normalize_if(f)
 
 
@@ -994,7 +999,7 @@  def check_exprs(exprs):
             assert False, 'unexpected meta type'
 
         normalize_if(expr)
-        check_if(expr, info)
+        check_if(expr, info, meta)
         check_flags(expr, info)
 
         if doc:
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err b/tests/qapi-schema/alternate-branch-if-invalid.err
index 8684829aca..6c68e5a922 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.err
+++ b/tests/qapi-schema/alternate-branch-if-invalid.err
@@ -1,2 +1,2 @@ 
 tests/qapi-schema/alternate-branch-if-invalid.json: In alternate 'Alt':
-tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes no sense
+tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' makes no sense
diff --git a/tests/qapi-schema/bad-if-empty-list.err b/tests/qapi-schema/bad-if-empty-list.err
index 2218c9279b..517519f500 100644
--- a/tests/qapi-schema/bad-if-empty-list.err
+++ b/tests/qapi-schema/bad-if-empty-list.err
@@ -1,2 +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
+tests/qapi-schema/bad-if-empty-list.json:2: 'if' condition [] of struct is useless
diff --git a/tests/qapi-schema/bad-if-empty.err b/tests/qapi-schema/bad-if-empty.err
index a3fdb3009d..5f1767388e 100644
--- a/tests/qapi-schema/bad-if-empty.err
+++ b/tests/qapi-schema/bad-if-empty.err
@@ -1,2 +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
+tests/qapi-schema/bad-if-empty.json:2: 'if' condition '' of struct makes no sense
diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err
index e03bf0fc3a..e5d72b2f39 100644
--- a/tests/qapi-schema/bad-if-list.err
+++ b/tests/qapi-schema/bad-if-list.err
@@ -1,2 +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
+tests/qapi-schema/bad-if-list.json:2: 'if' condition ' ' of struct makes no sense
diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
index 190216c109..65d8efd7e4 100644
--- a/tests/qapi-schema/bad-if.err
+++ b/tests/qapi-schema/bad-if.err
@@ -1,2 +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
+tests/qapi-schema/bad-if.json:2: 'if' condition of struct must be a string or a list of strings
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index db9eb45b25..30c1f0e91c 100644
--- a/tests/qapi-schema/enum-if-invalid.err
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -1,2 +1,2 @@ 
 tests/qapi-schema/enum-if-invalid.json: In enum 'TestIfEnum':
-tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
+tests/qapi-schema/enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' must be a string or a list of strings
diff --git a/tests/qapi-schema/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
index 90f4119ae7..ffb39378af 100644
--- a/tests/qapi-schema/features-if-invalid.err
+++ b/tests/qapi-schema/features-if-invalid.err
@@ -1,2 +1,2 @@ 
 tests/qapi-schema/features-if-invalid.json: In struct 'Stru':
-tests/qapi-schema/features-if-invalid.json:2: 'if' condition must be a string or a list of strings
+tests/qapi-schema/features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a list of strings
diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
index e8ad02b9fc..4c5983674b 100644
--- a/tests/qapi-schema/struct-member-if-invalid.err
+++ b/tests/qapi-schema/struct-member-if-invalid.err
@@ -1,2 +1,2 @@ 
 tests/qapi-schema/struct-member-if-invalid.json: In struct 'Stru':
-tests/qapi-schema/struct-member-if-invalid.json:2: 'if' condition must be a string or a list of strings
+tests/qapi-schema/struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a list of strings
diff --git a/tests/qapi-schema/union-branch-if-invalid.err b/tests/qapi-schema/union-branch-if-invalid.err
index b49cf9b664..14819bf8b8 100644
--- a/tests/qapi-schema/union-branch-if-invalid.err
+++ b/tests/qapi-schema/union-branch-if-invalid.err
@@ -1,2 +1,2 @@ 
 tests/qapi-schema/union-branch-if-invalid.json: In union 'Uni':
-tests/qapi-schema/union-branch-if-invalid.json:4: 'if' condition '' makes no sense
+tests/qapi-schema/union-branch-if-invalid.json:4: 'if' condition '' of 'data' member 'branch1' makes no sense