qapi: Clean up member name case checking

QAPISchemaMember.check_clash() checks for member names that map to the
same c_name().  Takes care of rejecting duplicate names.

It also checks a naming rule: no uppercase in member names.  That's a
rather odd place to do it.  Enforcing naming rules is
check_name_str()'s job.

qapi-code-gen.txt specifies the name case rule applies to the name as
it appears in the schema.  check_clash() checks c_name(name) instead.
No difference, as c_name() leaves alone case, but unclean.

Move the name case check into check_name_str(), less the c_name().
New argument @permit_upper suppresses it.  Pass permit_upper=True for
definitions (which are not members), and when the member's owner is
whitelisted with pragma name-case-whitelist.

Bonus: name-case-whitelist now applies to a union's inline base, too.
Update qapi/qapi-schema.json pragma to whitelist union CpuInfo instead
of CpuInfo's implicit base type's name q_obj_CpuInfo-base.

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

diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 920b03b0aac1a49fbba756423e1c904c15362ff6..9751b11f8f16e24a0e5d1cfa7ef82e3a3919c798 100644 (file)
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -71,7 +71,7 @@
         'QapiErrorClass',           # all members, visible through errors
         'UuidInfo',                 # UUID, visible through query-uuid
         'X86CPURegister32',         # all members, visible indirectly through qom-get
-        'q_obj_CpuInfo-base'        # CPU, visible through query-cpu
+        'CpuInfo'                   # CPU, visible through query-cpu
     ] } }
 
 # Documentation generated with qapi-gen.py is in source order, with

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f0e7d5ad347cd608b33df2a1d05e93d3e2cb2bee..ed4bff44794dc890ee50f24347faae2c6bf4064b 100644 (file)
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -704,8 +704,8 @@ valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
                         '[a-zA-Z][a-zA-Z0-9_-]*$')
 
 
-def check_name(info, source, name, allow_optional=False,
-               enum_member=False):
+def check_name(info, source, name,
+               allow_optional=False, enum_member=False, permit_upper=False):
     global valid_name
     membername = name
 
@@ -725,11 +725,14 @@ def check_name(info, source, name, allow_optional=False,
     if not valid_name.match(membername) or \
        c_name(membername, False).startswith('q_'):
         raise QAPISemError(info, "%s uses invalid name '%s'" % (source, name))
+    if not permit_upper and name.lower() != name:
+        raise QAPISemError(
+            info, "%s uses uppercase in name '%s'" % (source, name))
 
 
 def add_name(name, info, meta):
     global all_names
-    check_name(info, "'%s'" % meta, name)
+    check_name(info, "'%s'" % meta, name, permit_upper=True)
     # FIXME should reject names that differ only in '_' vs. '.'
     # vs. '-', because they're liable to clash in generated C.
     if name in all_names:
@@ -797,10 +800,12 @@ def check_type(info, source, value,
         raise QAPISemError(info,
                            "%s should be an object or type name" % source)
 
+    permit_upper = allow_dict in name_case_whitelist
+
     # value is a dictionary, check that each member is okay
     for (key, arg) in value.items():
         check_name(info, "Member of %s" % source, key,
-                   allow_optional=True)
+                   allow_optional=True, permit_upper=permit_upper)
         if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
             raise QAPISemError(info, "Member of %s uses reserved name '%s'"
                                % (source, key))
@@ -870,7 +875,7 @@ def check_union(expr, info):
     else:
         # The object must have a string or dictionary 'base'.
         check_type(info, "'base' for union '%s'" % name,
-                   base, allow_dict=True,
+                   base, allow_dict=name,
                    allow_metas=['struct'])
         if not base:
             raise QAPISemError(info, "Flat union '%s' must have a base"
@@ -982,13 +987,15 @@ def check_enum(expr, info):
         raise QAPISemError(info,
                            "Enum '%s' requires a string for 'prefix'" % name)
 
+    permit_upper = name in name_case_whitelist
+
     for member in members:
         check_known_keys(info, "member of enum '%s'" % name, member,
                          ['name'], ['if'])
         check_if(member, info)
         normalize_if(member)
         check_name(info, "Member of enum '%s'" % name, member['name'],
-                   enum_member=True)
+                   enum_member=True, permit_upper=permit_upper)
 
 
 def check_struct(expr, info):
@@ -997,7 +1004,7 @@ def check_struct(expr, info):
     features = expr.get('features')
 
     check_type(info, "'data' for struct '%s'" % name, members,
-               allow_dict=True)
+               allow_dict=name)
     check_type(info, "'base' for struct '%s'" % name, expr.get('base'),
                allow_metas=['struct'])
 
