Re: [PATCH v6 14/19] qapi/introspect.py: add type hint annotations

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> NB: The type aliases (SchemaInfo et al) declare intent for some of the
> "dictly-typed" objects we pass around in introspect.py. They do not
> enforce the shape of those objects, and cannot, until Python 3.7 or
> later. (And even then, it may not be "worth it".)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/introspect.py | 124 +++++++++++++++++++++++++++----------
>  scripts/qapi/mypy.ini      |   5 --
>  scripts/qapi/schema.py     |   2 +-
>  3 files changed, 92 insertions(+), 39 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index b0fcc4443c1..45284af1330 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -17,6 +17,7 @@
>      Iterable,
>      List,
>      Optional,
> +    Sequence,
>      Tuple,
>      TypeVar,
>      Union,
> @@ -30,10 +31,19 @@
>  )
>  from .gen import QAPISchemaMonolithicCVisitor
>  from .schema import (
> +    QAPISchema,
>      QAPISchemaArrayType,
>      QAPISchemaBuiltinType,
> +    QAPISchemaEntity,
> +    QAPISchemaEnumMember,
> +    QAPISchemaFeature,
> +    QAPISchemaObjectType,
> +    QAPISchemaObjectTypeMember,
>      QAPISchemaType,
> +    QAPISchemaVariant,
> +    QAPISchemaVariants,
>  )
> +from .source import QAPISourceInfo
>  
>  
>  # This module constructs a tree data structure that is used to
> @@ -58,6 +68,15 @@
>  _Value = Union[_Scalar, _NonScalar]
>  JSONValue = Union[_Value, 'Annotated[_Value]']
>  
> +# These types are based on structures defined in QEMU's schema, so we lack
> +# precise types for them here. Python 3.6 does not offer TypedDict 
> constructs,
> +# so they are broadly typed here as simple Python Dicts.

PEP 8: "For flowing long blocks of text with fewer structural
restrictions (docstrings or comments), the line length should be limited
to 72 characters."

> +SchemaInfo = Dict[str, object]
> +SchemaInfoObject = Dict[str, object]
> +SchemaInfoObjectVariant = Dict[str, object]
> +SchemaInfoObjectMember = Dict[str, object]
> +SchemaInfoCommand = Dict[str, object]
> +
>  
>  _ValueT = TypeVar('_ValueT', bound=_Value)
>  
> @@ -77,9 +96,11 @@ def __init__(self, value: _ValueT, ifcond: Iterable[str],
>          self.ifcond: Tuple[str, ...] = tuple(ifcond)
>  
>  
> -def _tree_to_qlit(obj, level=0, dict_value=False):
> +def _tree_to_qlit(obj: JSONValue,
> +                  level: int = 0,
> +                  dict_value: bool = False) -> str:
>  
> -    def indent(level):
> +    def indent(level: int) -> str:
>          return level * 4 * ' '
>  
>      if isinstance(obj, Annotated):
> @@ -136,21 +157,21 @@ def indent(level):
>      return ret
>  
>  
> -def to_c_string(string):
> +def to_c_string(string: str) -> str:
>      return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
>  
>  
>  class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
>  
> -    def __init__(self, prefix, unmask):
> +    def __init__(self, prefix: str, unmask: bool):
>          super().__init__(
>              prefix, 'qapi-introspect',
>              ' * QAPI/QMP schema introspection', __doc__)
>          self._unmask = unmask
> -        self._schema = None
> -        self._trees = []
> -        self._used_types = []
> -        self._name_map = {}
> +        self._schema: Optional[QAPISchema] = None
> +        self._trees: List[Annotated[SchemaInfo]] = []
> +        self._used_types: List[QAPISchemaType] = []
> +        self._name_map: Dict[str, str] = {}
>          self._genc.add(mcgen('''
>  #include "qemu/osdep.h"
>  #include "%(prefix)sqapi-introspect.h"
> @@ -158,10 +179,10 @@ def __init__(self, prefix, unmask):
>  ''',
>                               prefix=prefix))
>  
> -    def visit_begin(self, schema):
> +    def visit_begin(self, schema: QAPISchema) -> None:
>          self._schema = schema
>  
> -    def visit_end(self):
> +    def visit_end(self) -> None:
>          # visit the types that are actually used
>          for typ in self._used_types:
>              typ.visit(self)
> @@ -183,18 +204,18 @@ def visit_end(self):
>          self._used_types = []
>          self._name_map = {}
>  
> -    def visit_needed(self, entity):
> +    def visit_needed(self, entity: QAPISchemaEntity) -> bool:
>          # Ignore types on first pass; visit_end() will pick up used types
>          return not isinstance(entity, QAPISchemaType)
>  
> -    def _name(self, name):
> +    def _name(self, name: str) -> str:
>          if self._unmask:
>              return name
>          if name not in self._name_map:
>              self._name_map[name] = '%d' % len(self._name_map)
>          return self._name_map[name]
>  
> -    def _use_type(self, typ):
> +    def _use_type(self, typ: QAPISchemaType) -> str:
>          assert self._schema is not None
>  
>          # Map the various integer types to plain int
> @@ -216,10 +237,13 @@ def _use_type(self, typ):
>          return self._name(typ.name)
>  
>      @staticmethod
> -    def _gen_features(features):
> +    def _gen_features(features: List[QAPISchemaFeature]
> +                      ) -> List[Annotated[str]]:
>          return [Annotated(f.name, f.ifcond) for f in features]
>  
> -    def _gen_tree(self, name, mtype, obj, ifcond, features):
> +    def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],

