[Qemu-devel] [PATCH v5 03/46] qapi: Test for C member name collisions

[Eric Blake <eblake@redhat.com>]

Expose some weaknesses in the generator: we don't always forbid
the generation of structs that contain multiple members that map
to the same C name.  This has already been marked FIXME in
qapi.py, but having more tests will make sure future patches
produce desired behavior.

Some of these tests will be deleted later, and a positive test
added to qapi-schema-test.json in its place, when the code is
reworked so that the collision no longer occurs.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                                  |  6 ++++++
 tests/qapi-schema/args-name-clash.err           |  0
 tests/qapi-schema/args-name-clash.exit          |  1 +
 tests/qapi-schema/args-name-clash.json          |  2 ++
 tests/qapi-schema/args-name-clash.out           |  6 ++++++
 tests/qapi-schema/flat-union-branch-clash.json  |  2 +-
 tests/qapi-schema/flat-union-branch-clash2.err  |  0
 tests/qapi-schema/flat-union-branch-clash2.exit |  1 +
 tests/qapi-schema/flat-union-branch-clash2.json | 14 ++++++++++++++
 tests/qapi-schema/flat-union-branch-clash2.out  | 14 ++++++++++++++
 tests/qapi-schema/flat-union-cycle.err          |  1 +
 tests/qapi-schema/flat-union-cycle.exit         |  1 +
 tests/qapi-schema/flat-union-cycle.json         |  6 ++++++
 tests/qapi-schema/flat-union-cycle.out          |  0
 tests/qapi-schema/qapi-schema-test.json         |  5 +++--
 tests/qapi-schema/qapi-schema-test.out          |  2 ++
 tests/qapi-schema/struct-base-clash2.err        |  0
 tests/qapi-schema/struct-base-clash2.exit       |  1 +
 tests/qapi-schema/struct-base-clash2.json       |  5 +++++
 tests/qapi-schema/struct-base-clash2.out        |  5 +++++
 tests/qapi-schema/union-clash.err               |  1 +
 tests/qapi-schema/union-clash.exit              |  1 +
 tests/qapi-schema/union-clash.json              |  3 +++
 tests/qapi-schema/union-clash.out               |  0
 tests/qapi-schema/union-clash2.err              |  0
 tests/qapi-schema/union-clash2.exit             |  1 +
 tests/qapi-schema/union-clash2.json             |  3 +++
 tests/qapi-schema/union-clash2.out              |  6 ++++++
 28 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 tests/qapi-schema/args-name-clash.err
 create mode 100644 tests/qapi-schema/args-name-clash.exit
 create mode 100644 tests/qapi-schema/args-name-clash.json
 create mode 100644 tests/qapi-schema/args-name-clash.out
 create mode 100644 tests/qapi-schema/flat-union-branch-clash2.err
 create mode 100644 tests/qapi-schema/flat-union-branch-clash2.exit
 create mode 100644 tests/qapi-schema/flat-union-branch-clash2.json
 create mode 100644 tests/qapi-schema/flat-union-branch-clash2.out
 create mode 100644 tests/qapi-schema/flat-union-cycle.err
 create mode 100644 tests/qapi-schema/flat-union-cycle.exit
 create mode 100644 tests/qapi-schema/flat-union-cycle.json
 create mode 100644 tests/qapi-schema/flat-union-cycle.out
 create mode 100644 tests/qapi-schema/struct-base-clash2.err
 create mode 100644 tests/qapi-schema/struct-base-clash2.exit
 create mode 100644 tests/qapi-schema/struct-base-clash2.json
 create mode 100644 tests/qapi-schema/struct-base-clash2.out
 create mode 100644 tests/qapi-schema/union-clash.err
 create mode 100644 tests/qapi-schema/union-clash.exit
 create mode 100644 tests/qapi-schema/union-clash.json
 create mode 100644 tests/qapi-schema/union-clash.out
 create mode 100644 tests/qapi-schema/union-clash2.err
 create mode 100644 tests/qapi-schema/union-clash2.exit
 create mode 100644 tests/qapi-schema/union-clash2.json
 create mode 100644 tests/qapi-schema/union-clash2.out

diff --git a/tests/Makefile b/tests/Makefile
index 6fd5885..97434f6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -238,6 +238,7 @@ qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
 qapi-schema += args-member-array.json
 qapi-schema += args-member-unknown.json
+qapi-schema += args-name-clash.json
 qapi-schema += args-union.json
 qapi-schema += args-unknown.json
 qapi-schema += bad-base.json
@@ -274,6 +275,8 @@ qapi-schema += flat-union-bad-discriminator.json
 qapi-schema += flat-union-base-any.json
 qapi-schema += flat-union-base-union.json
 qapi-schema += flat-union-branch-clash.json
