| Message ID | 20200922210101.4081073-33-jsnow@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
On Tue, Sep 22, 2020 at 05:00:55PM -0400, John Snow wrote: > Replacing the un-typed tuple, add a typed Node that we can add typed > metadata to. > > Signed-off-by: John Snow <jsnow@redhat.com> This is the most complex patch so far, and it's very hard to understand what it does without type annotations. Have you consider adding type annotations to the code before patch 30/38 (even if using `object` in some parts), so we can make this easier to review? In case it's useful, below is an attempt to add type annotations to the old code. It can be applied after patch 29/38. It reuses portions of patch 33/38. Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- scripts/qapi/introspect.py | 138 ++++++++++++++++++++++++++----------- scripts/qapi/mypy.ini | 5 -- scripts/qapi/schema.py | 2 +- 3 files changed, 100 insertions(+), 45 deletions(-) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index b036fcf9ce7..4eaebdef58b 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -10,6 +10,17 @@ This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. """ +from typing import ( + Dict, + Generic, + List, + NamedTuple, + Optional, + Sequence, + TypeVar, + Tuple +) + from .common import ( c_name, gen_endif, @@ -17,25 +28,48 @@ from .common import ( mcgen, ) from .gen import QAPISchemaMonolithicCVisitor -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, - QAPISchemaType) - - -def _make_tree(obj, ifcond, features, extra=None): +from .schema import ( + QAPISchema, + QAPISchemaArrayType, + QAPISchemaBuiltinType, + QAPISchemaEntity, + QAPISchemaEnumMember, + QAPISchemaFeature, + QAPISchemaObjectType, + QAPISchemaObjectTypeMember, + QAPISchemaType, + QAPISchemaVariant, + QAPISchemaVariants, +) +from .source import QAPISourceInfo + +T = TypeVar('T') +# this should actually be: Union[str, list, dict, bool, 'AnnotatedNode'] +# but mypy doesn't support recursive types +TreeNode = object +TreeDict = Dict[str, TreeNode] +Extra = Dict[str, object] +AnnotatedNode = Tuple[T, Extra] + +def _make_tree(obj: TreeDict, ifcond: List[str], + features: List[QAPISchemaFeature], + extra: Optional[Extra] = None) -> TreeNode: if extra is None: extra = {} if ifcond: extra['if'] = ifcond if features: - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] + obj['features'] = ([(f.name, {'if': f.ifcond}) for f in features]) if extra: return (obj, extra) return obj -def _tree_to_qlit(obj, level=0, suppress_first_indent=False): +def _tree_to_qlit(obj: TreeNode, + level: int = 0, + suppress_first_indent : bool = False) -> str: - def indent(level): + def indent(level: int) -> str: return level * 4 * ' ' if isinstance(obj, tuple): @@ -85,21 +119,20 @@ def _tree_to_qlit(obj, level=0, suppress_first_indent=False): 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[TreeNode] = [] + self._used_types: List[QAPISchemaType] = [] + self._name_map: Dict[str, str] = {} self._genc.add(mcgen(''' #include "qemu/osdep.h" #include "%(prefix)sqapi-introspect.h" @@ -107,10 +140,10 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor): ''', 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) @@ -132,18 +165,18 @@ const QLitObject %(c_name)s = %(c_string)s; 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: # Map the various integer types to plain int if typ.json_type() == 'int': typ = self._schema.lookup_type('int') @@ -162,8 +195,10 @@ const QLitObject %(c_name)s = %(c_string)s; return '[' + self._use_type(typ.element_type) + ']' return self._name(typ.name) - def _gen_tree(self, name, mtype, obj, ifcond, features): - extra = None + def _gen_tree(self, name: str, mtype: str, obj: TreeDict, + ifcond: List[str], + features: Optional[List[QAPISchemaFeature]]) -> None: + extra: Extra = None if mtype not in ('command', 'event', 'builtin', 'array'): if not self._unmask: # Output a comment to make it easy to map masked names @@ -174,44 +209,60 @@ const QLitObject %(c_name)s = %(c_string)s; obj['meta-type'] = mtype self._trees.append(_make_tree(obj, ifcond, features, extra)) - def _gen_member(self, member): + def _gen_member(self, + member: QAPISchemaObjectTypeMember) -> TreeNode: obj = {'name': member.name, 'type': self._use_type(member.type)} if member.optional: obj['default'] = None return _make_tree(obj, member.ifcond, member.features) - def _gen_variants(self, tag_name, variants): + def _gen_variants(self, tag_name: str, + variants: List[QAPISchemaVariant]) -> TreeDict: return {'tag': tag_name, 'variants': [self._gen_variant(v) for v in variants]} - def _gen_variant(self, variant): + def _gen_variant(self, variant: QAPISchemaVariant) -> TreeNode: obj = {'case': variant.name, 'type': self._use_type(variant.type)} return _make_tree(obj, variant.ifcond, None) - 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: QAPISourceInfo, + ifcond: List[str], features: List[QAPISchemaFeature], + members: List[QAPISchemaEnumMember], + prefix: Optional[str]) -> None: self._gen_tree(name, 'enum', {'values': [_make_tree(m.name, m.ifcond, None) 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: List[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: List[str], + features: List[QAPISchemaFeature], + members: Sequence[QAPISchemaObjectTypeMember], + variants: Optional[QAPISchemaVariants]) -> None: + obj: TreeDict = {'members': [self._gen_member(m) for m in members]} if variants: obj.update(self._gen_variants(variants.tag_member.name, 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: QAPISourceInfo, + ifcond: List[str], + features: List[QAPISchemaFeature], + variants: QAPISchemaVariants) -> None: + self._gen_tree(name, 'alternate', {'members': [ _make_tree({'type': self._use_type(m.type)}, @@ -219,24 +270,33 @@ const QLitObject %(c_name)s = %(c_string)s; for m in variants.variants]}, ifcond, features) - def visit_command(self, name, info, ifcond, features, - arg_type, ret_type, gen, success_response, boxed, - allow_oob, allow_preconfig): + def visit_command(self, name: str, info: QAPISourceInfo, ifcond: List[str], + features: List[QAPISchemaFeature], + arg_type: QAPISchemaObjectType, + ret_type: Optional[QAPISchemaType], gen: bool, + success_response: bool, boxed: bool, allow_oob: bool, + allow_preconfig: bool) -> 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: TreeDict = { + '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: QAPISourceInfo, + ifcond: List[str], features: List[QAPISchemaFeature], + arg_type: QAPISchemaObjectType, boxed: bool) -> 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 dbfeda748cc..9ce8b56f225 100644 --- a/scripts/qapi/mypy.ini +++ b/scripts/qapi/mypy.ini @@ -19,11 +19,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 bb0cd717f1a..3023bab44b6 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -28,7 +28,7 @@ from .parser import QAPISchemaParser 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)
On 9/23/20 2:41 PM, Eduardo Habkost wrote: > On Tue, Sep 22, 2020 at 05:00:55PM -0400, John Snow wrote: >> Replacing the un-typed tuple, add a typed Node that we can add typed >> metadata to. >> >> Signed-off-by: John Snow <jsnow@redhat.com> > > This is the most complex patch so far, and it's very hard to > understand what it does without type annotations. > Mmm, sorry about that. > Have you consider adding type annotations to the code before > patch 30/38 (even if using `object` in some parts), so we can > make this easier to review? > If you feel like it isn't just noise to learn types we're about to destroy with something else, I'll try. > In case it's useful, below is an attempt to add type annotations > to the old code. It can be applied after patch 29/38. It reuses > portions of patch 33/38. > OK. End result is I will squash the Extra and Node types together, but I need to figure out how to make it look nice for the review history. Gimmie a few to make a mess and I'll try to put it back together afterward. > Signed-off-by: John Snow <jsnow@redhat.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > scripts/qapi/introspect.py | 138 ++++++++++++++++++++++++++----------- > scripts/qapi/mypy.ini | 5 -- > scripts/qapi/schema.py | 2 +- > 3 files changed, 100 insertions(+), 45 deletions(-) > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index b036fcf9ce7..4eaebdef58b 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -10,6 +10,17 @@ This work is licensed under the terms of the GNU GPL, version 2. > See the COPYING file in the top-level directory. > """ > > +from typing import ( > + Dict, > + Generic, > + List, > + NamedTuple, > + Optional, > + Sequence, > + TypeVar, > + Tuple > +) > + > from .common import ( > c_name, > gen_endif, > @@ -17,25 +28,48 @@ from .common import ( > mcgen, > ) > from .gen import QAPISchemaMonolithicCVisitor > -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, > - QAPISchemaType) > - > - > -def _make_tree(obj, ifcond, features, extra=None): > +from .schema import ( > + QAPISchema, > + QAPISchemaArrayType, > + QAPISchemaBuiltinType, > + QAPISchemaEntity, > + QAPISchemaEnumMember, > + QAPISchemaFeature, > + QAPISchemaObjectType, > + QAPISchemaObjectTypeMember, > + QAPISchemaType, > + QAPISchemaVariant, > + QAPISchemaVariants, > +) > +from .source import QAPISourceInfo > + > +T = TypeVar('T') > +# this should actually be: Union[str, list, dict, bool, 'AnnotatedNode'] > +# but mypy doesn't support recursive types > +TreeNode = object > +TreeDict = Dict[str, TreeNode] > +Extra = Dict[str, object] > +AnnotatedNode = Tuple[T, Extra] > + > +def _make_tree(obj: TreeDict, ifcond: List[str], > + features: List[QAPISchemaFeature], > + extra: Optional[Extra] = None) -> TreeNode: > if extra is None: > extra = {} > if ifcond: > extra['if'] = ifcond > if features: > - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] > + obj['features'] = ([(f.name, {'if': f.ifcond}) for f in features]) > if extra: > return (obj, extra) > return obj > > > -def _tree_to_qlit(obj, level=0, suppress_first_indent=False): > +def _tree_to_qlit(obj: TreeNode, > + level: int = 0, > + suppress_first_indent : bool = False) -> str: > > - def indent(level): > + def indent(level: int) -> str: > return level * 4 * ' ' > > if isinstance(obj, tuple): > @@ -85,21 +119,20 @@ def _tree_to_qlit(obj, level=0, suppress_first_indent=False): > 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[TreeNode] = [] > + self._used_types: List[QAPISchemaType] = [] > + self._name_map: Dict[str, str] = {} > self._genc.add(mcgen(''' > #include "qemu/osdep.h" > #include "%(prefix)sqapi-introspect.h" > @@ -107,10 +140,10 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor): > ''', > 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) > @@ -132,18 +165,18 @@ const QLitObject %(c_name)s = %(c_string)s; > 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: > # Map the various integer types to plain int > if typ.json_type() == 'int': > typ = self._schema.lookup_type('int') > @@ -162,8 +195,10 @@ const QLitObject %(c_name)s = %(c_string)s; > return '[' + self._use_type(typ.element_type) + ']' > return self._name(typ.name) > > - def _gen_tree(self, name, mtype, obj, ifcond, features): > - extra = None > + def _gen_tree(self, name: str, mtype: str, obj: TreeDict, > + ifcond: List[str], > + features: Optional[List[QAPISchemaFeature]]) -> None: > + extra: Extra = None > if mtype not in ('command', 'event', 'builtin', 'array'): > if not self._unmask: > # Output a comment to make it easy to map masked names > @@ -174,44 +209,60 @@ const QLitObject %(c_name)s = %(c_string)s; > obj['meta-type'] = mtype > self._trees.append(_make_tree(obj, ifcond, features, extra)) > > - def _gen_member(self, member): > + def _gen_member(self, > + member: QAPISchemaObjectTypeMember) -> TreeNode: > obj = {'name': member.name, 'type': self._use_type(member.type)} > if member.optional: > obj['default'] = None > return _make_tree(obj, member.ifcond, member.features) > > - def _gen_variants(self, tag_name, variants): > + def _gen_variants(self, tag_name: str, > + variants: List[QAPISchemaVariant]) -> TreeDict: > return {'tag': tag_name, > 'variants': [self._gen_variant(v) for v in variants]} > > - def _gen_variant(self, variant): > + def _gen_variant(self, variant: QAPISchemaVariant) -> TreeNode: > obj = {'case': variant.name, 'type': self._use_type(variant.type)} > return _make_tree(obj, variant.ifcond, None) > > - 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: QAPISourceInfo, > + ifcond: List[str], features: List[QAPISchemaFeature], > + members: List[QAPISchemaEnumMember], > + prefix: Optional[str]) -> None: > self._gen_tree(name, 'enum', > {'values': [_make_tree(m.name, m.ifcond, None) > 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: List[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: List[str], > + features: List[QAPISchemaFeature], > + members: Sequence[QAPISchemaObjectTypeMember], > + variants: Optional[QAPISchemaVariants]) -> None: > + obj: TreeDict = {'members': [self._gen_member(m) for m in members]} > if variants: > obj.update(self._gen_variants(variants.tag_member.name, > 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: QAPISourceInfo, > + ifcond: List[str], > + features: List[QAPISchemaFeature], > + variants: QAPISchemaVariants) -> None: > + > self._gen_tree(name, 'alternate', > {'members': [ > _make_tree({'type': self._use_type(m.type)}, > @@ -219,24 +270,33 @@ const QLitObject %(c_name)s = %(c_string)s; > for m in variants.variants]}, > ifcond, features) > > - def visit_command(self, name, info, ifcond, features, > - arg_type, ret_type, gen, success_response, boxed, > - allow_oob, allow_preconfig): > + def visit_command(self, name: str, info: QAPISourceInfo, ifcond: List[str], > + features: List[QAPISchemaFeature], > + arg_type: QAPISchemaObjectType, > + ret_type: Optional[QAPISchemaType], gen: bool, > + success_response: bool, boxed: bool, allow_oob: bool, > + allow_preconfig: bool) -> 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: TreeDict = { > + '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: QAPISourceInfo, > + ifcond: List[str], features: List[QAPISchemaFeature], > + arg_type: QAPISchemaObjectType, boxed: bool) -> 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 dbfeda748cc..9ce8b56f225 100644 > --- a/scripts/qapi/mypy.ini > +++ b/scripts/qapi/mypy.ini > @@ -19,11 +19,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 bb0cd717f1a..3023bab44b6 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -28,7 +28,7 @@ from .parser import QAPISchemaParser > 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) >
On 9/23/20 2:41 PM, Eduardo Habkost wrote: > On Tue, Sep 22, 2020 at 05:00:55PM -0400, John Snow wrote: >> Replacing the un-typed tuple, add a typed Node that we can add typed >> metadata to. >> >> Signed-off-by: John Snow <jsnow@redhat.com> > > This is the most complex patch so far, and it's very hard to > understand what it does without type annotations. > > Have you consider adding type annotations to the code before > patch 30/38 (even if using `object` in some parts), so we can > make this easier to review? > > In case it's useful, below is an attempt to add type annotations > to the old code. It can be applied after patch 29/38. It reuses > portions of patch 33/38. > > Signed-off-by: John Snow <jsnow@redhat.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > scripts/qapi/introspect.py | 138 ++++++++++++++++++++++++++----------- > scripts/qapi/mypy.ini | 5 -- > scripts/qapi/schema.py | 2 +- > 3 files changed, 100 insertions(+), 45 deletions(-) > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index b036fcf9ce7..4eaebdef58b 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -10,6 +10,17 @@ This work is licensed under the terms of the GNU GPL, version 2. > See the COPYING file in the top-level directory. > """ > > +from typing import ( > + Dict, > + Generic, > + List, > + NamedTuple, > + Optional, > + Sequence, > + TypeVar, > + Tuple > +) > + > from .common import ( > c_name, > gen_endif, > @@ -17,25 +28,48 @@ from .common import ( > mcgen, > ) > from .gen import QAPISchemaMonolithicCVisitor > -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, > - QAPISchemaType) > - > - > -def _make_tree(obj, ifcond, features, extra=None): > +from .schema import ( > + QAPISchema, > + QAPISchemaArrayType, > + QAPISchemaBuiltinType, > + QAPISchemaEntity, > + QAPISchemaEnumMember, > + QAPISchemaFeature, > + QAPISchemaObjectType, > + QAPISchemaObjectTypeMember, > + QAPISchemaType, > + QAPISchemaVariant, > + QAPISchemaVariants, > +) > +from .source import QAPISourceInfo > + > +T = TypeVar('T') > +# this should actually be: Union[str, list, dict, bool, 'AnnotatedNode'] > +# but mypy doesn't support recursive types > +TreeNode = object > +TreeDict = Dict[str, TreeNode] > +Extra = Dict[str, object] > +AnnotatedNode = Tuple[T, Extra] > + > +def _make_tree(obj: TreeDict, ifcond: List[str], ^^^^^^^^ Technically untrue! obj may be a SchemaInfo-like dict, or a string. (And I will tell you why mypy didn't catch this!) > + features: List[QAPISchemaFeature], > + extra: Optional[Extra] = None) -> TreeNode: > if extra is None: > extra = {} > if ifcond: > extra['if'] = ifcond > if features: > - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] > + obj['features'] = ([(f.name, {'if': f.ifcond}) for f in features]) > if extra: > return (obj, extra) > return obj > > > -def _tree_to_qlit(obj, level=0, suppress_first_indent=False): > +def _tree_to_qlit(obj: TreeNode, > + level: int = 0, > + suppress_first_indent : bool = False) -> str: > > - def indent(level): > + def indent(level: int) -> str: > return level * 4 * ' ' > > if isinstance(obj, tuple): > @@ -85,21 +119,20 @@ def _tree_to_qlit(obj, level=0, suppress_first_indent=False): > 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[TreeNode] = [] > + self._used_types: List[QAPISchemaType] = [] > + self._name_map: Dict[str, str] = {} > self._genc.add(mcgen(''' > #include "qemu/osdep.h" > #include "%(prefix)sqapi-introspect.h" > @@ -107,10 +140,10 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor): > ''', > 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) > @@ -132,18 +165,18 @@ const QLitObject %(c_name)s = %(c_string)s; > 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: > # Map the various integer types to plain int > if typ.json_type() == 'int': > typ = self._schema.lookup_type('int') > @@ -162,8 +195,10 @@ const QLitObject %(c_name)s = %(c_string)s; > return '[' + self._use_type(typ.element_type) + ']' > return self._name(typ.name) > > - def _gen_tree(self, name, mtype, obj, ifcond, features): > - extra = None > + def _gen_tree(self, name: str, mtype: str, obj: TreeDict, > + ifcond: List[str], > + features: Optional[List[QAPISchemaFeature]]) -> None: > + extra: Extra = None > if mtype not in ('command', 'event', 'builtin', 'array'): > if not self._unmask: > # Output a comment to make it easy to map masked names > @@ -174,44 +209,60 @@ const QLitObject %(c_name)s = %(c_string)s; > obj['meta-type'] = mtype > self._trees.append(_make_tree(obj, ifcond, features, extra)) > > - def _gen_member(self, member): > + def _gen_member(self, > + member: QAPISchemaObjectTypeMember) -> TreeNode: > obj = {'name': member.name, 'type': self._use_type(member.type)} > if member.optional: > obj['default'] = None > return _make_tree(obj, member.ifcond, member.features) > > - def _gen_variants(self, tag_name, variants): > + def _gen_variants(self, tag_name: str, > + variants: List[QAPISchemaVariant]) -> TreeDict: > return {'tag': tag_name, > 'variants': [self._gen_variant(v) for v in variants]} > > - def _gen_variant(self, variant): > + def _gen_variant(self, variant: QAPISchemaVariant) -> TreeNode: > obj = {'case': variant.name, 'type': self._use_type(variant.type)} > return _make_tree(obj, variant.ifcond, None) > > - 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: QAPISourceInfo, > + ifcond: List[str], features: List[QAPISchemaFeature], > + members: List[QAPISchemaEnumMember], > + prefix: Optional[str]) -> None: > self._gen_tree(name, 'enum', > {'values': [_make_tree(m.name, m.ifcond, None) > for m in members]}, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Over here, Members is List[QAPISchemaEnumMember], which is true enough, but we don't yet have a typing for the 'str' field. mypy fills in with the "Any" type, which is *assumed* to fill the type we want. Nope! It's a string. Once you add the type annotations for schema.py, this would fail. > ifcond, features) > > - def visit_array_type(self, name, info, ifcond, element_type): > + def visit_array_type(self, name: str, info: Optional[QAPISourceInfo], > + ifcond: List[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: List[str], > + features: List[QAPISchemaFeature], > + members: Sequence[QAPISchemaObjectTypeMember], > + variants: Optional[QAPISchemaVariants]) -> None: > + obj: TreeDict = {'members': [self._gen_member(m) for m in members]} > if variants: > obj.update(self._gen_variants(variants.tag_member.name, > 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: QAPISourceInfo, > + ifcond: List[str], > + features: List[QAPISchemaFeature], > + variants: QAPISchemaVariants) -> None: > + > self._gen_tree(name, 'alternate', > {'members': [ > _make_tree({'type': self._use_type(m.type)}, > @@ -219,24 +270,33 @@ const QLitObject %(c_name)s = %(c_string)s; > for m in variants.variants]}, > ifcond, features) > > - def visit_command(self, name, info, ifcond, features, > - arg_type, ret_type, gen, success_response, boxed, > - allow_oob, allow_preconfig): > + def visit_command(self, name: str, info: QAPISourceInfo, ifcond: List[str], > + features: List[QAPISchemaFeature], > + arg_type: QAPISchemaObjectType, > + ret_type: Optional[QAPISchemaType], gen: bool, > + success_response: bool, boxed: bool, allow_oob: bool, > + allow_preconfig: bool) -> 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: TreeDict = { > + '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: QAPISourceInfo, > + ifcond: List[str], features: List[QAPISchemaFeature], > + arg_type: QAPISchemaObjectType, boxed: bool) -> 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 dbfeda748cc..9ce8b56f225 100644 > --- a/scripts/qapi/mypy.ini > +++ b/scripts/qapi/mypy.ini > @@ -19,11 +19,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 bb0cd717f1a..3023bab44b6 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -28,7 +28,7 @@ from .parser import QAPISchemaParser > 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) >
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index e1edd0b179..e0f5007ab7 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -31,11 +31,18 @@ class Extra(NamedTuple): ifcond: Sequence[str] = tuple() -def _make_tree(obj, ifcond, - extra: Optional[Extra] = None): - comment = extra.comment if extra else None - extra = Extra(comment, ifcond) - return (obj, extra) +class Node: + """ + Node generally contains a SchemaInfo-like type (as a dict), + But it also used to wrap comments/ifconds around leaf value types. + """ + # Remove after 3.7 adds @dataclass: + # pylint: disable=too-few-public-methods + def __init__(self, data, ifcond: List[str], + extra: Optional[Extra] = None): + self.data = data + comment = extra.comment if extra else None + self.extra = Extra(comment, ifcond) def _tree_to_qlit(obj, level=0, suppress_first_indent=False): @@ -43,18 +50,15 @@ def _tree_to_qlit(obj, level=0, suppress_first_indent=False): def indent(level): return level * 4 * ' ' - if isinstance(obj, tuple): - ifobj, extra = obj - ifcond = extra.ifcond - comment = extra.comment + if isinstance(obj, Node): ret = '' - if comment: - ret += indent(level) + '/* %s */\n' % comment - if ifcond: - ret += gen_if(ifcond) - ret += _tree_to_qlit(ifobj, level) - if ifcond: - ret += '\n' + gen_endif(ifcond) + if obj.extra.comment: + ret += indent(level) + '/* %s */\n' % obj.extra.comment + if obj.extra.ifcond: + ret += gen_if(obj.extra.ifcond) + ret += _tree_to_qlit(obj.data, level) + if obj.extra.ifcond: + ret += '\n' + gen_endif(obj.extra.ifcond) return ret ret = '' @@ -169,7 +173,7 @@ def _use_type(self, typ): @classmethod def _gen_features(cls, features: List[QAPISchemaFeature]): - return [_make_tree(f.name, f.ifcond) for f in features] + return [Node(f.name, f.ifcond) for f in features] def _gen_tree(self, name, mtype, obj, ifcond, features): extra = None @@ -183,7 +187,7 @@ def _gen_tree(self, name, mtype, obj, ifcond, features): obj['meta-type'] = mtype if features: obj['features'] = self._gen_features(features) - self._trees.append(_make_tree(obj, ifcond, extra)) + self._trees.append(Node(obj, ifcond, extra)) def _gen_member(self, member): obj = {'name': member.name, 'type': self._use_type(member.type)} @@ -191,7 +195,7 @@ def _gen_member(self, member): obj['default'] = None if member.features: obj['features'] = self._gen_features(member.features) - return _make_tree(obj, member.ifcond) + return Node(obj, member.ifcond) def _gen_variants(self, tag_name, variants): return {'tag': tag_name, @@ -199,15 +203,14 @@ def _gen_variants(self, tag_name, variants): def _gen_variant(self, variant): obj = {'case': variant.name, 'type': self._use_type(variant.type)} - return _make_tree(obj, variant.ifcond) + return Node(obj, variant.ifcond) def visit_builtin_type(self, name, info, json_type): self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None) def visit_enum_type(self, name, info, ifcond, features, members, prefix): self._gen_tree(name, 'enum', - {'values': [_make_tree(m.name, m.ifcond, None) - for m in members]}, + {'values': [Node(m.name, m.ifcond) for m in members]}, ifcond, features) def visit_array_type(self, name, info, ifcond, element_type): @@ -227,9 +230,9 @@ def visit_object_type_flat(self, name, info, ifcond, features, def visit_alternate_type(self, name, info, ifcond, features, variants): self._gen_tree(name, 'alternate', {'members': [ - _make_tree({'type': self._use_type(m.type)}, - m.ifcond, None) - for m in variants.variants]}, + Node({'type': self._use_type(m.type)}, m.ifcond) + for m in variants.variants + ]}, ifcond, features) def visit_command(self, name, info, ifcond, features,
Replacing the un-typed tuple, add a typed Node that we can add typed metadata to. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/introspect.py | 53 ++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 25 deletions(-)