[Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw

[Markus Armbruster <armbru@redhat.com>]

Reproducer: with

    { 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } }

added to qapi-schema-test.json, qapi-commands.py dies when it tries to
generate the command handler function

    Traceback (most recent call last):
      File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in <module>
        ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
      File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in generate_command_decl
        ret_type=c_type(ret_type), name=c_name(name),
      File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type
        assert isinstance(value, str) and value != ""
    AssertionError

because the return type doesn't exist.

Simply outlaw this usage.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt                                  | 17 ++++++++---------
 scripts/qapi.py                                         |  2 +-
 tests/Makefile                                          |  4 ++--
 tests/qapi-schema/nested-struct-returns.err             |  1 -
 tests/qapi-schema/nested-struct-returns.json            |  3 ---
 tests/qapi-schema/returns-dict.err                      |  1 +
 .../{nested-struct-returns.exit => returns-dict.exit}   |  0
 tests/qapi-schema/returns-dict.json                     |  2 ++
 .../{nested-struct-returns.out => returns-dict.out}     |  0
 9 files changed, 14 insertions(+), 16 deletions(-)
 delete mode 100644 tests/qapi-schema/nested-struct-returns.err
 delete mode 100644 tests/qapi-schema/nested-struct-returns.json
 create mode 100644 tests/qapi-schema/returns-dict.err
 rename tests/qapi-schema/{nested-struct-returns.exit => returns-dict.exit} (100%)
 create mode 100644 tests/qapi-schema/returns-dict.json
 rename tests/qapi-schema/{nested-struct-returns.out => returns-dict.out} (100%)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index c2ac21c..279c681 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -394,7 +394,7 @@ following example objects:
 === Commands ===
 
 Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
-         '*returns': TYPE-NAME-OR-DICT,
+         '*returns': TYPE-NAME,
          '*gen': false, '*success-response': false }
 
 Commands are defined by using a dictionary containing several members,
@@ -416,14 +416,13 @@ The member is optional from the command declaration; if absent, the
 "return" field will be an empty dictionary.  If 'returns' is present,
 it must be the string name of a complex or built-in type, a
 one-element array containing the name of a complex or built-in type,
-or a dictionary that declares an anonymous type with the same
-semantics as a 'struct' expression, with one exception noted below
-when 'gen' is used.  Although it is permitted to have the 'returns'
-member name a built-in type or an array of built-in types, any command
-that does this cannot be extended to return additional information in
-the future; thus, new commands should strongly consider returning a
-dictionary-based type or an array of dictionaries, even if the
-dictionary only contains one field at the present.
+with one exception noted below when 'gen' is used.  Although it is
+permitted to have the 'returns' member name a built-in type or an
+array of built-in types, any command that does this cannot be extended
+to return additional information in the future; thus, new commands
+should strongly consider returning a dictionary-based type or an array
+of dictionaries, even if the dictionary only contains one field at the
+present.
 
 All commands in Client JSON Protocol use a dictionary to report
 failure, with no way to specify that in QAPI.  Where the error return
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ec7923a..56efb18 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -504,7 +504,7 @@ def check_command(expr, expr_info):
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
     check_type(expr_info, "'returns' for command '%s'" % name,
-               expr.get('returns'), allow_array=True, allow_dict=True,
+               expr.get('returns'), allow_array=True,
                allow_optional=True, allow_metas=returns_meta,
                allow_star=allow_star)
 
diff --git a/tests/Makefile b/tests/Makefile
index fd0ae45..2d6ca9d 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -232,10 +232,10 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	args-array-empty.json args-array-unknown.json args-int.json \
 	args-unknown.json args-member-unknown.json args-member-array.json \
 	args-member-array-bad.json args-alternate.json args-union.json \
-	returns-array-bad.json returns-int.json \
+	returns-array-bad.json returns-int.json returns-dict.json \
 	returns-unknown.json returns-alternate.json returns-whitelist.json \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
-	nested-struct-data.json nested-struct-returns.json non-objects.json \
+	nested-struct-data.json non-objects.json \
 	qapi-schema-test.json quoted-structural-chars.json \
 	trailing-comma-list.json trailing-comma-object.json \
 	unclosed-list.json unclosed-object.json unclosed-string.json \
diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err
deleted file mode 100644
index 5238d07..0000000
--- a/tests/qapi-schema/nested-struct-returns.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/nested-struct-returns.json:2: Member 'a' of 'returns' for command 'foo' should be a type name
diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json
deleted file mode 100644
index d2cd047..0000000
--- a/tests/qapi-schema/nested-struct-returns.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# inline subtypes collide with our desired future use of defaults
-{ 'command': 'foo',
-  'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/returns-dict.err b/tests/qapi-schema/returns-dict.err
new file mode 100644
index 0000000..eb2d0c4
--- /dev/null
+++ b/tests/qapi-schema/returns-dict.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-dict.json:2: 'returns' for command 'oops' should be a type name
diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/returns-dict.exit
similarity index 100%
rename from tests/qapi-schema/nested-struct-returns.exit
rename to tests/qapi-schema/returns-dict.exit
diff --git a/tests/qapi-schema/returns-dict.json b/tests/qapi-schema/returns-dict.json
new file mode 100644
index 0000000..1cfef3e
--- /dev/null
+++ b/tests/qapi-schema/returns-dict.json
@@ -0,0 +1,2 @@
+# we reject inline struct return type
+{ 'command': 'oops', 'returns': { 'a': 'str' } }
diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/returns-dict.out
similarity index 100%
rename from tests/qapi-schema/nested-struct-returns.out
rename to tests/qapi-schema/returns-dict.out
-- 
2.4.3