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

Commit Message

John Snow Jan. 12, 2024, 10:29 p.m. UTC

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

Signed-off-by: John Snow <[e-mail]>
Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
---
 scripts/qapi/schema.py | 2 ++
 1 file changed, 2 insertions(+)

Comments

Markus Armbruster Jan. 16, 2024, 1:37 p.m. UTC | #1

John Snow <[e-mail]> 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 <[e-mail]>
> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
> ---
>  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(

Patch

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(