Schould this be obj: SchemaInfo?

> +                  ifcond: Sequence[str],
> +                  features: Optional[List[QAPISchemaFeature]]) -> None:
>          comment: Optional[str] = None
>          if mtype not in ('command', 'event', 'builtin', 'array'):
>              if not self._unmask:
> @@ -233,42 +257,65 @@ def _gen_tree(self, name, mtype, obj, ifcond, features):
>              obj['features'] = self._gen_features(features)
>          self._trees.append(Annotated(obj, ifcond, comment))
>  
> -    def _gen_member(self, member):
> -        obj = {'name': member.name, 'type': self._use_type(member.type)}
> +    def _gen_member(self, member: QAPISchemaObjectTypeMember
> +                    ) -> Annotated[SchemaInfoObjectMember]:
> +        obj: SchemaInfoObjectMember = {
> +            'name': member.name,
> +            'type': self._use_type(member.type)
> +        }
>          if member.optional:
>              obj['default'] = None
>          if member.features:
>              obj['features'] = self._gen_features(member.features)
>          return Annotated(obj, member.ifcond)
>  
> -    def _gen_variant(self, variant):
> -        obj = {'case': variant.name, 'type': self._use_type(variant.type)}
> +    def _gen_variant(self, variant: QAPISchemaVariant
> +                     ) -> Annotated[SchemaInfoObjectVariant]:
> +        obj: SchemaInfoObjectVariant = {
> +            'case': variant.name,
> +            'type': self._use_type(variant.type)
> +        }
>          return Annotated(obj, variant.ifcond)
>  
> -    def visit_builtin_type(self, name, info, json_type):
> +    def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
> +                           json_type: str) -> None:
>          self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None)
>  
> -    def visit_enum_type(self, name, info, ifcond, features, members, prefix):
> +    def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
> +                        ifcond: Sequence[str],
> +                        features: List[QAPISchemaFeature],
> +                        members: List[QAPISchemaEnumMember],
> +                        prefix: Optional[str]) -> None:
>          self._gen_tree(
>              name, 'enum',
>              {'values': [Annotated(m.name, m.ifcond) for m in members]},
>              ifcond, features
>          )
>  
> -    def visit_array_type(self, name, info, ifcond, element_type):
> +    def visit_array_type(self, name: str, info: Optional[QAPISourceInfo],
> +                         ifcond: Sequence[str],
> +                         element_type: QAPISchemaType) -> None:
>          element = self._use_type(element_type)
>          self._gen_tree('[' + element + ']', 'array', {'element-type': 
> element},
>                         ifcond, None)
>  
> -    def visit_object_type_flat(self, name, info, ifcond, features,
> -                               members, variants):
> -        obj = {'members': [self._gen_member(m) for m in members]}
> +    def visit_object_type_flat(self, name: str, info: 
> Optional[QAPISourceInfo],
> +                               ifcond: Sequence[str],
> +                               features: List[QAPISchemaFeature],
> +                               members: List[QAPISchemaObjectTypeMember],
> +                               variants: Optional[QAPISchemaVariants]) -> 
> None:
> +        obj: SchemaInfoObject = {
> +            'members': [self._gen_member(m) for m in members]
> +        }
>          if variants:
>              obj['tag'] = variants.tag_member.name
>              obj['variants'] = [self._gen_variant(v) for v in 
> variants.variants]
>          self._gen_tree(name, 'object', obj, ifcond, features)
>  
> -    def visit_alternate_type(self, name, info, ifcond, features, variants):
> +    def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
> +                             ifcond: Sequence[str],
> +                             features: List[QAPISchemaFeature],
> +                             variants: QAPISchemaVariants) -> None:
>          self._gen_tree(
>              name, 'alternate',
>              {'members': [Annotated({'type': self._use_type(m.type)},
> @@ -277,27 +324,38 @@ def visit_alternate_type(self, name, info, ifcond, 
> features, variants):
>              ifcond, features
>          )
>  
> -    def visit_command(self, name, info, ifcond, features,
> -                      arg_type, ret_type, gen, success_response, boxed,
> -                      allow_oob, allow_preconfig, coroutine):
> +    def visit_command(self, name: str, info: Optional[QAPISourceInfo],
> +                      ifcond: Sequence[str],
> +                      features: List[QAPISchemaFeature],
> +                      arg_type: Optional[QAPISchemaObjectType],
> +                      ret_type: Optional[QAPISchemaType], gen: bool,
> +                      success_response: bool, boxed: bool, allow_oob: bool,
> +                      allow_preconfig: bool, coroutine: bool) -> None:
>          assert self._schema is not None
>  
>          arg_type = arg_type or self._schema.the_empty_object_type
>          ret_type = ret_type or self._schema.the_empty_object_type
> -        obj = {'arg-type': self._use_type(arg_type),
> -               'ret-type': self._use_type(ret_type)}
> +        obj: SchemaInfoCommand = {
> +            'arg-type': self._use_type(arg_type),
> +            'ret-type': self._use_type(ret_type)
> +        }
>          if allow_oob:
>              obj['allow-oob'] = allow_oob
>          self._gen_tree(name, 'command', obj, ifcond, features)
>  
> -    def visit_event(self, name, info, ifcond, features, arg_type, boxed):
> +    def visit_event(self, name: str, info: Optional[QAPISourceInfo],
> +                    ifcond: Sequence[str], features: List[QAPISchemaFeature],
> +                    arg_type: Optional[QAPISchemaObjectType],
> +                    boxed: bool) -> None:
>          assert self._schema is not None
> +
>          arg_type = arg_type or self._schema.the_empty_object_type
>          self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)},
>                         ifcond, features)
>  
>  
> -def gen_introspect(schema, output_dir, prefix, opt_unmask):
> +def gen_introspect(schema: QAPISchema, output_dir: str, prefix: str,
> +                   opt_unmask: bool) -> None:
>      vis = QAPISchemaGenIntrospectVisitor(prefix, opt_unmask)
>      schema.visit(vis)
>      vis.write(output_dir)
> diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
> index 04bd5db5278..0a000d58b37 100644
> --- a/scripts/qapi/mypy.ini
> +++ b/scripts/qapi/mypy.ini
> @@ -13,11 +13,6 @@ disallow_untyped_defs = False
>  disallow_incomplete_defs = False
>  check_untyped_defs = False
>  
> -[mypy-qapi.introspect]
> -disallow_untyped_defs = False
> -disallow_incomplete_defs = False
> -check_untyped_defs = False
> -
>  [mypy-qapi.parser]
>  disallow_untyped_defs = False
>  disallow_incomplete_defs = False
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 353e8020a27..ff16578f6de 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -28,7 +28,7 @@
>  class QAPISchemaEntity:
>      meta: Optional[str] = None
>  
> -    def __init__(self, name, info, doc, ifcond=None, features=None):
> +    def __init__(self, name: str, info, doc, ifcond=None, features=None):
>          assert name is None or isinstance(name, str)
>          for f in features or []:
>              assert isinstance(f, QAPISchemaFeature)

How is this hunk related to typing introspect.py