Re: [PATCH v3 09/20] qapi/schema: assert resolve_type has 'info' and 'what' args on error

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Tue, Feb 20, 2024 at 5:48 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > resolve_type() is generally used to resolve configuration-provided type
>> > names into type objects, and generally requires valid 'info' and 'what'
>> > parameters.
>> >
>> > In some cases, such as with QAPISchemaArrayType.check(), resolve_type
>> > may be used to resolve built-in types and as such will not have an
>> > 'info' argument, but also must not fail in this scenario.
>> >
>> > Use an assertion to sate mypy that we will indeed have 'info' and 'what'
>> > parameters for the error pathway in resolve_type.
>> >
>> > Note: there are only three callsites to resolve_type at present where
>> > "info" is perceived to be possibly None:
>>
>> Who is the perceiver?  mypy?
>
> Deep.
>
> Yes.

Recommend active voice: "where mypy preceives @info to be possibly None".

>>
>> >
>> >     1) QAPISchemaArrayType.check()
>> >     2) QAPISchemaObjectTypeMember.check()
>> >     3) QAPISchemaEvent.check()
>> >
>> >     Of those three, only the first actually ever passes None; the other two
>> >     are limited by their base class initializers which accept info=None, 
>> > but
>> >     neither subclass actually use a None value in practice, currently.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  scripts/qapi/schema.py | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> > index e617abb03af..573be7275a6 100644
>> > --- a/scripts/qapi/schema.py
>> > +++ b/scripts/qapi/schema.py
>> > @@ -1004,6 +1004,7 @@ def lookup_type(self, name):
>> >      def resolve_type(self, name, info, what):
>> >          typ = self.lookup_type(name)
>> >          if not typ:
>> > +            assert info and what  # built-in types must not fail lookup
>> >              if callable(what):
>> >                  what = what(info)
>> >              raise QAPISemError(
>> <
>>