| Message ID | 20210323094025.3569441-14-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: Enforce naming rules | expand |
On 3/23/21 4:40 AM, Markus Armbruster wrote: > Event names should be ALL_CAPS with words separated by underscore. > Enforce this. The only offenders are in tests/. Fix them. Existing > test event-case covers the new error. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- Reviewed-by: Eric Blake <eblake@redhat.com>
On 3/23/21 5:40 AM, Markus Armbruster wrote: > Event names should be ALL_CAPS with words separated by underscore. > Enforce this. The only offenders are in tests/. Fix them. Existing > test event-case covers the new error. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/unit/test-qmp-event.c | 6 +++--- > scripts/qapi/expr.py | 4 +++- > tests/qapi-schema/doc-good.json | 4 ++-- > tests/qapi-schema/doc-good.out | 4 ++-- > tests/qapi-schema/doc-good.txt | 2 +- > tests/qapi-schema/doc-invalid-return.json | 4 ++-- > tests/qapi-schema/event-case.err | 2 ++ > tests/qapi-schema/event-case.json | 2 -- > tests/qapi-schema/event-case.out | 14 -------------- > tests/qapi-schema/qapi-schema-test.json | 6 +++--- > tests/qapi-schema/qapi-schema-test.out | 8 ++++---- > 11 files changed, 22 insertions(+), 34 deletions(-) > > diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c > index 047f44ff9a..d58c3b78f2 100644 > --- a/tests/unit/test-qmp-event.c > +++ b/tests/unit/test-qmp-event.c > @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, > > static void test_event_deprecated(TestEventData *data, const void *unused) > { > - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); > + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); > > memset(&compat_policy, 0, sizeof(compat_policy)); > > @@ -163,7 +163,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) > { > memset(&compat_policy, 0, sizeof(compat_policy)); > > - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0'," > + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," > " 'data': { 'foo': 42 } }"); > qapi_event_send_test_event_features0(42); > g_assert(data->emitted); > @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) > > compat_policy.has_deprecated_output = true; > compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; > - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); > + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); > qapi_event_send_test_event_features0(42); > g_assert(data->emitted); > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index b5fb0be48b..c065505b27 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -45,7 +45,9 @@ def check_name_str(name, info, source): > > def check_name_upper(name, info, source): > stem = check_name_str(name, info, source) > - # TODO reject '[a-z-]' in @stem > + if re.search(r'[a-z-]', stem): > + raise QAPISemError( > + info, "name of %s must not use lowercase or '-'" % source) > Does a little bit more than check_name_upper. Is this only used for event names? I guess so. Should it be inlined into check_defn_name_str instead in this case, or nah?
John Snow <jsnow@redhat.com> writes: > On 3/23/21 5:40 AM, Markus Armbruster wrote: >> Event names should be ALL_CAPS with words separated by underscore. >> Enforce this. The only offenders are in tests/. Fix them. Existing >> test event-case covers the new error. >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/unit/test-qmp-event.c | 6 +++--- >> scripts/qapi/expr.py | 4 +++- >> tests/qapi-schema/doc-good.json | 4 ++-- >> tests/qapi-schema/doc-good.out | 4 ++-- >> tests/qapi-schema/doc-good.txt | 2 +- >> tests/qapi-schema/doc-invalid-return.json | 4 ++-- >> tests/qapi-schema/event-case.err | 2 ++ >> tests/qapi-schema/event-case.json | 2 -- >> tests/qapi-schema/event-case.out | 14 -------------- >> tests/qapi-schema/qapi-schema-test.json | 6 +++--- >> tests/qapi-schema/qapi-schema-test.out | 8 ++++---- >> 11 files changed, 22 insertions(+), 34 deletions(-) >> diff --git a/tests/unit/test-qmp-event.c >> b/tests/unit/test-qmp-event.c >> index 047f44ff9a..d58c3b78f2 100644 >> --- a/tests/unit/test-qmp-event.c >> +++ b/tests/unit/test-qmp-event.c >> @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, >> static void test_event_deprecated(TestEventData *data, const >> void *unused) >> { >> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); >> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); >> memset(&compat_policy, 0, sizeof(compat_policy)); >> @@ -163,7 +163,7 @@ static void >> test_event_deprecated_data(TestEventData *data, const void *unused) >> { >> memset(&compat_policy, 0, sizeof(compat_policy)); >> - data->expect = qdict_from_jsonf_nofail("{ 'event': >> 'TEST-EVENT-FEATURES0'," >> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," >> " 'data': { 'foo': 42 } }"); >> qapi_event_send_test_event_features0(42); >> g_assert(data->emitted); >> @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) >> compat_policy.has_deprecated_output = true; >> compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; >> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); >> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); >> qapi_event_send_test_event_features0(42); >> g_assert(data->emitted); >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >> index b5fb0be48b..c065505b27 100644 >> --- a/scripts/qapi/expr.py >> +++ b/scripts/qapi/expr.py >> @@ -45,7 +45,9 @@ def check_name_str(name, info, source): >> def check_name_upper(name, info, source): >> stem = check_name_str(name, info, source) >> - # TODO reject '[a-z-]' in @stem >> + if re.search(r'[a-z-]', stem): >> + raise QAPISemError( >> + info, "name of %s must not use lowercase or '-'" % source) >> > > Does a little bit more than check_name_upper. Is this only used for > event names? I guess so. Should it be inlined into check_defn_name_str > instead in this case, or nah? I'd prefer not to inline. I'm open to better function names. We have three name styles. qapi-code-gen.txt: [Type] definitions should always use CamelCase for user-defined type names, while built-in types are lowercase. [...] Command names, and member names within a type, should be all lower case with words separated by a hyphen. [...] Event names should be ALL_CAPS with words separated by underscore. I define three functions for them: check_name_camel(), check_name_lower(), and check_name_upper(). The functions factor out the naming rule aspect, and they let us keep the naming rule aspect together. That's why I'd prefer not to inline. We could name them after their purpose instead: check_name_user_defined_type(), check_name_command_or_member(), check_name_event(). The first two are rather long. Shorter: check_name_type(), check_name_other(), check_name_event(). Thoughts?
On 3/24/21 2:22 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> On 3/23/21 5:40 AM, Markus Armbruster wrote: >>> Event names should be ALL_CAPS with words separated by underscore. >>> Enforce this. The only offenders are in tests/. Fix them. Existing >>> test event-case covers the new error. >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> tests/unit/test-qmp-event.c | 6 +++--- >>> scripts/qapi/expr.py | 4 +++- >>> tests/qapi-schema/doc-good.json | 4 ++-- >>> tests/qapi-schema/doc-good.out | 4 ++-- >>> tests/qapi-schema/doc-good.txt | 2 +- >>> tests/qapi-schema/doc-invalid-return.json | 4 ++-- >>> tests/qapi-schema/event-case.err | 2 ++ >>> tests/qapi-schema/event-case.json | 2 -- >>> tests/qapi-schema/event-case.out | 14 -------------- >>> tests/qapi-schema/qapi-schema-test.json | 6 +++--- >>> tests/qapi-schema/qapi-schema-test.out | 8 ++++---- >>> 11 files changed, 22 insertions(+), 34 deletions(-) >>> diff --git a/tests/unit/test-qmp-event.c >>> b/tests/unit/test-qmp-event.c >>> index 047f44ff9a..d58c3b78f2 100644 >>> --- a/tests/unit/test-qmp-event.c >>> +++ b/tests/unit/test-qmp-event.c >>> @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, >>> static void test_event_deprecated(TestEventData *data, const >>> void *unused) >>> { >>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); >>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); >>> memset(&compat_policy, 0, sizeof(compat_policy)); >>> @@ -163,7 +163,7 @@ static void >>> test_event_deprecated_data(TestEventData *data, const void *unused) >>> { >>> memset(&compat_policy, 0, sizeof(compat_policy)); >>> - data->expect = qdict_from_jsonf_nofail("{ 'event': >>> 'TEST-EVENT-FEATURES0'," >>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," >>> " 'data': { 'foo': 42 } }"); >>> qapi_event_send_test_event_features0(42); >>> g_assert(data->emitted); >>> @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) >>> compat_policy.has_deprecated_output = true; >>> compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; >>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); >>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); >>> qapi_event_send_test_event_features0(42); >>> g_assert(data->emitted); >>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >>> index b5fb0be48b..c065505b27 100644 >>> --- a/scripts/qapi/expr.py >>> +++ b/scripts/qapi/expr.py >>> @@ -45,7 +45,9 @@ def check_name_str(name, info, source): >>> def check_name_upper(name, info, source): >>> stem = check_name_str(name, info, source) >>> - # TODO reject '[a-z-]' in @stem >>> + if re.search(r'[a-z-]', stem): >>> + raise QAPISemError( >>> + info, "name of %s must not use lowercase or '-'" % source) >>> >> >> Does a little bit more than check_name_upper. Is this only used for >> event names? I guess so. Should it be inlined into check_defn_name_str >> instead in this case, or nah? > > I'd prefer not to inline. I'm open to better function names. > > We have three name styles. qapi-code-gen.txt: > > [Type] definitions should always use CamelCase for > user-defined type names, while built-in types are lowercase. > > [...] > > Command names, and member names within a type, should be all lower > case with words separated by a hyphen. [...] > > Event names should be ALL_CAPS with words separated by underscore. > > I define three functions for them: check_name_camel(), > check_name_lower(), and check_name_upper(). > > The functions factor out the naming rule aspect, and they let us keep > the naming rule aspect together. That's why I'd prefer not to inline. > > We could name them after their purpose instead: > check_name_user_defined_type(), check_name_command_or_member(), > check_name_event(). The first two are rather long. Shorter: > check_name_type(), check_name_other(), check_name_event(). > > Thoughts? > The long names are nice and descriptive.
John Snow <jsnow@redhat.com> writes: > On 3/24/21 2:22 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> On 3/23/21 5:40 AM, Markus Armbruster wrote: >>>> Event names should be ALL_CAPS with words separated by underscore. >>>> Enforce this. The only offenders are in tests/. Fix them. Existing >>>> test event-case covers the new error. >>>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>>> --- >>>> tests/unit/test-qmp-event.c | 6 +++--- >>>> scripts/qapi/expr.py | 4 +++- >>>> tests/qapi-schema/doc-good.json | 4 ++-- >>>> tests/qapi-schema/doc-good.out | 4 ++-- >>>> tests/qapi-schema/doc-good.txt | 2 +- >>>> tests/qapi-schema/doc-invalid-return.json | 4 ++-- >>>> tests/qapi-schema/event-case.err | 2 ++ >>>> tests/qapi-schema/event-case.json | 2 -- >>>> tests/qapi-schema/event-case.out | 14 -------------- >>>> tests/qapi-schema/qapi-schema-test.json | 6 +++--- >>>> tests/qapi-schema/qapi-schema-test.out | 8 ++++---- >>>> 11 files changed, 22 insertions(+), 34 deletions(-) >>>> diff --git a/tests/unit/test-qmp-event.c >>>> b/tests/unit/test-qmp-event.c >>>> index 047f44ff9a..d58c3b78f2 100644 >>>> --- a/tests/unit/test-qmp-event.c >>>> +++ b/tests/unit/test-qmp-event.c >>>> @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, >>>> static void test_event_deprecated(TestEventData *data, const >>>> void *unused) >>>> { >>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); >>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); >>>> memset(&compat_policy, 0, sizeof(compat_policy)); >>>> @@ -163,7 +163,7 @@ static void >>>> test_event_deprecated_data(TestEventData *data, const void *unused) >>>> { >>>> memset(&compat_policy, 0, sizeof(compat_policy)); >>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': >>>> 'TEST-EVENT-FEATURES0'," >>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," >>>> " 'data': { 'foo': 42 } }"); >>>> qapi_event_send_test_event_features0(42); >>>> g_assert(data->emitted); >>>> @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) >>>> compat_policy.has_deprecated_output = true; >>>> compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; >>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); >>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); >>>> qapi_event_send_test_event_features0(42); >>>> g_assert(data->emitted); >>>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >>>> index b5fb0be48b..c065505b27 100644 >>>> --- a/scripts/qapi/expr.py >>>> +++ b/scripts/qapi/expr.py >>>> @@ -45,7 +45,9 @@ def check_name_str(name, info, source): >>>> def check_name_upper(name, info, source): >>>> stem = check_name_str(name, info, source) >>>> - # TODO reject '[a-z-]' in @stem >>>> + if re.search(r'[a-z-]', stem): >>>> + raise QAPISemError( >>>> + info, "name of %s must not use lowercase or '-'" % source) >>>> >>> >>> Does a little bit more than check_name_upper. Is this only used for >>> event names? I guess so. Should it be inlined into check_defn_name_str >>> instead in this case, or nah? >> >> I'd prefer not to inline. I'm open to better function names. >> >> We have three name styles. qapi-code-gen.txt: >> >> [Type] definitions should always use CamelCase for >> user-defined type names, while built-in types are lowercase. >> >> [...] >> >> Command names, and member names within a type, should be all lower >> case with words separated by a hyphen. [...] >> >> Event names should be ALL_CAPS with words separated by underscore. >> >> I define three functions for them: check_name_camel(), >> check_name_lower(), and check_name_upper(). >> >> The functions factor out the naming rule aspect, and they let us keep >> the naming rule aspect together. That's why I'd prefer not to inline. >> >> We could name them after their purpose instead: >> check_name_user_defined_type(), check_name_command_or_member(), >> check_name_event(). The first two are rather long. Shorter: >> check_name_type(), check_name_other(), check_name_event(). >> >> Thoughts? >> > > The long names are nice and descriptive. Then I should give them a try to see whether the result feels neat or ugly.
On 3/25/21 2:22 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> On 3/24/21 2:22 AM, Markus Armbruster wrote: >>> John Snow <jsnow@redhat.com> writes: >>> >>>> On 3/23/21 5:40 AM, Markus Armbruster wrote: >>>>> Event names should be ALL_CAPS with words separated by underscore. >>>>> Enforce this. The only offenders are in tests/. Fix them. Existing >>>>> test event-case covers the new error. >>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>>>> --- >>>>> tests/unit/test-qmp-event.c | 6 +++--- >>>>> scripts/qapi/expr.py | 4 +++- >>>>> tests/qapi-schema/doc-good.json | 4 ++-- >>>>> tests/qapi-schema/doc-good.out | 4 ++-- >>>>> tests/qapi-schema/doc-good.txt | 2 +- >>>>> tests/qapi-schema/doc-invalid-return.json | 4 ++-- >>>>> tests/qapi-schema/event-case.err | 2 ++ >>>>> tests/qapi-schema/event-case.json | 2 -- >>>>> tests/qapi-schema/event-case.out | 14 -------------- >>>>> tests/qapi-schema/qapi-schema-test.json | 6 +++--- >>>>> tests/qapi-schema/qapi-schema-test.out | 8 ++++---- >>>>> 11 files changed, 22 insertions(+), 34 deletions(-) >>>>> diff --git a/tests/unit/test-qmp-event.c >>>>> b/tests/unit/test-qmp-event.c >>>>> index 047f44ff9a..d58c3b78f2 100644 >>>>> --- a/tests/unit/test-qmp-event.c >>>>> +++ b/tests/unit/test-qmp-event.c >>>>> @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, >>>>> static void test_event_deprecated(TestEventData *data, const >>>>> void *unused) >>>>> { >>>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); >>>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); >>>>> memset(&compat_policy, 0, sizeof(compat_policy)); >>>>> @@ -163,7 +163,7 @@ static void >>>>> test_event_deprecated_data(TestEventData *data, const void *unused) >>>>> { >>>>> memset(&compat_policy, 0, sizeof(compat_policy)); >>>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': >>>>> 'TEST-EVENT-FEATURES0'," >>>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," >>>>> " 'data': { 'foo': 42 } }"); >>>>> qapi_event_send_test_event_features0(42); >>>>> g_assert(data->emitted); >>>>> @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) >>>>> compat_policy.has_deprecated_output = true; >>>>> compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; >>>>> - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); >>>>> + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); >>>>> qapi_event_send_test_event_features0(42); >>>>> g_assert(data->emitted); >>>>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >>>>> index b5fb0be48b..c065505b27 100644 >>>>> --- a/scripts/qapi/expr.py >>>>> +++ b/scripts/qapi/expr.py >>>>> @@ -45,7 +45,9 @@ def check_name_str(name, info, source): >>>>> def check_name_upper(name, info, source): >>>>> stem = check_name_str(name, info, source) >>>>> - # TODO reject '[a-z-]' in @stem >>>>> + if re.search(r'[a-z-]', stem): >>>>> + raise QAPISemError( >>>>> + info, "name of %s must not use lowercase or '-'" % source) >>>>> >>>> >>>> Does a little bit more than check_name_upper. Is this only used for >>>> event names? I guess so. Should it be inlined into check_defn_name_str >>>> instead in this case, or nah? >>> >>> I'd prefer not to inline. I'm open to better function names. >>> >>> We have three name styles. qapi-code-gen.txt: >>> >>> [Type] definitions should always use CamelCase for >>> user-defined type names, while built-in types are lowercase. >>> >>> [...] >>> >>> Command names, and member names within a type, should be all lower >>> case with words separated by a hyphen. [...] >>> >>> Event names should be ALL_CAPS with words separated by underscore. >>> >>> I define three functions for them: check_name_camel(), >>> check_name_lower(), and check_name_upper(). >>> >>> The functions factor out the naming rule aspect, and they let us keep >>> the naming rule aspect together. That's why I'd prefer not to inline. >>> >>> We could name them after their purpose instead: >>> check_name_user_defined_type(), check_name_command_or_member(), >>> check_name_event(). The first two are rather long. Shorter: >>> check_name_type(), check_name_other(), check_name_event(). >>> >>> Thoughts? >>> >> >> The long names are nice and descriptive. > > Then I should give them a try to see whether the result feels neat or > ugly. > I tried my hand at documenting them in my respin; I am not entirely confident I got the names and purposes and semantics exactly right. I didn't try to rename them, but it would be easy to do. You'll have to let me know your preferences. --js
diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c index 047f44ff9a..d58c3b78f2 100644 --- a/tests/unit/test-qmp-event.c +++ b/tests/unit/test-qmp-event.c @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, static void test_event_deprecated(TestEventData *data, const void *unused) { - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); memset(&compat_policy, 0, sizeof(compat_policy)); @@ -163,7 +163,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) { memset(&compat_policy, 0, sizeof(compat_policy)); - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0'," + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," " 'data': { 'foo': 42 } }"); qapi_event_send_test_event_features0(42); g_assert(data->emitted); @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); qapi_event_send_test_event_features0(42); g_assert(data->emitted); diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index b5fb0be48b..c065505b27 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -45,7 +45,9 @@ def check_name_str(name, info, source): def check_name_upper(name, info, source): stem = check_name_str(name, info, source) - # TODO reject '[a-z-]' in @stem + if re.search(r'[a-z-]', stem): + raise QAPISemError( + info, "name of %s must not use lowercase or '-'" % source) def check_name_lower(name, info, source, diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index e9af0857db..423ea23e07 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -179,10 +179,10 @@ 'features': [ 'cmd-feat1', 'cmd-feat2' ] } ## -# @EVT-BOXED: +# @EVT_BOXED: # Features: # @feat3: a feature ## -{ 'event': 'EVT-BOXED', 'boxed': true, +{ 'event': 'EVT_BOXED', 'boxed': true, 'features': [ 'feat3' ], 'data': 'Object' } diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 715b0bbc1a..8f54ceff2e 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -63,7 +63,7 @@ command cmd-boxed Object -> None gen=True success_response=True boxed=True oob=False preconfig=False feature cmd-feat1 feature cmd-feat2 -event EVT-BOXED Object +event EVT_BOXED Object boxed=True feature feat3 doc freeform @@ -211,7 +211,7 @@ another feature -> in <- out -doc symbol=EVT-BOXED +doc symbol=EVT_BOXED body= feature=feat3 diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt index 6ca03d49d0..726727af74 100644 --- a/tests/qapi-schema/doc-good.txt +++ b/tests/qapi-schema/doc-good.txt @@ -272,7 +272,7 @@ Example <- out -"EVT-BOXED" (Event) +"EVT_BOXED" (Event) ------------------- diff --git a/tests/qapi-schema/doc-invalid-return.json b/tests/qapi-schema/doc-invalid-return.json index 1ba45de414..95e7583930 100644 --- a/tests/qapi-schema/doc-invalid-return.json +++ b/tests/qapi-schema/doc-invalid-return.json @@ -1,7 +1,7 @@ # Events can't have 'Returns' section ## -# @foo: +# @FOO: # Returns: blah ## -{ 'event': 'foo' } +{ 'event': 'FOO' } diff --git a/tests/qapi-schema/event-case.err b/tests/qapi-schema/event-case.err index e69de29bb2..d3007cfa63 100644 --- a/tests/qapi-schema/event-case.err +++ b/tests/qapi-schema/event-case.err @@ -0,0 +1,2 @@ +event-case.json: In event 'oops': +event-case.json:1: name of event must not use lowercase or '-' diff --git a/tests/qapi-schema/event-case.json b/tests/qapi-schema/event-case.json index 3a92d8b610..4d8a5d8a71 100644 --- a/tests/qapi-schema/event-case.json +++ b/tests/qapi-schema/event-case.json @@ -1,3 +1 @@ -# TODO: might be nice to enforce naming conventions; but until then this works -# even though events should usually be ALL_CAPS { 'event': 'oops' } diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out index 9ae44052ac..e69de29bb2 100644 --- a/tests/qapi-schema/event-case.out +++ b/tests/qapi-schema/event-case.out @@ -1,14 +0,0 @@ -module ./builtin -object q_empty -enum QType - prefix QTYPE - member none - member qnull - member qnum - member qstring - member qdict - member qlist - member qbool -module event-case.json -event oops None - boxed=False diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 12ec588b52..a355321258 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -249,7 +249,7 @@ { 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' } -{ 'event': 'TestIfEvent', 'data': +{ 'event': 'TEST_IF_EVENT', 'data': { 'foo': 'TestIfStruct', 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } }, 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } @@ -324,8 +324,8 @@ 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] } ] } -{ 'event': 'TEST-EVENT-FEATURES0', +{ 'event': 'TEST_EVENT_FEATURES0', 'data': 'FeatureStruct1' } -{ 'event': 'TEST-EVENT-FEATURES1', +{ 'event': 'TEST_EVENT_FEATURES1', 'features': [ 'deprecated' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index f5741df97f..882d0e7c56 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -349,12 +349,12 @@ command TestCmdReturnDefThree None -> UserDefThree gen=True success_response=True boxed=False oob=False preconfig=False array TestIfEnumList TestIfEnum if ['defined(TEST_IF_ENUM)'] -object q_obj_TestIfEvent-arg +object q_obj_TEST_IF_EVENT-arg member foo: TestIfStruct optional=False member bar: TestIfEnumList optional=False if ['defined(TEST_IF_EVT_BAR)'] if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] -event TestIfEvent q_obj_TestIfEvent-arg +event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg boxed=False if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] object FeatureStruct0 @@ -440,9 +440,9 @@ command test-command-cond-features3 None -> None gen=True success_response=True boxed=False oob=False preconfig=False feature feature1 if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] -event TEST-EVENT-FEATURES0 FeatureStruct1 +event TEST_EVENT_FEATURES0 FeatureStruct1 boxed=False -event TEST-EVENT-FEATURES1 None +event TEST_EVENT_FEATURES1 None boxed=False feature deprecated module include/sub-module.json
Event names should be ALL_CAPS with words separated by underscore. Enforce this. The only offenders are in tests/. Fix them. Existing test event-case covers the new error. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/unit/test-qmp-event.c | 6 +++--- scripts/qapi/expr.py | 4 +++- tests/qapi-schema/doc-good.json | 4 ++-- tests/qapi-schema/doc-good.out | 4 ++-- tests/qapi-schema/doc-good.txt | 2 +- tests/qapi-schema/doc-invalid-return.json | 4 ++-- tests/qapi-schema/event-case.err | 2 ++ tests/qapi-schema/event-case.json | 2 -- tests/qapi-schema/event-case.out | 14 -------------- tests/qapi-schema/qapi-schema-test.json | 6 +++--- tests/qapi-schema/qapi-schema-test.out | 8 ++++---- 11 files changed, 22 insertions(+), 34 deletions(-)