+qapi-schema += flat-union-branch-clash2.json
+qapi-schema += flat-union-cycle.json
 qapi-schema += flat-union-inline.json
 qapi-schema += flat-union-int-branch.json
 qapi-schema += flat-union-invalid-branch-key.json
@@ -317,6 +320,7 @@ qapi-schema += returns-unknown.json
 qapi-schema += returns-whitelist.json
 qapi-schema += struct-base-clash-deep.json
 qapi-schema += struct-base-clash.json
+qapi-schema += struct-base-clash2.json
 qapi-schema += struct-data-invalid.json
 qapi-schema += struct-member-invalid.json
 qapi-schema += trailing-comma-list.json
@@ -328,6 +332,8 @@ qapi-schema += unclosed-string.json
 qapi-schema += unicode-str.json
 qapi-schema += union-bad-branch.json
 qapi-schema += union-base-no-discriminator.json
+qapi-schema += union-clash.json
+qapi-schema += union-clash2.json
 qapi-schema += union-invalid-base.json
 qapi-schema += union-max.json
 qapi-schema += union-optional-branch.json
diff --git a/tests/qapi-schema/args-name-clash.err b/tests/qapi-schema/args-name-clash.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/args-name-clash.exit b/tests/qapi-schema/args-name-clash.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/args-name-clash.json
new file mode 100644
index 0000000..19bf792
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.json
@@ -0,0 +1,2 @@
+# FIXME - we should reject data with members that clash when mapped to C names
+{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/args-name-clash.out
new file mode 100644
index 0000000..9b2f6e4
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.out
@@ -0,0 +1,6 @@
+object :empty
+object :obj-oops-arg
+    member a-b: str optional=False
+    member a_b: str optional=False
+command oops :obj-oops-arg -> None
+   gen=True success_response=True
diff --git a/tests/qapi-schema/flat-union-branch-clash.json b/tests/qapi-schema/flat-union-branch-clash.json
index 8fb054f..3d7e6c6 100644
--- a/tests/qapi-schema/flat-union-branch-clash.json
+++ b/tests/qapi-schema/flat-union-branch-clash.json
@@ -1,4 +1,4 @@
-# we check for no duplicate keys between branches and base
+# we check for no duplicate keys between branch members and base
 { 'enum': 'TestEnum',
   'data': [ 'value1', 'value2' ] }
 { 'struct': 'Base',
diff --git a/tests/qapi-schema/flat-union-branch-clash2.err b/tests/qapi-schema/flat-union-branch-clash2.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-branch-clash2.exit b/tests/qapi-schema/flat-union-branch-clash2.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-branch-clash2.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-branch-clash2.json b/tests/qapi-schema/flat-union-branch-clash2.json
new file mode 100644
index 0000000..b3eefb3
--- /dev/null
+++ b/tests/qapi-schema/flat-union-branch-clash2.json
@@ -0,0 +1,14 @@
+# FIXME: we should check for no duplicate C names between branches and base
+{ 'enum': 'TestEnum',
+  'data': [ 'base', 'c-d' ] }
+{ 'struct': 'Base',
+  'data': { 'enum1': 'TestEnum', '*c_d': 'str' } }
+{ 'struct': 'Branch1',
+  'data': { 'string': 'str' } }
+{ 'struct': 'Branch2',
+  'data': { 'value': 'int' } }
+{ 'union': 'TestUnion',
+  'base': 'Base',
+  'discriminator': 'enum1',
+  'data': { 'base': 'Branch1',
+            'c-d': 'Branch2' } }
diff --git a/tests/qapi-schema/flat-union-branch-clash2.out b/tests/qapi-schema/flat-union-branch-clash2.out
new file mode 100644
index 0000000..8e0da73
--- /dev/null
+++ b/tests/qapi-schema/flat-union-branch-clash2.out
@@ -0,0 +1,14 @@
+object :empty
+object Base
+    member enum1: TestEnum optional=False
+    member c_d: str optional=True
+object Branch1
+    member string: str optional=False
+object Branch2
+    member value: int optional=False
+enum TestEnum ['base', 'c-d']
+object TestUnion
+    base Base
+    tag enum1
+    case base: Branch1
+    case c-d: Branch2
diff --git a/tests/qapi-schema/flat-union-cycle.err b/tests/qapi-schema/flat-union-cycle.err
new file mode 100644
index 0000000..152c6f0
--- /dev/null
+++ b/tests/qapi-schema/flat-union-cycle.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-cycle.json:5: Member name 'switch' of branch 'loop' clashes with base 'Base'
diff --git a/tests/qapi-schema/flat-union-cycle.exit b/tests/qapi-schema/flat-union-cycle.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-cycle.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-cycle.json b/tests/qapi-schema/flat-union-cycle.json
new file mode 100644
index 0000000..8f6cd93
--- /dev/null
+++ b/tests/qapi-schema/flat-union-cycle.json
@@ -0,0 +1,6 @@
+# we reject a loop in flat unions, due to member collision
+{ 'enum': 'Enum', 'data': [ 'okay', 'loop' ] }
+{ 'struct': 'Base', 'data': { 'switch': 'Enum' } }
+{ 'struct': 'Okay', 'data': { 'int': 'int' } }
+{ 'union': 'Union', 'base': 'Base', 'discriminator': 'switch',
+  'data': { 'okay': 'Okay', 'loop': 'Base' } }
diff --git a/tests/qapi-schema/flat-union-cycle.out b/tests/qapi-schema/flat-union-cycle.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6897a6e..c904db4 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -32,11 +32,12 @@
             'dict1': 'UserDefTwoDict' } }

 # for testing unions
