John Snow <js...@redhat.com> writes:
> Add some aliases that declare intent for some of the "dictly-typed"
> objects we pass around in introspect.py.
>
> Signed-off-by: John Snow <js...@redhat.com>
>
> ---
>
> This patch is optional, it can be dropped if desired. If it's taken,
> it's probably best to squash it with the prior patch. It is purely for
> the sake of demonstrating what the _DObject was meant to convey: a
> Python Dict that represents some JSON Object. It does not add any type
> safety in and of itself, but does have some (light) annotational
> benefit. In this case, it's usually a specific data structure from the
> QAPI Schema we are referencing, but do not have "compile-time" access to
> in Python.
>
> These are loosely typed and don't bother reproducing the exact structure
> of the real types. Python 3.6 does not have support for TypedDict
> structures, so this is as good as we can do without involving a
> third-party library (e.g. Pydantic), in which we might be able to say:
>
>
> class SchemaMetaType(str, enum.Enum):
> BUILTIN = "builtin"
> ENUM = "enum"
> ARRAY = "array"
> OBJECT = "object"
> ALTERNATE = "alternate"
> COMMAND = "command"
> EVENT = "event"
>
>
> class SchemaInfo(pydantic.BaseModel):
> name: str
> meta-type: SchemaMetaType
> features: Optional[List[str]]
> data: Union[SchemaInfoBuiltin, SchemaInfoEnum, SchemaInfoArray,
> SchemaInfoObject, SchemaInfoAlternate, SchemaInfoCommand,
> SchemaInfoEvent]
>
>
> However, the cost of reproducing and keeping these structure definitions
> in-sync between the user-defined portion of the schema and the code
> generator is likely not worth doing any such thing. However, it does
> illustrate an interesting dependency the generator has on the
> user-defined schema itself in terms of types.
>
> So, I settled on using some light types that suggest the form of the
> object instead of enforcing the form.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> scripts/qapi/introspect.py | 51 +++++++++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 3afcdda7446..2a39726f40a 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -66,10 +66,15 @@
> _value = Union[_scalar, _nonscalar]
> TreeValue = Union[_value, 'Annotated[_value]']
>
> -# This is an alias for an arbitrary JSON object, represented here as a Dict.
> -# It is stricter on the value type than the recursive definition above.
> -# It is used to represent SchemaInfo-related structures exclusively.
> -_DObject = Dict[str, object]
> +# 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.
> +SchemaInfo = Dict[str, object]
> +SchemaInfoObject = Dict[str, object]
> +SchemaInfoObjectVariant = Dict[str, object]
> +SchemaInfoObjectMember = Dict[str, object]
> +SchemaInfoCommand = Dict[str, object]
> +
>
> _NodeT = TypeVar('_NodeT', bound=_value)
>
> @@ -162,7 +167,7 @@ def __init__(self, prefix: str, unmask: bool):
> ' * QAPI/QMP schema introspection', __doc__)
> self._unmask = unmask
> self._schema: Optional[QAPISchema] = None
> - self._trees: List[Annotated[_DObject]] = []
> + self._trees: List[Annotated[SchemaInfo]] = []
> self._used_types: List[QAPISchemaType] = []
> self._name_map: Dict[str, str] = {}
> self._genc.add(mcgen('''
> @@ -234,9 +239,18 @@ def _gen_features(features: List[QAPISchemaFeature]
> ) -> List[Annotated[str]]:
> return [Annotated(f.name, f.ifcond) for f in features]
>
> - def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> + def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
> ifcond: List[str],
> features: Optional[List[QAPISchemaFeature]]) -> None:
> + """
> + Build and append a SchemaInfo object to self._trees.
> +
> + :param name: The entity's name.
> + :param mtype: The entity's meta-type.
> + :param obj: Additional entity fields, as appropriate for the
> meta-type.
> + :param ifcond: List of conditionals that apply to this entire entity.
> + :param features: Optional features field for SchemaInfo.
> + """
> comment: Optional[str] = None
> if mtype not in ('command', 'event', 'builtin', 'array'):
> if not self._unmask:
> @@ -251,8 +265,8 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> self._trees.append(Annotated(obj, ifcond, comment))
>
> def _gen_member(self, member: QAPISchemaObjectTypeMember
> - ) -> Annotated[_DObject]:
> - obj: _DObject = {
> + ) -> Annotated[SchemaInfoObjectMember]:
> + obj: SchemaInfoObjectMember = {
> 'name': member.name,
> 'type': self._use_type(member.type)
> }
> @@ -262,13 +276,9 @@ def _gen_member(self, member: QAPISchemaObjectTypeMember
> obj['features'] = self._gen_features(member.features)
> return Annotated(obj, member.ifcond)
>
> - def _gen_variants(self, tag_name: str,
> - variants: List[QAPISchemaVariant]) -> _DObject:
> - return {'tag': tag_name,
> - 'variants': [self._gen_variant(v) for v in variants]}
> -
> - def _gen_variant(self, variant: QAPISchemaVariant) ->
> Annotated[_DObject]:
> - obj: _DObject = {
> + def _gen_variant(self, variant: QAPISchemaVariant
> + ) -> Annotated[SchemaInfoObjectVariant]:
> + obj: SchemaInfoObjectVariant = {
> 'case': variant.name,
> 'type': self._use_type(variant.type)
> }
> @@ -300,11 +310,12 @@ def visit_object_type_flat(self, name: str, info:
> Optional[QAPISourceInfo],
> features: List[QAPISchemaFeature],
> members: List[QAPISchemaObjectTypeMember],
> variants: Optional[QAPISchemaVariants]) ->
> None:
> - obj: _DObject = {'members': [self._gen_member(m) for m in members]}
> + obj: SchemaInfoObject = {
> + 'members': [self._gen_member(m) for m in members]
> + }
> if variants:
> - obj.update(self._gen_variants(variants.tag_member.name,
> - variants.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: str, info: Optional[QAPISourceInfo],
> @@ -329,7 +340,7 @@ def visit_command(self, name: str, info:
> Optional[QAPISourceInfo],
>
> arg_type = arg_type or self._schema.the_empty_object_type
> ret_type = ret_type or self._schema.the_empty_object_type
> - obj: _DObject = {
> + obj: SchemaInfoCommand = {
> 'arg-type': self._use_type(arg_type),
> 'ret-type': self._use_type(ret_type)
> }
I see what you mean by "some (light) annotational benefit".
I figure some of is simply due to going from _DObject, a name that tells
me nothing, to SchemaInfo, which makes me go aha!
The remainder is having the subtypes of SchemaInfo, too. albeit without
actual type checking. Worthwhile?
I agree it should be squashed if we want it, or parts of it.
