qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members
Date: Tue,  1 Dec 2015 22:20:57 -0700	[thread overview]
Message-ID: <1449033659-25497-14-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1449033659-25497-1-git-send-email-eblake@redhat.com>

We document that members of enums and objects should be
'lower-case', although we were not enforcing it.  We have to
whitelist a few pre-existing entities that violate the norms.
Add three new tests to expose the new error message, each of
which first uses the whitelisted name 'UuidInfo' to prove the
whitelist works, then triggers the failure (this is the same
pattern used in the existing returns-whitelist.json test).

Note that by adding this check, we have effectively forbidden
an entity with a case-insensitive clash of member names, for
any entity that is not on the whitelist (although there is
still the possibility to clash via '-' vs. '_').

Not done here: a future patch should also add naming convention
support and whitelist exceptions for command, event, and type
names.

The additions to QAPISchemaMember.check_clash() check whether
info['name'] is in the whitelist (the top-most entity name at
the point 'info' tracks), rather than self.owner (the type,
possibly implicit, that directly owns the member), because it
is easier to maintain the whitelist by the names actually in
the user's .json file, rather than worrying about the names
of implicit types.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v14: improved commit message, add comments justifying each
whitelist member
v13: new patch
---
 scripts/qapi.py                          | 19 +++++++++++++++++++
 tests/Makefile                           |  3 +++
 tests/qapi-schema/args-member-case.err   |  1 +
 tests/qapi-schema/args-member-case.exit  |  1 +
 tests/qapi-schema/args-member-case.json  |  3 +++
 tests/qapi-schema/args-member-case.out   |  0
 tests/qapi-schema/enum-member-case.err   |  1 +
 tests/qapi-schema/enum-member-case.exit  |  1 +
 tests/qapi-schema/enum-member-case.json  |  3 +++
 tests/qapi-schema/enum-member-case.out   |  0
 tests/qapi-schema/union-branch-case.err  |  1 +
 tests/qapi-schema/union-branch-case.exit |  1 +
 tests/qapi-schema/union-branch-case.json |  3 +++
 tests/qapi-schema/union-branch-case.out  |  0
 14 files changed, 37 insertions(+)
 create mode 100644 tests/qapi-schema/args-member-case.err
 create mode 100644 tests/qapi-schema/args-member-case.exit
 create mode 100644 tests/qapi-schema/args-member-case.json
 create mode 100644 tests/qapi-schema/args-member-case.out
 create mode 100644 tests/qapi-schema/enum-member-case.err
 create mode 100644 tests/qapi-schema/enum-member-case.exit
 create mode 100644 tests/qapi-schema/enum-member-case.json
 create mode 100644 tests/qapi-schema/enum-member-case.out
 create mode 100644 tests/qapi-schema/union-branch-case.err
 create mode 100644 tests/qapi-schema/union-branch-case.exit
 create mode 100644 tests/qapi-schema/union-branch-case.json
 create mode 100644 tests/qapi-schema/union-branch-case.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7de405c..3fc1ff5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -59,6 +59,21 @@ returns_whitelist = [
     'guest-sync-delimited',
 ]

+# Whitelist of entities allowed to violate case conventions
+case_whitelist = [
+    # From QMP:
+    'ACPISlotType',         # DIMM, visible through query-acpi-ospm-status
+    'CpuInfo',              # CPU, PC, visible through query-cpu
+    'CpuInfoBase',          # CPU, visible through query-cpu
+    'CpuInfoMIPS',          # PC, visible through query-cpu
+    'CpuInfoTricore',       # PC, visible through query-cpu
+    'InputAxis',            # TODO: drop when x-input-send-event is fixed
+    'InputButton',          # TODO: drop when x-input-send-event is fixed
+    'QapiErrorClass',       # all members, visible through errors
+    'UuidInfo',             # UUID, visible through query-uuid
+    'X86CPURegister32',     # all members, visible indirectly through qom-get
+]
+
 enum_types = []
 struct_types = []
 union_types = []
@@ -1038,6 +1053,10 @@ class QAPISchemaMember(object):

     def check_clash(self, info, seen):
         cname = c_name(self.name)
