Re: [PATCH v2 12/19] qapi/schema: assert info is present when necessary

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> QAPISchemaInfo instances are sometimes defined as an Optional
> field/argument because built-in definitions don't *have* a source
> definition. As a consequence, there are a few places where we need to
> assert that it's present because the root entity definition can only
> enforce that it is "Optional".

Well, we need to assert to help mypy over the hump.  But that's later in
this series.  Just like "enforce that is is 'Optional'".  My point is:
the commit message is less than clear unless the reader already knows
where we're going.

Here's my try:

  QAPISchemaInfo arguments can often be None because build-in
  definitions don't have such information.  The type hint can only be
  Optional[QAPISchemaInfo] then.  But then mypy gets upset about all the
  places where we exploit that it can't actually be None there.  Add
  assertions that will help mypy over the hump, to enable adding the
  type hints.

>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  scripts/qapi/schema.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 43af756ed47..eefa58a798b 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -758,6 +758,7 @@ def describe(self, info):
>              else:
>                  assert False
>  
> +        assert info is not None
>          if defined_in != info.defn_name:
>              return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in)
>          return "%s '%s'" % (role, self.name)
> @@ -848,6 +849,7 @@ def __init__(self, name, info, doc, ifcond, features,
>          self.coroutine = coroutine
>  
>      def check(self, schema):
> +        assert self.info is not None
>          super().check(schema)
>          if self._arg_type_name:
>              arg_type = schema.resolve_type(