+# name collisions between branches should not clash
 { 'struct': 'UserDefA',
-  'data': { 'boolean': 'bool' } }
+  'data': { 'boolean': 'bool', '*a_b': 'int' } }

 { 'struct': 'UserDefB',
-  'data': { 'intb': 'int' } }
+  'data': { 'intb': 'int', '*a-b': 'bool' } }

 { 'union': 'UserDefFlatUnion',
   'base': 'UserDefUnionBase',   # intentional forward reference
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 1f6e858..28a0b3c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -71,6 +71,7 @@ enum QEnumTwo ['value1', 'value2']
     prefix QENUM_TWO
 object UserDefA
     member boolean: bool optional=False
+    member a_b: int optional=True
 alternate UserDefAlternate
     case uda: UserDefA
     case s: str
@@ -78,6 +79,7 @@ alternate UserDefAlternate
 enum UserDefAlternateKind ['uda', 's', 'i']
 object UserDefB
     member intb: int optional=False
+    member a-b: bool optional=True
 object UserDefC
     member string1: str optional=False
     member string2: str optional=False
diff --git a/tests/qapi-schema/struct-base-clash2.err b/tests/qapi-schema/struct-base-clash2.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/struct-base-clash2.exit b/tests/qapi-schema/struct-base-clash2.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash2.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/struct-base-clash2.json b/tests/qapi-schema/struct-base-clash2.json
new file mode 100644
index 0000000..56166e0
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash2.json
@@ -0,0 +1,5 @@
+# FIXME - a base class collides with a member named base
+{ 'struct': 'Base', 'data': {} }
+{ 'struct': 'Sub',
+  'base': 'Base',
+  'data': { 'base': 'str' } }
diff --git a/tests/qapi-schema/struct-base-clash2.out b/tests/qapi-schema/struct-base-clash2.out
new file mode 100644
index 0000000..e69a416
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash2.out
@@ -0,0 +1,5 @@
+object :empty
+object Base
+object Sub
+    base Base
+    member base: str optional=False
diff --git a/tests/qapi-schema/union-clash.err b/tests/qapi-schema/union-clash.err
new file mode 100644
index 0000000..64637ed
--- /dev/null
+++ b/tests/qapi-schema/union-clash.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-clash.json:2: Union 'TestUnion' member 'a_b' clashes with 'a-b'
diff --git a/tests/qapi-schema/union-clash.exit b/tests/qapi-schema/union-clash.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-clash.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-clash.json b/tests/qapi-schema/union-clash.json
new file mode 100644
index 0000000..0393ed8
--- /dev/null
+++ b/tests/qapi-schema/union-clash.json
@@ -0,0 +1,3 @@
+# we reject unions where branch names clash when mapped to C
+{ 'union': 'TestUnion',
+  'data': { 'a-b': 'int', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/union-clash.out b/tests/qapi-schema/union-clash.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-clash2.err b/tests/qapi-schema/union-clash2.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-clash2.exit b/tests/qapi-schema/union-clash2.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/union-clash2.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/union-clash2.json b/tests/qapi-schema/union-clash2.json
new file mode 100644
index 0000000..b2d45fb
--- /dev/null
+++ b/tests/qapi-schema/union-clash2.json
@@ -0,0 +1,3 @@
+# FIXME - a union branch named 'data' collides with generated C code
+{ 'union': 'TestUnion',
+  'data': { 'data': 'int' } }
diff --git a/tests/qapi-schema/union-clash2.out b/tests/qapi-schema/union-clash2.out
new file mode 100644
index 0000000..6277239
--- /dev/null
+++ b/tests/qapi-schema/union-clash2.out
@@ -0,0 +1,6 @@
+object :empty
+object :obj-int-wrapper
+    member data: int optional=False
+object TestUnion
+    case data: :obj-int-wrapper
+enum TestUnionKind ['data']
-- 
2.4.3