+        if cname.lower() != cname and info['name'] not in case_whitelist:
+            raise QAPIExprError(info,
+                                "Member '%s' of '%s' should use lowercase"
+                                % (self.name, info['name']))
         if cname in seen:
             raise QAPIExprError(info,
                                 "%s collides with %s"
diff --git a/tests/Makefile b/tests/Makefile
index 5d83fc6..c3fd3c1 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -246,6 +246,7 @@ qapi-schema += args-array-unknown.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
+qapi-schema += args-member-case.json
 qapi-schema += args-member-unknown.json
 qapi-schema += args-name-clash.json
 qapi-schema += args-union.json
@@ -267,6 +268,7 @@ qapi-schema += enum-bad-prefix.json
 qapi-schema += enum-clash-member.json
 qapi-schema += enum-dict-member.json
 qapi-schema += enum-int-member.json
+qapi-schema += enum-member-case.json
 qapi-schema += enum-missing-data.json
 qapi-schema += enum-wrong-data.json
 qapi-schema += escape-outside-string.json
@@ -341,6 +343,7 @@ 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-branch-case.json
 qapi-schema += union-clash-branches.json
 qapi-schema += union-clash-data.json
 qapi-schema += union-empty.json
diff --git a/tests/qapi-schema/args-member-case.err b/tests/qapi-schema/args-member-case.err
new file mode 100644
index 0000000..7bace48
--- /dev/null
+++ b/tests/qapi-schema/args-member-case.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-member-case.json:3: Member 'Arg' of 'Foo' should use lowercase
diff --git a/tests/qapi-schema/args-member-case.exit b/tests/qapi-schema/args-member-case.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-member-case.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-member-case.json b/tests/qapi-schema/args-member-case.json
new file mode 100644
index 0000000..1bc823a
--- /dev/null
+++ b/tests/qapi-schema/args-member-case.json
@@ -0,0 +1,3 @@
+# Member names should be 'lower-case' unless the struct/command is whitelisted
+{ 'command': 'UuidInfo', 'data': { 'Arg': 'int' } }
+{ 'command': 'Foo', 'data': { 'Arg': 'int' } }
diff --git a/tests/qapi-schema/args-member-case.out b/tests/qapi-schema/args-member-case.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err
new file mode 100644
index 0000000..e50b12a
--- /dev/null
+++ b/tests/qapi-schema/enum-member-case.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-member-case.json:3: Member 'Value' of 'Foo' should use lowercase
diff --git a/tests/qapi-schema/enum-member-case.exit b/tests/qapi-schema/enum-member-case.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/enum-member-case.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/enum-member-case.json b/tests/qapi-schema/enum-member-case.json
new file mode 100644
index 0000000..5101275
--- /dev/null
+++ b/tests/qapi-schema/enum-member-case.json
@@ -0,0 +1,3 @@
+# Member names should be 'lower-case' unless the enum is whitelisted
+{ 'enum': 'UuidInfo', 'data': [ 'Value' ] }
+{ 'enum': 'Foo', 'data': [ 'Value' ] }
diff --git a/tests/qapi-schema/enum-member-case.out b/tests/qapi-schema/enum-member-case.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-branch-case.err b/tests/qapi-schema/union-branch-case.err
new file mode 100644
index 0000000..6c6b740
--- /dev/null
+++ b/tests/qapi-schema/union-branch-case.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-branch-case.json:3: Member 'Branch' of 'Foo' should use lowercase
diff --git a/tests/qapi-schema/union-branch-case.exit b/tests/qapi-schema/union-branch-case.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-branch-case.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-branch-case.json b/tests/qapi-schema/union-branch-case.json
new file mode 100644
index 0000000..a5951f1
--- /dev/null
+++ b/tests/qapi-schema/union-branch-case.json
@@ -0,0 +1,3 @@
+# Branch names should be 'lower-case' unless the union is whitelisted
+{ 'union': 'UuidInfo', 'data': { 'Branch': 'int' } }
+{ 'union': 'Foo', 'data': { 'Branch': 'int' } }
diff --git a/tests/qapi-schema/union-branch-case.out b/tests/qapi-schema/union-branch-case.out
new file mode 100644
index 0000000..e69de29
-- 
2.4.3

  parent reply	other threads:[~2015-12-02  5:21 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-02  5:20 [Qemu-devel] [PATCH v14 00/15] qapi member collision (post-introspection cleanups, subset D) Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 01/15] qobject: Simplify QObject Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 02/15] qobject: Rename qtype_code to QType Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 03/15] qapi: Convert QType into QAPI built-in enum type Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 04/15] qapi: Simplify visiting of alternate types Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 05/15] qapi-types: Drop unnedeed ._fwdefn Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 06/15] qapi: Inline _make_implicit_tag() Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 07/15] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 08/15] qapi: Simplify visits of optional fields Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 09/15] qapi: Shorter " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 10/15] qapi: Prepare new QAPISchemaMember base class Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 11/15] qapi: Track enum values by QAPISchemaMember, not string Eric Blake
2015-12-02 15:52   ` Markus Armbruster
2015-12-02 16:09     ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 12/15] qapi: Populate info['name'] for each entity Eric Blake
2015-12-02  5:20 ` Eric Blake [this message]
2015-12-02 11:51   ` [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members Markus Armbruster
2015-12-02 13:41     ` Eric Blake
2015-12-02 15:19       ` Eric Blake
2015-12-02 17:19         ` Markus Armbruster
2015-12-02 19:43           ` Eric Blake
2015-12-02 20:27             ` Markus Armbruster
2015-12-02 16:48       ` Markus Armbruster
2015-12-02 14:11     ` Eric Blake
2015-12-02 15:30     ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 14/15] qapi: Move duplicate collision checks to schema check() Eric Blake
2015-12-02  5:20 ` [Qemu-devel] [PATCH v14 15/15] qapi: Detect base class loops Eric Blake
2015-12-02 17:57 ` [Qemu-devel] [PATCH v14 00/15] qapi member collision (post-introspection cleanups, subset D) Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1449033659-25497-14-git-send-email-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).