qapi: Simplify error reporting for array types

check_type() first checks and peels off the array type, then checks
the element type.  For two out of four error messages, it takes pains
to report errors for "array of T" instead of just T.  Odd.  Let's
examine the errors.

* Unknown element type, e.g.
  tests/qapi-schema/args-array-unknown.json:

      Member 'array' of 'data' for command 'oops' uses unknown type
      'array of NoSuchType'

  To make sense of this, you need to know that 'array of NoSuchType'
  refers to '[NoSuchType]'.  Easy enough.  However, simply reporting

      Member 'array' of 'data' for command 'oops' uses unknown type
      'NoSuchType'

  is at least as easy to understand.

* Element type's meta-type is inadmissible, e.g.
  tests/qapi-schema/returns-whitelist.json:

      'returns' for command 'no-way-this-will-get-whitelisted' cannot
      use built-in type 'array of int'

  'array of int' is technically not a built-in type, but that's
  pedantry.  However, simply reporting

      'returns' for command 'no-way-this-will-get-whitelisted' cannot
      use built-in type 'int'

  avoids the issue, and is at least as easy to understand.

* The remaining two errors are unreachable, because the array checking
  ensures that value is a string.

Thus, reporting some errors for "array of T" instead of just T works,
but doesn't really improve things.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 36560590ba0324723c357f8d3b9e76c771a0b507..ac0e45e2e63808a26ca6aa918da74757d6be37e9 100644 (file)
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -426,7 +426,6 @@ def check_type(expr_info, source, value, allow_array = False,
                allow_dict = False, allow_optional = False,
                allow_star = False, allow_metas = []):
     global all_names
-    orig_value = value
 
     if value is None:
         return
@@ -444,7 +443,6 @@ def check_type(expr_info, source, value, allow_array = False,
                                 "%s: array type must contain single type name"
                                 % source)
         value = value[0]
-        orig_value = "array of %s" %value
 
     # Check if type name for value is okay
     if isinstance(value, str):
@@ -455,11 +453,11 @@ def check_type(expr_info, source, value, allow_array = False,
         if not value in all_names:
             raise QAPIExprError(expr_info,
                                 "%s uses unknown type '%s'"
-                                % (source, orig_value))
+                                % (source, value))
         if not all_names[value] in allow_metas:
             raise QAPIExprError(expr_info,
                                 "%s cannot use %s type '%s'"
-                                % (source, all_names[value], orig_value))
+                                % (source, all_names[value], value))
         return
 
     if not allow_dict:

diff --git a/tests/qapi-schema/args-array-unknown.err b/tests/qapi-schema/args-array-unknown.err
index 7834d11bc11c5019e6f94f523be556b8096ab0cb..cd7a0f98d799a55948dd8f1bc6aff4fe44ca9cb4 100644 (file)
--- a/tests/qapi-schema/args-array-unknown.err
+++ b/tests/qapi-schema/args-array-unknown.err
@@ -1 +1 @@
-tests/qapi-schema/args-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'array of NoSuchType'
+tests/qapi-schema/args-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'NoSuchType'

diff --git a/tests/qapi-schema/returns-whitelist.err b/tests/qapi-schema/returns-whitelist.err
index a41f019a52e445e5a7087a275eabfe5ce9039c30..f47c1ee7ca4a0ed05fb3bc651df094d1137dc5e9 100644 (file)
--- a/tests/qapi-schema/returns-whitelist.err
+++ b/tests/qapi-schema/returns-whitelist.err
@@ -1 +1 @@
-tests/qapi-schema/returns-whitelist.json:10: 'returns' for command 'no-way-this-will-get-whitelisted' cannot use built-in type 'array of int'
+tests/qapi-schema/returns-whitelist.json:10: 'returns' for command 'no-way-this-will-get-whitelisted' cannot use built-in type 'int'