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

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> 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.

I'll drop it.

>> +++ 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.

I was unsure whether to complicate the regexp slightly so
single-character type names keep working.  Then I decided against it.

>> +++ 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).

Yes, that's prudent.

> Such a tweak is minor enough that I'm fine with
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!