Re: [PATCH 14/28] qapi: Enforce type naming rules

[Eric Blake]

On 3/23/21 4:40 AM, Markus Armbruster wrote:
> Type names should be CamelCase.  Enforce this.  The only offenders are
> in tests/.  Fix them.  Add test type-case to cover the new error.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/scripts/qapi/expr.py
> @@ -61,7 +61,8 @@ def check_name_lower(name, info, source,
>  
>  def check_name_camel(name, info, source):
>      stem = check_name_str(name, info, source)
> -    # TODO reject '[_-]' in stem, require CamelCase
> +    if not re.match(r'[A-Z]+[A-Za-z0-9]*[a-z][A-Za-z0-9]*$', stem):

Requires one or more leading capital, and at least one lowercase.  This
permits oddballs like PCIELinkSpeed in common.json that are eventually
camel case but with a rather long all-caps start.

As written, the + isn't necessary, you'd match the same set of strings
with it omitted.  But leaving it doesn't hurt.

> +++ b/tests/qapi-schema/doc-bad-union-member.json
> @@ -11,9 +11,9 @@
>    'data': { 'nothing': 'Empty' } }
>  
>  { 'struct': 'Base',
> -  'data': { 'type': 'T' } }
> +  'data': { 'type': 'FrobType' } }

No single-character type names is fallout from the tighter rules, but is
fine with me.


> +++ b/tests/qapi-schema/type-case.json
> @@ -0,0 +1,2 @@
> +# Type names should use CamelCase
> +{ 'struct': 'not-a-camel' }

You should probably include a 'data':{...} here, to ensure that we
aren't rejecting this for missing data (yes, the .err file does test our
actual error message, but no reason to not be otherwise compliant to
what we normally expect).

Such a tweak is minor enough that I'm fine with

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org