@@ -1555,10 +1562,6 @@ class QAPISchemaMember(object):
 
     def check_clash(self, info, seen):
         cname = c_name(self.name)
-        if (cname.lower() != cname
-                and self.defined_in not in name_case_whitelist):
-            raise QAPISemError(info,
-                               "%s should not use uppercase" % self.describe())
         if cname in seen:
             raise QAPISemError(info, "%s collides with %s" %
                                (self.describe(), seen[cname].describe()))

diff --git a/tests/qapi-schema/args-member-case.err b/tests/qapi-schema/args-member-case.err
index 725ba161920920e56a9e0a16366a27548eba0de5..da183957b2eff14782b4f1e0e2155956f4553d9a 100644 (file)
--- a/tests/qapi-schema/args-member-case.err
+++ b/tests/qapi-schema/args-member-case.err
@@ -1,2 +1,2 @@
 tests/qapi-schema/args-member-case.json: In command 'no-way-this-will-get-whitelisted':
-tests/qapi-schema/args-member-case.json:2: 'Arg' (parameter of no-way-this-will-get-whitelisted) should not use uppercase
+tests/qapi-schema/args-member-case.json:2: Member of 'data' for command 'no-way-this-will-get-whitelisted' uses uppercase in name 'Arg'

diff --git a/tests/qapi-schema/args-member-case.json b/tests/qapi-schema/args-member-case.json
index 93439bee8b337349452b5dc7a2657aed48f8074c..e27c6035481c01cd3c710a647e0d8decaf6d7c4d 100644 (file)
--- a/tests/qapi-schema/args-member-case.json
+++ b/tests/qapi-schema/args-member-case.json
@@ -1,2 +1,2 @@
-# Member names should be 'lower-case' unless the struct/command is whitelisted
+# Member names should be 'lower-case' unless the struct is whitelisted
 { 'command': 'no-way-this-will-get-whitelisted', 'data': { 'Arg': 'int' } }

diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err
index f6c872d3bfd27164d0883f6a62bd710424f76ac2..8f2007c86f8a75d9ac83300862e7e066d7c8b452 100644 (file)
--- a/tests/qapi-schema/enum-member-case.err
+++ b/tests/qapi-schema/enum-member-case.err
@@ -1,2 +1,2 @@
 tests/qapi-schema/enum-member-case.json: In enum 'NoWayThisWillGetWhitelisted':
-tests/qapi-schema/enum-member-case.json:4: 'Value' (value of NoWayThisWillGetWhitelisted) should not use uppercase
+tests/qapi-schema/enum-member-case.json:4: Member of enum 'NoWayThisWillGetWhitelisted' uses uppercase in name 'Value'

diff --git a/tests/qapi-schema/union-branch-case.err b/tests/qapi-schema/union-branch-case.err
index 8e81a2d0b6e953ae0d85a1f3bcce8263bd6ed055..09313d7f83f634b39f1ee04ce1c91895a71a63f2 100644 (file)
--- a/tests/qapi-schema/union-branch-case.err
+++ b/tests/qapi-schema/union-branch-case.err
@@ -1,2 +1,2 @@
-tests/qapi-schema/union-branch-case.json: In union 'NoWayThisWillGetWhitelisted':
-tests/qapi-schema/union-branch-case.json:2: 'Branch' (branch of NoWayThisWillGetWhitelisted) should not use uppercase
+tests/qapi-schema/union-branch-case.json: In union 'Uni':
+tests/qapi-schema/union-branch-case.json:2: Member of union 'Uni' uses uppercase in name 'Branch'

diff --git a/tests/qapi-schema/union-branch-case.json b/tests/qapi-schema/union-branch-case.json
index e6565dc3b35d9e60feb09f202bc87edd0b49aa2f..b7894b75d6033022a67cab86913646067f0d5e03 100644 (file)
--- a/tests/qapi-schema/union-branch-case.json
+++ b/tests/qapi-schema/union-branch-case.json
@@ -1,2 +1,2 @@
-# Branch names should be 'lower-case' unless the union is whitelisted
-{ 'union': 'NoWayThisWillGetWhitelisted', 'data': { 'Branch': 'int' } }
+# Branch names should be 'lower-case'
+{ 'union': 'Uni', 'data': { 'Branch': 'int' } }