diff mbox series

[v3,5/7] qapi/parser: add QAPIExpression type

Message ID 20230209184758.1017863-6-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: static typing conversion, pt5c | expand

Commit Message

John Snow Feb. 9, 2023, 6:47 p.m. UTC
This patch creates a new type, QAPIExpression, which represents a parsed
expression complete with QAPIDoc and QAPISourceInfo.

This patch turns parser.exprs into a list of QAPIExpression instead,
and adjusts expr.py to match.

This allows the types we specify in parser.py to be "remembered" all the
way through expr.py and into schema.py. Several assertions around
packing and unpacking this data can be removed as a result.

NB: This necessitates a change of typing for check_if() and
check_keys(), because mypy does not believe UserDict[str, object] ⊆
Dict[str, object]. It will, however, accept Mapping or
MutableMapping. In this case, the immutable form is preferred as an
input parameter because we don't actually mutate the input.

Without this change, we will observe:
qapi/expr.py:631: error: Argument 1 to "check_keys" has incompatible
type "QAPIExpression"; expected "Dict[str, object]"

NB2: Python 3.6 has an oversight for typing UserDict that makes it
impossible to type for both runtime and analysis time. The problem is
described in detail at https://github.com/python/typing/issues/60 - this
workaround can be safely removed when 3.7 is our minimum version.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/expr.py   | 89 +++++++++++++++++-------------------------
 scripts/qapi/parser.py | 54 ++++++++++++++++---------
 scripts/qapi/schema.py | 72 ++++++++++++++++++----------------
 3 files changed, 110 insertions(+), 105 deletions(-)

Comments

Markus Armbruster Feb. 11, 2023, 6:49 a.m. UTC | #1
John Snow <jsnow@redhat.com> writes:

> This patch creates a new type, QAPIExpression, which represents a parsed
> expression complete with QAPIDoc and QAPISourceInfo.
>
> This patch turns parser.exprs into a list of QAPIExpression instead,
> and adjusts expr.py to match.
>
> This allows the types we specify in parser.py to be "remembered" all the
> way through expr.py and into schema.py. Several assertions around
> packing and unpacking this data can be removed as a result.
>
> NB: This necessitates a change of typing for check_if() and
> check_keys(), because mypy does not believe UserDict[str, object] ⊆
> Dict[str, object]. It will, however, accept Mapping or
> MutableMapping. In this case, the immutable form is preferred as an
> input parameter because we don't actually mutate the input.
>
> Without this change, we will observe:
> qapi/expr.py:631: error: Argument 1 to "check_keys" has incompatible
> type "QAPIExpression"; expected "Dict[str, object]"
>
> NB2: Python 3.6 has an oversight for typing UserDict that makes it
> impossible to type for both runtime and analysis time. The problem is
> described in detail at https://github.com/python/typing/issues/60 - this
> workaround can be safely removed when 3.7 is our minimum version.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---

Looks good to me, just one question:

[...]

> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 1b006cdc133..87b46db7fba 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -14,13 +14,14 @@
>  # This work is licensed under the terms of the GNU GPL, version 2.
>  # See the COPYING file in the top-level directory.
>  
> -from collections import OrderedDict
> +from collections import OrderedDict, UserDict
>  import os
>  import re
>  from typing import (
>      TYPE_CHECKING,
>      Dict,
>      List,
> +    Mapping,
>      Optional,
>      Set,
>      Union,
> @@ -37,15 +38,30 @@
>      from .schema import QAPISchemaFeature, QAPISchemaMember
>  
>  
> -#: Represents a single Top Level QAPI schema expression.
> -TopLevelExpr = Dict[str, object]
> -
>  # Return value alias for get_expr().
>  _ExprValue = Union[List[object], Dict[str, object], str, bool]
>  
> -# FIXME: Consolidate and centralize definitions for TopLevelExpr,
> -# _ExprValue, _JSONValue, and _JSONObject; currently scattered across
> -# several modules.
> +
> +# FIXME: Consolidate and centralize definitions for _ExprValue,
> +# JSONValue, and _JSONObject; currently scattered across several
> +# modules.
> +
> +
> +# 3.6 workaround: can be removed when Python 3.7+ is our required version.
> +if TYPE_CHECKING:
> +    _UserDict = UserDict[str, object]
> +else:
> +    _UserDict = UserDict
> +
> +
> +class QAPIExpression(_UserDict):

Can we subclass dict instead?

> +    def __init__(self,
> +                 initialdata: Mapping[str, object],
> +                 info: QAPISourceInfo,
> +                 doc: Optional['QAPIDoc'] = None):
> +        super().__init__(initialdata)
> +        self.info = info
> +        self.doc: Optional['QAPIDoc'] = doc
>  
>  
>  class QAPIParseError(QAPISourceError):

[...]
John Snow Feb. 14, 2023, 4:57 p.m. UTC | #2
On Sat, Feb 11, 2023, 1:49 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This patch creates a new type, QAPIExpression, which represents a parsed
> > expression complete with QAPIDoc and QAPISourceInfo.
> >
> > This patch turns parser.exprs into a list of QAPIExpression instead,
> > and adjusts expr.py to match.
> >
> > This allows the types we specify in parser.py to be "remembered" all the
> > way through expr.py and into schema.py. Several assertions around
> > packing and unpacking this data can be removed as a result.
> >
> > NB: This necessitates a change of typing for check_if() and
> > check_keys(), because mypy does not believe UserDict[str, object] ⊆
> > Dict[str, object]. It will, however, accept Mapping or
> > MutableMapping. In this case, the immutable form is preferred as an
> > input parameter because we don't actually mutate the input.
> >
> > Without this change, we will observe:
> > qapi/expr.py:631: error: Argument 1 to "check_keys" has incompatible
> > type "QAPIExpression"; expected "Dict[str, object]"
> >
> > NB2: Python 3.6 has an oversight for typing UserDict that makes it
> > impossible to type for both runtime and analysis time. The problem is
> > described in detail at https://github.com/python/typing/issues/60 - this
> > workaround can be safely removed when 3.7 is our minimum version.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
>
> Looks good to me, just one question:
>
> [...]
>
> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> > index 1b006cdc133..87b46db7fba 100644
> > --- a/scripts/qapi/parser.py
> > +++ b/scripts/qapi/parser.py
> > @@ -14,13 +14,14 @@
> >  # This work is licensed under the terms of the GNU GPL, version 2.
> >  # See the COPYING file in the top-level directory.
> >
> > -from collections import OrderedDict
> > +from collections import OrderedDict, UserDict
> >  import os
> >  import re
> >  from typing import (
> >      TYPE_CHECKING,
> >      Dict,
> >      List,
> > +    Mapping,
> >      Optional,
> >      Set,
> >      Union,
> > @@ -37,15 +38,30 @@
> >      from .schema import QAPISchemaFeature, QAPISchemaMember
> >
> >
> > -#: Represents a single Top Level QAPI schema expression.
> > -TopLevelExpr = Dict[str, object]
> > -
> >  # Return value alias for get_expr().
> >  _ExprValue = Union[List[object], Dict[str, object], str, bool]
> >
> > -# FIXME: Consolidate and centralize definitions for TopLevelExpr,
> > -# _ExprValue, _JSONValue, and _JSONObject; currently scattered across
> > -# several modules.
> > +
> > +# FIXME: Consolidate and centralize definitions for _ExprValue,
> > +# JSONValue, and _JSONObject; currently scattered across several
> > +# modules.
> > +
> > +
> > +# 3.6 workaround: can be removed when Python 3.7+ is our required
> version.
> > +if TYPE_CHECKING:
> > +    _UserDict = UserDict[str, object]
> > +else:
> > +    _UserDict = UserDict
> > +
> > +
> > +class QAPIExpression(_UserDict):
>
> Can we subclass dict instead?
>

...apparently yes?

class QAPIExpression(Dict[str, object]): ...

Only question I have is what the init signature ought to be, though keeping
it as what I already have here apparently passes the type checker.

I think we lose the ability to do kwargs init in that case, but mypy
doesn't give us a lyskov warning, so maybe it's fine? I'll have to play
around with it. dict is implemented at the cpython level, so I'm not as
sure of how subclassing it works. They document three distinct signatures
for its initializer, which is also a python built-in.

I can rename that initialdata parameter too, pending what I discover above.


> > +    def __init__(self,
> > +                 initialdata: Mapping[str, object],
> > +                 info: QAPISourceInfo,
> > +                 doc: Optional['QAPIDoc'] = None):
> > +        super().__init__(initialdata)
> > +        self.info = info
> > +        self.doc: Optional['QAPIDoc'] = doc
> >
> >
> >  class QAPIParseError(QAPISourceError):
>
> [...]
>
>
diff mbox series

Patch

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index af802367eff..01cfc13fc3a 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -34,9 +34,9 @@ 
 import re
 from typing import (
     Collection,
-    Dict,
     Iterable,
     List,
+    Mapping,
     Optional,
     Union,
     cast,
@@ -44,7 +44,7 @@ 
 
 from .common import c_name
 from .error import QAPISemError
-from .parser import QAPIDoc
+from .parser import QAPIExpression
 from .source import QAPISourceInfo
 
 
@@ -53,7 +53,7 @@ 
 # here (and also not practical as long as mypy lacks recursive
 # types), because the purpose of this module is to interrogate that
 # type.
-_JSONObject = Dict[str, object]
+_JSONObject = Mapping[str, object]
 
 
 # See check_name_str(), below.
@@ -229,12 +229,11 @@  def pprint(elems: Iterable[str]) -> str:
                pprint(unknown), pprint(allowed)))
 
 
-def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_flags(expr: QAPIExpression) -> None:
     """
     Ensure flag members (if present) have valid values.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError:
         When certain flags have an invalid value, or when
@@ -243,18 +242,18 @@  def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
     for key in ('gen', 'success-response'):
         if key in expr and expr[key] is not False:
             raise QAPISemError(
-                info, "flag '%s' may only use false value" % key)
+                expr.info, "flag '%s' may only use false value" % key)
     for key in ('boxed', 'allow-oob', 'allow-preconfig', 'coroutine'):
         if key in expr and expr[key] is not True:
             raise QAPISemError(
-                info, "flag '%s' may only use true value" % key)
+                expr.info, "flag '%s' may only use true value" % key)
     if 'allow-oob' in expr and 'coroutine' in expr:
         # This is not necessarily a fundamental incompatibility, but
         # we don't have a use case and the desired semantics isn't
         # obvious.  The simplest solution is to forbid it until we get
         # a use case for it.
-        raise QAPISemError(info, "flags 'allow-oob' and 'coroutine' "
-                                 "are incompatible")
+        raise QAPISemError(
+            expr.info, "flags 'allow-oob' and 'coroutine' are incompatible")
 
 
 def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
@@ -447,12 +446,11 @@  def check_features(features: Optional[object],
         check_if(feat, info, source)
 
 
-def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_enum(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as an ``enum`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: When ``expr`` is not a valid ``enum``.
     :return: None, ``expr`` is normalized in-place as needed.
@@ -460,6 +458,7 @@  def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
     name = expr['enum']
     members = expr['data']
     prefix = expr.get('prefix')
+    info = expr.info
 
     if not isinstance(members, list):
         raise QAPISemError(info, "'data' must be an array")
@@ -486,12 +485,11 @@  def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_features(member.get('features'), info)
 
 
-def check_struct(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_struct(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as a ``struct`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: When ``expr`` is not a valid ``struct``.
     :return: None, ``expr`` is normalized in-place as needed.
@@ -499,16 +497,15 @@  def check_struct(expr: _JSONObject, info: QAPISourceInfo) -> None:
     name = cast(str, expr['struct'])  # Checked in check_exprs
     members = expr['data']
 
-    check_type(members, info, "'data'", allow_dict=name)
-    check_type(expr.get('base'), info, "'base'")
+    check_type(members, expr.info, "'data'", allow_dict=name)
+    check_type(expr.get('base'), expr.info, "'base'")
 
 
-def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_union(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as a ``union`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: when ``expr`` is not a valid ``union``.
     :return: None, ``expr`` is normalized in-place as needed.
@@ -517,6 +514,7 @@  def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
     base = expr['base']
     discriminator = expr['discriminator']
     members = expr['data']
+    info = expr.info
 
     check_type(base, info, "'base'", allow_dict=name)
     check_name_is_str(discriminator, info, "'discriminator'")
@@ -531,17 +529,17 @@  def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_type(value['type'], info, source, allow_array=not base)
 
 
-def check_alternate(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_alternate(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as an ``alternate`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: When ``expr`` is not a valid ``alternate``.
     :return: None, ``expr`` is normalized in-place as needed.
     """
     members = expr['data']
+    info = expr.info
 
     if not members:
         raise QAPISemError(info, "'data' must not be empty")
@@ -557,12 +555,11 @@  def check_alternate(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_type(value['type'], info, source, allow_array=True)
 
 
-def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_command(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as a ``command`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: When ``expr`` is not a valid ``command``.
     :return: None, ``expr`` is normalized in-place as needed.
@@ -572,17 +569,16 @@  def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
     boxed = expr.get('boxed', False)
 
     if boxed and args is None:
-        raise QAPISemError(info, "'boxed': true requires 'data'")
-    check_type(args, info, "'data'", allow_dict=not boxed)
-    check_type(rets, info, "'returns'", allow_array=True)
+        raise QAPISemError(expr.info, "'boxed': true requires 'data'")
+    check_type(args, expr.info, "'data'", allow_dict=not boxed)
+    check_type(rets, expr.info, "'returns'", allow_array=True)
 
 
-def check_event(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_event(expr: QAPIExpression) -> None:
     """
     Normalize and validate this expression as an ``event`` definition.
 
     :param expr: The expression to validate.
-    :param info: QAPI schema source file information.
 
     :raise QAPISemError: When ``expr`` is not a valid ``event``.
     :return: None, ``expr`` is normalized in-place as needed.
@@ -591,37 +587,23 @@  def check_event(expr: _JSONObject, info: QAPISourceInfo) -> None:
     boxed = expr.get('boxed', False)
 
     if boxed and args is None:
-        raise QAPISemError(info, "'boxed': true requires 'data'")
-    check_type(args, info, "'data'", allow_dict=not boxed)
+        raise QAPISemError(expr.info, "'boxed': true requires 'data'")
+    check_type(args, expr.info, "'data'", allow_dict=not boxed)
 
 
-def check_expr(expr_elem: _JSONObject) -> None:
+def check_expr(expr: QAPIExpression) -> None:
     """
     Validate and normalize a parsed QAPI schema expression.
 
-    :param expr_elem: The parsed expression to normalize and validate.
+    :param expr: The parsed expression to normalize and validate.
 
     :raise QAPISemError: When this expression fails validation.
     :return: None, The expression is normalized in-place as needed.
     """
-    # Expression
-    assert isinstance(expr_elem['expr'], dict)
-    for key in expr_elem['expr'].keys():
-        assert isinstance(key, str)
-    expr: _JSONObject = expr_elem['expr']
-
-    # QAPISourceInfo
-    assert isinstance(expr_elem['info'], QAPISourceInfo)
-    info: QAPISourceInfo = expr_elem['info']
-
-    # Optional[QAPIDoc]
-    tmp = expr_elem.get('doc')
-    assert tmp is None or isinstance(tmp, QAPIDoc)
-    doc: Optional[QAPIDoc] = tmp
-
     if 'include' in expr:
         return
 
+    info = expr.info
     metas = set(expr.keys() & {
         'enum', 'struct', 'union', 'alternate', 'command', 'event'})
     if len(metas) != 1:
@@ -637,6 +619,7 @@  def check_expr(expr_elem: _JSONObject) -> None:
     info.set_defn(meta, name)
     check_defn_name_str(name, info, meta)
 
+    doc = expr.doc
     if doc:
         if doc.symbol != name:
             raise QAPISemError(
@@ -649,24 +632,24 @@  def check_expr(expr_elem: _JSONObject) -> None:
     if meta == 'enum':
         check_keys(expr, info, meta,
                    ['enum', 'data'], ['if', 'features', 'prefix'])
-        check_enum(expr, info)
+        check_enum(expr)
     elif meta == 'union':
         check_keys(expr, info, meta,
                    ['union', 'base', 'discriminator', 'data'],
                    ['if', 'features'])
         normalize_members(expr.get('base'))
         normalize_members(expr['data'])
-        check_union(expr, info)
+        check_union(expr)
     elif meta == 'alternate':
         check_keys(expr, info, meta,
                    ['alternate', 'data'], ['if', 'features'])
         normalize_members(expr['data'])
-        check_alternate(expr, info)
+        check_alternate(expr)
     elif meta == 'struct':
         check_keys(expr, info, meta,
                    ['struct', 'data'], ['base', 'if', 'features'])
         normalize_members(expr['data'])
-        check_struct(expr, info)
+        check_struct(expr)
     elif meta == 'command':
         check_keys(expr, info, meta,
                    ['command'],
@@ -674,21 +657,21 @@  def check_expr(expr_elem: _JSONObject) -> None:
                     'gen', 'success-response', 'allow-oob',
                     'allow-preconfig', 'coroutine'])
         normalize_members(expr.get('data'))
-        check_command(expr, info)
+        check_command(expr)
     elif meta == 'event':
         check_keys(expr, info, meta,
                    ['event'], ['data', 'boxed', 'if', 'features'])
         normalize_members(expr.get('data'))
-        check_event(expr, info)
+        check_event(expr)
     else:
         assert False, 'unexpected meta type'
 
     check_if(expr, info, meta)
     check_features(expr.get('features'), info)
-    check_flags(expr, info)
+    check_flags(expr)
 
 
-def check_exprs(exprs: List[_JSONObject]) -> List[_JSONObject]:
+def check_exprs(exprs: List[QAPIExpression]) -> List[QAPIExpression]:
     """
     Validate and normalize a list of parsed QAPI schema expressions.
 
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 1b006cdc133..87b46db7fba 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -14,13 +14,14 @@ 
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-from collections import OrderedDict
+from collections import OrderedDict, UserDict
 import os
 import re
 from typing import (
     TYPE_CHECKING,
     Dict,
     List,
+    Mapping,
     Optional,
     Set,
     Union,
@@ -37,15 +38,30 @@ 
     from .schema import QAPISchemaFeature, QAPISchemaMember
 
 
-#: Represents a single Top Level QAPI schema expression.
-TopLevelExpr = Dict[str, object]
-
 # Return value alias for get_expr().
 _ExprValue = Union[List[object], Dict[str, object], str, bool]
 
-# FIXME: Consolidate and centralize definitions for TopLevelExpr,
-# _ExprValue, _JSONValue, and _JSONObject; currently scattered across
-# several modules.
+
+# FIXME: Consolidate and centralize definitions for _ExprValue,
+# JSONValue, and _JSONObject; currently scattered across several
+# modules.
+
+
+# 3.6 workaround: can be removed when Python 3.7+ is our required version.
+if TYPE_CHECKING:
+    _UserDict = UserDict[str, object]
+else:
+    _UserDict = UserDict
+
+
+class QAPIExpression(_UserDict):
+    def __init__(self,
+                 initialdata: Mapping[str, object],
+                 info: QAPISourceInfo,
+                 doc: Optional['QAPIDoc'] = None):
+        super().__init__(initialdata)
+        self.info = info
+        self.doc: Optional['QAPIDoc'] = doc
 
 
 class QAPIParseError(QAPISourceError):
@@ -100,7 +116,7 @@  def __init__(self,
         self.line_pos = 0
 
         # Parser output:
-        self.exprs: List[Dict[str, object]] = []
+        self.exprs: List[QAPIExpression] = []
         self.docs: List[QAPIDoc] = []
 
         # Showtime!
@@ -147,8 +163,7 @@  def _parse(self) -> None:
                                        "value of 'include' must be a string")
                 incl_fname = os.path.join(os.path.dirname(self._fname),
                                           include)
-                self.exprs.append({'expr': {'include': incl_fname},
-                                   'info': info})
+                self._add_expr(OrderedDict({'include': incl_fname}), info)
                 exprs_include = self._include(include, info, incl_fname,
                                               self._included)
                 if exprs_include:
@@ -165,17 +180,18 @@  def _parse(self) -> None:
                 for name, value in pragma.items():
                     self._pragma(name, value, info)
             else:
-                expr_elem = {'expr': expr,
-                             'info': info}
-                if cur_doc:
-                    if not cur_doc.symbol:
-                        raise QAPISemError(
-                            cur_doc.info, "definition documentation required")
-                    expr_elem['doc'] = cur_doc
-                self.exprs.append(expr_elem)
+                if cur_doc and not cur_doc.symbol:
+                    raise QAPISemError(
+                        cur_doc.info, "definition documentation required")
+                self._add_expr(expr, info, cur_doc)
             cur_doc = None
         self.reject_expr_doc(cur_doc)
 
+    def _add_expr(self, expr: Mapping[str, object],
+                  info: QAPISourceInfo,
+                  doc: Optional['QAPIDoc'] = None) -> None:
+        self.exprs.append(QAPIExpression(expr, info, doc))
+
     @staticmethod
     def reject_expr_doc(doc: Optional['QAPIDoc']) -> None:
         if doc and doc.symbol:
@@ -784,7 +800,7 @@  def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
                                % feature.name)
         self.features[feature.name].connect(feature)
 
-    def check_expr(self, expr: TopLevelExpr) -> None:
+    def check_expr(self, expr: QAPIExpression) -> None:
         if self.has_section('Returns') and 'command' not in expr:
             raise QAPISemError(self.info,
                                "'Returns:' is only valid for commands")
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cd8661125cd..207e4d71f39 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -17,7 +17,7 @@ 
 from collections import OrderedDict
 import os
 import re
-from typing import Optional
+from typing import List, Optional
 
 from .common import (
     POINTER_SUFFIX,
@@ -29,7 +29,7 @@ 
 )
 from .error import QAPIError, QAPISemError, QAPISourceError
 from .expr import check_exprs
-from .parser import QAPISchemaParser
+from .parser import QAPIExpression, QAPISchemaParser
 
 
 class QAPISchemaIfCond:
@@ -964,10 +964,11 @@  def module_by_fname(self, fname):
         name = self._module_name(fname)
         return self._module_dict[name]
 
-    def _def_include(self, expr, info, doc):
+    def _def_include(self, expr: QAPIExpression):
         include = expr['include']
-        assert doc is None
-        self._def_entity(QAPISchemaInclude(self._make_module(include), info))
+        assert expr.doc is None
+        self._def_entity(
+            QAPISchemaInclude(self._make_module(include), expr.info))
 
     def _def_builtin_type(self, name, json_type, c_type):
         self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
@@ -1045,14 +1046,15 @@  def _make_implicit_object_type(self, name, info, ifcond, role, members):
                 name, info, None, ifcond, None, None, members, None))
         return name
 
-    def _def_enum_type(self, expr, info, doc):
+    def _def_enum_type(self, expr: QAPIExpression):
         name = expr['enum']
         data = expr['data']
         prefix = expr.get('prefix')
         ifcond = QAPISchemaIfCond(expr.get('if'))
+        info = expr.info
         features = self._make_features(expr.get('features'), info)
         self._def_entity(QAPISchemaEnumType(
-            name, info, doc, ifcond, features,
+            name, info, expr.doc, ifcond, features,
             self._make_enum_members(data, info), prefix))
 
     def _make_member(self, name, typ, ifcond, features, info):
@@ -1072,14 +1074,15 @@  def _make_members(self, data, info):
                                   value.get('features'), info)
                 for (key, value) in data.items()]
 
-    def _def_struct_type(self, expr, info, doc):
+    def _def_struct_type(self, expr: QAPIExpression):
         name = expr['struct']
         base = expr.get('base')
         data = expr['data']
+        info = expr.info
         ifcond = QAPISchemaIfCond(expr.get('if'))
         features = self._make_features(expr.get('features'), info)
         self._def_entity(QAPISchemaObjectType(
-            name, info, doc, ifcond, features, base,
+            name, info, expr.doc, ifcond, features, base,
             self._make_members(data, info),
             None))
 
@@ -1089,11 +1092,13 @@  def _make_variant(self, case, typ, ifcond, info):
             typ = self._make_array_type(typ[0], info)
         return QAPISchemaVariant(case, info, typ, ifcond)
 
-    def _def_union_type(self, expr, info, doc):
+    def _def_union_type(self, expr: QAPIExpression):
         name = expr['union']
         base = expr['base']
         tag_name = expr['discriminator']
         data = expr['data']
+        assert isinstance(data, dict)
+        info = expr.info
         ifcond = QAPISchemaIfCond(expr.get('if'))
         features = self._make_features(expr.get('features'), info)
         if isinstance(base, dict):
@@ -1105,17 +1110,19 @@  def _def_union_type(self, expr, info, doc):
                                QAPISchemaIfCond(value.get('if')),
                                info)
             for (key, value) in data.items()]
-        members = []
+        members: List[QAPISchemaObjectTypeMember] = []
         self._def_entity(
-            QAPISchemaObjectType(name, info, doc, ifcond, features,
+            QAPISchemaObjectType(name, info, expr.doc, ifcond, features,
                                  base, members,
                                  QAPISchemaVariants(
                                      tag_name, info, None, variants)))
 
-    def _def_alternate_type(self, expr, info, doc):
+    def _def_alternate_type(self, expr: QAPIExpression):
         name = expr['alternate']
         data = expr['data']
+        assert isinstance(data, dict)
         ifcond = QAPISchemaIfCond(expr.get('if'))
+        info = expr.info
         features = self._make_features(expr.get('features'), info)
         variants = [
             self._make_variant(key, value['type'],
@@ -1124,11 +1131,11 @@  def _def_alternate_type(self, expr, info, doc):
             for (key, value) in data.items()]
         tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False)
         self._def_entity(
-            QAPISchemaAlternateType(name, info, doc, ifcond, features,
-                                    QAPISchemaVariants(
-                                        None, info, tag_member, variants)))
+            QAPISchemaAlternateType(
+                name, info, expr.doc, ifcond, features,
+                QAPISchemaVariants(None, info, tag_member, variants)))
 
-    def _def_command(self, expr, info, doc):
+    def _def_command(self, expr: QAPIExpression):
         name = expr['command']
         data = expr.get('data')
         rets = expr.get('returns')
@@ -1139,6 +1146,7 @@  def _def_command(self, expr, info, doc):
         allow_preconfig = expr.get('allow-preconfig', False)
         coroutine = expr.get('coroutine', False)
         ifcond = QAPISchemaIfCond(expr.get('if'))
+        info = expr.info
         features = self._make_features(expr.get('features'), info)
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
@@ -1147,44 +1155,42 @@  def _def_command(self, expr, info, doc):
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
-        self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, features,
-                                           data, rets,
+        self._def_entity(QAPISchemaCommand(name, info, expr.doc, ifcond,
+                                           features, data, rets,
                                            gen, success_response,
                                            boxed, allow_oob, allow_preconfig,
                                            coroutine))
 
-    def _def_event(self, expr, info, doc):
+    def _def_event(self, expr: QAPIExpression):
         name = expr['event']
         data = expr.get('data')
         boxed = expr.get('boxed', False)
         ifcond = QAPISchemaIfCond(expr.get('if'))
+        info = expr.info
         features = self._make_features(expr.get('features'), info)
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
                 name, info, ifcond,
                 'arg', self._make_members(data, info))
-        self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, features,
-                                         data, boxed))
+        self._def_entity(QAPISchemaEvent(name, info, expr.doc, ifcond,
+                                         features, data, boxed))
 
     def _def_exprs(self, exprs):
-        for expr_elem in exprs:
-            expr = expr_elem['expr']
-            info = expr_elem['info']
-            doc = expr_elem.get('doc')
+        for expr in exprs:
             if 'enum' in expr:
-                self._def_enum_type(expr, info, doc)
+                self._def_enum_type(expr)
             elif 'struct' in expr:
-                self._def_struct_type(expr, info, doc)
+                self._def_struct_type(expr)
             elif 'union' in expr:
-                self._def_union_type(expr, info, doc)
+                self._def_union_type(expr)
             elif 'alternate' in expr:
-                self._def_alternate_type(expr, info, doc)
+                self._def_alternate_type(expr)
             elif 'command' in expr:
-                self._def_command(expr, info, doc)
+                self._def_command(expr)
             elif 'event' in expr:
-                self._def_event(expr, info, doc)
+                self._def_event(expr)
             elif 'include' in expr:
-                self._def_include(expr, info, doc)
+                self._def_include(expr)
             else:
                 assert False