John Snow <jsnow@redhat.com> writes:
> lookup_type() is capable of returning None, but introspect.py isn't
> prepared for that. (And rightly so, if these built-in types are absent,
> something has gone hugely wrong.)
>
> RFC: This is slightly cumbersome as-is, but a patch at the end of this series
> tries to address it with some slightly slicker lookup functions that
> don't need as much hand-holding.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/introspect.py | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 67c7d89aae0..42981bce163 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str:
>
> # Map the various integer types to plain int
> if typ.json_type() == 'int':
> - typ = self._schema.lookup_type('int')
> + tmp = self._schema.lookup_type('int')
> + assert tmp is not None
More laconic: assert tmp
*looks up "laconic"*
hey, "terse" is even fewer letters!
(but, you're right. I think I adopted the "is not none" out of a habit for distinguishing false-y values from the None value, but in this case we really wouldn't want to have either, so the shorter form is fine, though for mypy's sake we only care about guarding against None here.)
> + typ = tmp
> elif (isinstance(typ, QAPISchemaArrayType) and
> typ.element_type.json_type() == 'int'):
> - typ = self._schema.lookup_type('intList')
> + tmp = self._schema.lookup_type('intList')
> + assert tmp is not None
> + typ = tmp
> # Add type to work queue if new
> if typ not in self._used_types:
> self._used_types.append(typ)
Not fond of naming things @tmp, but I don't have a better name to offer.
We could avoid the lookup by having _def_predefineds() set suitable
attributes, like it serts .the_empty_object_type. Matter of taste. Not
now unless you want to.
Check the end of the series for different lookup methods, too. We can discuss your preferred solution then, perhaps?