* [PATCH 00/25] qapi: static typing conversion, pt6 @ 2020-09-22 22:44 John Snow 2020-09-22 22:44 ` [PATCH 01/25] qapi/schema: add Visitable mixin John Snow ` (25 more replies) 0 siblings, 26 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa based-on: <20200922223525.4085762-1-jsnow@redhat.com> [PATCH 00/26] qapi: static typing conversion, pt5 Hi, this series adds static type hints to the QAPI module. This is the final part, part six! Part 6 (Everything): https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6 - Requires Python 3.6+ - Requires mypy 0.770 or newer (for type analysis only) - Requires pylint 2.6.0 or newer (for lint checking only) This part of the series focuses on schema.py. Type hints are added in patches that add *only* type hints and change no other behavior. Any necessary changes to behavior to accommodate typing are split out into their own tiny patches. Every commit should pass with: - flake8 qapi/ - pylint --rcfile=qapi/pylintrc qapi/ - mypy --config-file=qapi/mypy.ini qapi/ John Snow (25): qapi/schema: add Visitable mixin qapi/schema.py: Move meta-type into class instances qapi/schema.py: add assert in stub methods qapi/schema.py: constrain QAPISchemaObjectType base type qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type qapi/schema.py: constrain QAPISchemaEvent arg_type type qapi/schema.py: constrain tag_member type qapi/schema.py: Allow alternate_type to assert qapi/schema.py: remove superfluous assert qapi/schema.py: Add assertion to ifcond property qapi/schema.py: Constrain type of QAPISchemaObjectType members field qapi/schema.py: remove 'and' from non-bool rvalue expressions qapi/schema.py: Test type of self.ret_type instead of local temp qapi/schema.py: Assert variants of an object are also objects qapi/schema.py: add type hint annotations qapi/schema.py: enable checking qapi: Disable similarity checks in pylint entirely qapi/schema.py: Add pylint warning suppressions qapi/schema.py: Convert several methods to classmethods qapi/schema.py: Replace one-letter variable names qapi/schema.py: disable pylint line limit qapi/schema.py: Ignore unused argument for check() qapi/schema.py: enable pylint checks qapi/schema.py: Add module docstring qapi/schema.py: Use python3 style super() scripts/qapi/mypy.ini | 6 - scripts/qapi/pylintrc | 6 +- scripts/qapi/schema.py | 848 +++++++++++++++++++++++++++-------------- 3 files changed, 557 insertions(+), 303 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 01/25] qapi/schema: add Visitable mixin 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 02/25] qapi/schema.py: Move meta-type into class instances John Snow ` (24 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Python doesn't have anything quite exactly like Traits, Interfaces, or Mixins; but we can approximate it. Add a 'Visitable' class that enforces a type signature for the visit() method; this way, mypy is ensuring that even for otherwise unrelated classes that do not inherit from a common base, this signature will always be forced to be compatible. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 51af0449f5..55434f5c82 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -25,7 +25,13 @@ from .parser import QAPISchemaParser -class QAPISchemaEntity: +class Visitable: + """Abstract duck that suggests a class is visitable.""" + def visit(self, visitor: 'QAPISchemaVisitor') -> None: + raise NotImplementedError + + +class QAPISchemaEntity(Visitable): meta: Optional[str] = None def __init__(self, name: str, info, doc, ifcond=None, features=None): @@ -136,7 +142,7 @@ def visit_event(self, name, info, ifcond, features, arg_type, boxed): pass -class QAPISchemaModule: +class QAPISchemaModule(Visitable): def __init__(self, name): self.name = name self._entity_list = [] @@ -812,7 +818,7 @@ def visit(self, visitor): self.arg_type, self.boxed) -class QAPISchema: +class QAPISchema(Visitable): def __init__(self, fname): self.fname = fname parser = QAPISchemaParser(fname) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 02/25] qapi/schema.py: Move meta-type into class instances 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow 2020-09-22 22:44 ` [PATCH 01/25] qapi/schema: add Visitable mixin John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 03/25] qapi/schema.py: add assert in stub methods John Snow ` (23 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa We are using meta as a class variable, but union types use this as a mutable value which changes the value for the class, not the instance. Use the empty string as the default/empty value for ease of typing. It is still false-ish, so it will work just fine. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 55434f5c82..0201b42095 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -17,7 +17,6 @@ import os import re from collections import OrderedDict -from typing import Optional from .common import c_name, POINTER_SUFFIX from .error import QAPISourceError, QAPISemError @@ -32,8 +31,6 @@ def visit(self, visitor: 'QAPISchemaVisitor') -> None: class QAPISchemaEntity(Visitable): - meta: Optional[str] = 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 []: @@ -51,6 +48,15 @@ def __init__(self, name: str, info, doc, ifcond=None, features=None): self._ifcond = ifcond or [] self.features = features or [] self._checked = False + self._meta = '' + + @property + def meta(self) -> str: + return self._meta + + @meta.setter + def meta(self, value: str) -> None: + self._meta = value def c_name(self): return c_name(self.name) @@ -212,8 +218,6 @@ def describe(self): class QAPISchemaBuiltinType(QAPISchemaType): - meta = 'built-in' - def __init__(self, name, json_type, c_type): super().__init__(name, None, None) assert not c_type or isinstance(c_type, str) @@ -221,6 +225,7 @@ def __init__(self, name, json_type, c_type): 'value') self._json_type_name = json_type self._c_type_name = c_type + self._meta = 'built-in' def c_name(self): return self.name @@ -245,8 +250,6 @@ def visit(self, visitor): class QAPISchemaEnumType(QAPISchemaType): - meta = 'enum' - def __init__(self, name, info, doc, ifcond, features, members, prefix): super().__init__(name, info, doc, ifcond, features) for m in members: @@ -255,6 +258,7 @@ def __init__(self, name, info, doc, ifcond, features, members, prefix): assert prefix is None or isinstance(prefix, str) self.members = members self.prefix = prefix + self._meta = 'enum' def check(self, schema): super().check(schema) @@ -289,13 +293,12 @@ def visit(self, visitor): class QAPISchemaArrayType(QAPISchemaType): - meta = 'array' - def __init__(self, name, info, element_type): super().__init__(name, info, None) assert isinstance(element_type, str) self._element_type_name = element_type self.element_type = None + self._meta = 'array' def check(self, schema): super().check(schema) @@ -344,7 +347,7 @@ def __init__(self, name, info, doc, ifcond, features, # flat union has base, variants, and no local_members # simple union has local_members, variants, and no base super().__init__(name, info, doc, ifcond, features) - self.meta = 'union' if variants else 'struct' + self._meta = 'union' if variants else 'struct' assert base is None or isinstance(base, str) for m in local_members: assert isinstance(m, QAPISchemaObjectTypeMember) @@ -456,8 +459,6 @@ def visit(self, visitor): class QAPISchemaAlternateType(QAPISchemaType): - meta = 'alternate' - def __init__(self, name, info, doc, ifcond, features, variants): super().__init__(name, info, doc, ifcond, features) assert isinstance(variants, QAPISchemaVariants) @@ -465,6 +466,7 @@ def __init__(self, name, info, doc, ifcond, features, variants): variants.set_defined_in(name) variants.tag_member.set_defined_in(self.name) self.variants = variants + self._meta = 'alternate' def check(self, schema): super().check(schema) @@ -716,8 +718,6 @@ def __init__(self, name, info, typ, ifcond=None): class QAPISchemaCommand(QAPISchemaEntity): - meta = 'command' - def __init__(self, name, info, doc, ifcond, features, arg_type, ret_type, gen, success_response, boxed, allow_oob, allow_preconfig): @@ -733,6 +733,7 @@ def __init__(self, name, info, doc, ifcond, features, self.boxed = boxed self.allow_oob = allow_oob self.allow_preconfig = allow_preconfig + self._meta = 'command' def check(self, schema): super().check(schema) @@ -779,14 +780,13 @@ def visit(self, visitor): class QAPISchemaEvent(QAPISchemaEntity): - meta = 'event' - def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): super().__init__(name, info, doc, ifcond, features) assert not arg_type or isinstance(arg_type, str) self._arg_type_name = arg_type self.arg_type = None self.boxed = boxed + self._meta = 'event' def check(self, schema): super().check(schema) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 03/25] qapi/schema.py: add assert in stub methods 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow 2020-09-22 22:44 ` [PATCH 01/25] qapi/schema: add Visitable mixin John Snow 2020-09-22 22:44 ` [PATCH 02/25] qapi/schema.py: Move meta-type into class instances John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 04/25] qapi/schema.py: constrain QAPISchemaObjectType base type John Snow ` (22 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Instead of pass (an implicit return none), use raise NotImplementedError to mark a function as abstract -- one that doesn't return. This allows us to correctly type the stub. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 0201b42095..a53631e660 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -176,8 +176,8 @@ def visit(self, visitor): class QAPISchemaType(QAPISchemaEntity): # Return the C type for common use. # For the types we commonly box, this is a pointer type. - def c_type(self): - pass + def c_type(self) -> str: + raise NotImplementedError() # Return the C type to be used in a parameter list. def c_param_type(self): @@ -187,8 +187,8 @@ def c_param_type(self): def c_unboxed_type(self): return self.c_type() - def json_type(self): - pass + def json_type(self) -> str: + raise NotImplementedError() def alternate_qtype(self): json2qtype = { -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 04/25] qapi/schema.py: constrain QAPISchemaObjectType base type 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (2 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 03/25] qapi/schema.py: add assert in stub methods John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 05/25] qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type John Snow ` (21 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Re-order check slightly so we can provide a stronger guarantee on the typing of the base field. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index a53631e660..3aa842be08 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -378,14 +378,14 @@ def check(self, schema): seen = OrderedDict() if self._base_name: - self.base = schema.resolve_type(self._base_name, self.info, - "'base'") - if (not isinstance(self.base, QAPISchemaObjectType) - or self.base.variants): + base = schema.resolve_type(self._base_name, self.info, "'base'") + if (not isinstance(base, QAPISchemaObjectType) + or base.variants): raise QAPISemError( self.info, "'base' requires a struct type, %s isn't" - % self.base.describe()) + % base.describe()) + self.base = base self.base.check(schema) self.base.check_clash(self.info, seen) for m in self.local_members: -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 05/25] qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (3 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 04/25] qapi/schema.py: constrain QAPISchemaObjectType base type John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 06/25] qapi/schema.py: constrain QAPISchemaEvent " John Snow ` (20 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Re-order the check method slightly in order to provide stronger typing for the arg_type field. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 3aa842be08..c9a62711c2 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -738,13 +738,14 @@ def __init__(self, name, info, doc, ifcond, features, def check(self, schema): super().check(schema) if self._arg_type_name: - self.arg_type = schema.resolve_type( + arg_type = schema.resolve_type( self._arg_type_name, self.info, "command's 'data'") - if not isinstance(self.arg_type, QAPISchemaObjectType): + if not isinstance(arg_type, QAPISchemaObjectType): raise QAPISemError( self.info, "command's 'data' cannot take %s" - % self.arg_type.describe()) + % arg_type.describe()) + self.arg_type = arg_type if self.arg_type.variants and not self.boxed: raise QAPISemError( self.info, -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 06/25] qapi/schema.py: constrain QAPISchemaEvent arg_type type 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (4 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 05/25] qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 07/25] qapi/schema.py: constrain tag_member type John Snow ` (19 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index c9a62711c2..aaf20f776b 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -792,13 +792,14 @@ def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): def check(self, schema): super().check(schema) if self._arg_type_name: - self.arg_type = schema.resolve_type( + arg_type = schema.resolve_type( self._arg_type_name, self.info, "event's 'data'") - if not isinstance(self.arg_type, QAPISchemaObjectType): + if not isinstance(arg_type, QAPISchemaObjectType): raise QAPISemError( self.info, "event's 'data' cannot take %s" - % self.arg_type.describe()) + % arg_type.describe()) + self.arg_type = arg_type if self.arg_type.variants and not self.boxed: raise QAPISemError( self.info, -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 07/25] qapi/schema.py: constrain tag_member type 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (5 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 06/25] qapi/schema.py: constrain QAPISchemaEvent " John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 08/25] qapi/schema.py: Allow alternate_type to assert John Snow ` (18 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index aaf20f776b..15ff441660 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -546,15 +546,18 @@ def set_defined_in(self, name): def check(self, schema, seen): if not self.tag_member: # flat union - self.tag_member = seen.get(c_name(self._tag_name)) + tag_member = seen.get(c_name(self._tag_name)) base = "'base'" # Pointing to the base type when not implicit would be # nice, but we don't know it here - if not self.tag_member or self._tag_name != self.tag_member.name: + if not tag_member or self._tag_name != tag_member.name: raise QAPISemError( self.info, "discriminator '%s' is not a member of %s" % (self._tag_name, base)) + + assert isinstance(tag_member, QAPISchemaObjectTypeMember) + self.tag_member = tag_member # Here we do: base_type = schema.lookup_type(self.tag_member.defined_in) assert base_type -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 08/25] qapi/schema.py: Allow alternate_type to assert 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (6 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 07/25] qapi/schema.py: constrain tag_member type John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 09/25] qapi/schema.py: remove superfluous assert John Snow ` (17 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa It is generally nicer to just let things fail, because it makes the static type hints less infected with Optional[T], where a future programmer using the library has to wonder what that means. Let errors be errors. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 15ff441660..a84d8549a4 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -199,7 +199,7 @@ def alternate_qtype(self): 'boolean': 'QTYPE_QBOOL', 'object': 'QTYPE_QDICT' } - return json2qtype.get(self.json_type()) + return json2qtype[self.json_type()] def doc_type(self): if self.is_implicit(): @@ -480,8 +480,9 @@ def check(self, schema): types_seen = {} for v in self.variants.variants: v.check_clash(self.info, seen) - qtype = v.type.alternate_qtype() - if not qtype: + try: + qtype = v.type.alternate_qtype() + except KeyError: raise QAPISemError( self.info, "%s cannot use %s" -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 09/25] qapi/schema.py: remove superfluous assert 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (7 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 08/25] qapi/schema.py: Allow alternate_type to assert John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 10/25] qapi/schema.py: Add assertion to ifcond property John Snow ` (16 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa We'll actually get a better error message by just letting the dictionary lookup fail. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index a84d8549a4..b35f741c6f 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -892,7 +892,6 @@ def _make_module(self, fname): def module_by_fname(self, fname): name = self._module_name(fname) - assert name in self._module_dict return self._module_dict[name] def _def_include(self, expr, info, doc): -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 10/25] qapi/schema.py: Add assertion to ifcond property 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (8 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 09/25] qapi/schema.py: remove superfluous assert John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 11/25] qapi/schema.py: Constrain type of QAPISchemaObjectType members field John Snow ` (15 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa ifcond's initialization allows a fairly confusing type, but we want to assert that after the QAPISchema is built, this always returns a List[str]. Add an assertion to allow us to say that. (Note: Technically, I only assert that it's a list -- but type hints that will be added later will make it clear that the only possible list we could have here is a list[str], and this assertion is sufficient to prove the point to mypy.) Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index b35f741c6f..62b1a7e890 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -88,7 +88,7 @@ def set_module(self, schema): @property def ifcond(self): - assert self._checked + assert self._checked and isinstance(self._ifcond, list) return self._ifcond def is_implicit(self): -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 11/25] qapi/schema.py: Constrain type of QAPISchemaObjectType members field 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (9 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 10/25] qapi/schema.py: Add assertion to ifcond property John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 12/25] qapi/schema.py: remove 'and' from non-bool rvalue expressions John Snow ` (14 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa This must always be QAPISchemaObjectTypeMember, and we do check and enforce this -- but because the seen dict is invariant and check methods need to operate in terms of a more abstract type, we need to tell the type system to believe us. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 62b1a7e890..57343a1002 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -17,6 +17,7 @@ import os import re from collections import OrderedDict +from typing import cast, List from .common import c_name, POINTER_SUFFIX from .error import QAPISourceError, QAPISemError @@ -391,7 +392,10 @@ def check(self, schema): for m in self.local_members: m.check(schema) m.check_clash(self.info, seen) - members = seen.values() + + # check_clash is abstract, but local_members is asserted to be + # Sequence[QAPISchemaObjectTypeMember]. Cast to the narrower type. + members = cast(List[QAPISchemaObjectTypeMember], list(seen.values())) if self.variants: self.variants.check(schema, seen) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 12/25] qapi/schema.py: remove 'and' from non-bool rvalue expressions 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (10 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 11/25] qapi/schema.py: Constrain type of QAPISchemaObjectType members field John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 13/25] qapi/schema.py: Test type of self.ret_type instead of local temp John Snow ` (13 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa There's nothing inherently wrong with using 'and' in this manner, but the type returned is more loose than you might expect. Any value can be false-ish, so mypy assumes that the type of the expression must be the Union of both types, because a type can always be false-ish. To tighten the static analysis type, we have to use the slightly clunkier but more formally correct ternary. Fixes these errors: qapi/schema.py:103: error: Argument 1 to "module_by_fname" of "QAPISchema" has incompatible type "Union[QAPISourceInfo, str]"; expected "Optional[str]" qapi/schema.py:380: error: Argument 3 to "resolve_type" of "QAPISchema" has incompatible type "Union[QAPISourceInfo, str]"; expected "Union[str, Callable[[Optional[QAPISourceInfo]], str], None]" Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 57343a1002..943f234ee2 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -81,7 +81,7 @@ def check_doc(self): def _set_module(self, schema, info): assert self._checked - self._module = schema.module_by_fname(info and info.fname) + self._module = schema.module_by_fname(info.fname if info else None) self._module.add_entity(self) def set_module(self, schema): @@ -305,7 +305,7 @@ def check(self, schema): super().check(schema) self.element_type = schema.resolve_type( self._element_type_name, self.info, - self.info and self.info.defn_meta) + self.info.defn_meta if self.info else None) assert not isinstance(self.element_type, QAPISchemaArrayType) def set_module(self, schema): -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 13/25] qapi/schema.py: Test type of self.ret_type instead of local temp 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (11 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 12/25] qapi/schema.py: remove 'and' from non-bool rvalue expressions John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 14/25] qapi/schema.py: Assert variants of an object are also objects John Snow ` (12 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa This is obscure: If we test the type on a local copy instead of the stored state AFTER the assignment, mypy does not constrain the type of the copy. If we test on the stored state, it works out fine. Corrects this warning: qapi/schema.py:887: error: "QAPISchemaType" has no attribute "element_type" Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 943f234ee2..09c7ceab41 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -764,7 +764,7 @@ def check(self, schema): self._ret_type_name, self.info, "command's 'returns'") if self.name not in self.info.pragma.returns_whitelist: typ = self.ret_type - if isinstance(typ, QAPISchemaArrayType): + if isinstance(self.ret_type, QAPISchemaArrayType): typ = self.ret_type.element_type assert typ if not isinstance(typ, QAPISchemaObjectType): -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 14/25] qapi/schema.py: Assert variants of an object are also objects 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (12 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 13/25] qapi/schema.py: Test type of self.ret_type instead of local temp John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 15/25] qapi/schema.py: add type hint annotations John Snow ` (11 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa The 'seen' dictionaries are invariant types and require the most abstracted type to maintain a consistent interface with other check_clash implementations. In this case, we happen to know (and require) that they will be object types, so add a runtime assertion to constrain that type. Corrects this warning: qapi/schema.py:718: error: Item "QAPISchemaType" of "Optional[QAPISchemaType]" has no attribute "check_clash" Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 09c7ceab41..b502eee9cb 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -620,6 +620,7 @@ def check_clash(self, info, seen): for v in self.variants: # Reset seen map for each variant, since qapi names from one # branch do not affect another branch + assert isinstance(v.type, QAPISchemaObjectType) v.type.check_clash(info, dict(seen)) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 15/25] qapi/schema.py: add type hint annotations 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (13 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 14/25] qapi/schema.py: Assert variants of an object are also objects John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 16/25] qapi/schema.py: enable checking John Snow ` (10 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Annotations do not change runtime behavior. This commit *only* adds annotations. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 519 ++++++++++++++++++++++++++++------------- 1 file changed, 361 insertions(+), 158 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index b502eee9cb..8907bec0b5 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -14,15 +14,27 @@ # TODO catching name collisions in generated code would be nice -import os -import re from collections import OrderedDict -from typing import cast, List +import re +import os +from typing import ( + Any, + Callable, + Dict, + List, + Optional, + Type, + TypeVar, + Union, + cast, + overload, +) from .common import c_name, POINTER_SUFFIX from .error import QAPISourceError, QAPISemError from .expr import check_exprs -from .parser import QAPISchemaParser +from .parser import QAPIDoc, QAPISchemaParser, ParsedExpression +from .source import QAPISourceInfo class Visitable: @@ -32,13 +44,18 @@ def visit(self, visitor: 'QAPISchemaVisitor') -> None: class QAPISchemaEntity(Visitable): - def __init__(self, name: str, info, doc, ifcond=None, features=None): + def __init__(self, + name: str, + info: Optional[QAPISourceInfo], + doc: Optional[QAPIDoc], + ifcond: Optional[Union[List[str], 'QAPISchemaType']] = None, + features: Optional[List['QAPISchemaFeature']] = None): assert name is None or isinstance(name, str) for f in features or []: assert isinstance(f, QAPISchemaFeature) f.set_defined_in(name) self.name = name - self._module = None + self._module: Optional[QAPISchemaModule] = None # For explicitly defined entities, info points to the (explicit) # definition. For builtins (and their arrays), info is None. # For implicitly defined entities, info points to a place that @@ -59,105 +76,151 @@ def meta(self) -> str: def meta(self, value: str) -> None: self._meta = value - def c_name(self): + def c_name(self) -> str: return c_name(self.name) - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: assert not self._checked - seen = {} + seen: Dict[str, 'QAPISchemaMember'] = {} for f in self.features: f.check_clash(self.info, seen) self._checked = True - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: doc = doc or self.doc if doc: for f in self.features: doc.connect_feature(f) - def check_doc(self): + def check_doc(self) -> None: if self.doc: self.doc.check() - def _set_module(self, schema, info): + def _set_module(self, + schema: 'QAPISchema', + info: Optional[QAPISourceInfo]) -> None: assert self._checked self._module = schema.module_by_fname(info.fname if info else None) self._module.add_entity(self) - def set_module(self, schema): + def set_module(self, schema: 'QAPISchema') -> None: self._set_module(schema, self.info) @property - def ifcond(self): + def ifcond(self) -> List[str]: assert self._checked and isinstance(self._ifcond, list) return self._ifcond - def is_implicit(self): + def is_implicit(self) -> bool: return not self.info - def visit(self, visitor): + def visit(self, visitor: 'QAPISchemaVisitor') -> None: assert self._checked - def describe(self): + def describe(self) -> str: assert self.meta return "%s '%s'" % (self.meta, self.name) class QAPISchemaVisitor: - def visit_begin(self, schema): + def visit_begin(self, schema: 'QAPISchema') -> None: pass - def visit_end(self): + def visit_end(self) -> None: pass - def visit_module(self, name): + def visit_module(self, name: Optional[str]) -> None: pass - def visit_needed(self, entity): + def visit_needed(self, entity: QAPISchemaEntity) -> bool: # Default to visiting everything return True - def visit_include(self, name, info): + def visit_include(self, name: str, info: QAPISourceInfo) -> None: pass - def visit_builtin_type(self, name, info, json_type): + def visit_builtin_type(self, name: str, + info: Optional[QAPISourceInfo], + json_type: str) -> None: pass - def visit_enum_type(self, name, info, ifcond, features, members, prefix): + def visit_enum_type(self, + name: str, + info: Optional[QAPISourceInfo], + ifcond: List[str], + features: List['QAPISchemaFeature'], + members: List['QAPISchemaEnumMember'], + prefix: Optional[str]) -> None: pass - 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: pass - def visit_object_type(self, name, info, ifcond, features, - base, members, variants): + def visit_object_type(self, + name: str, + info: Optional[QAPISourceInfo], + ifcond: List[str], + features: List['QAPISchemaFeature'], + base: Optional['QAPISchemaObjectType'], + members: List['QAPISchemaObjectTypeMember'], + variants: Optional['QAPISchemaVariants']) -> None: pass - def visit_object_type_flat(self, name, info, ifcond, features, - members, variants): + def visit_object_type_flat(self, + name: str, + info: Optional[QAPISourceInfo], + ifcond: List[str], + features: List['QAPISchemaFeature'], + members: List['QAPISchemaObjectTypeMember'], + variants: Optional['QAPISchemaVariants'], + ) -> None: pass - 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: pass - 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: pass - 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: pass class QAPISchemaModule(Visitable): - def __init__(self, name): + def __init__(self, name: Optional[str]): self.name = name - self._entity_list = [] + self._entity_list: List[QAPISchemaEntity] = [] - def add_entity(self, ent): + def add_entity(self, ent: QAPISchemaEntity) -> None: self._entity_list.append(ent) - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: visitor.visit_module(self.name) for entity in self._entity_list: if visitor.visit_needed(entity): @@ -165,11 +228,11 @@ def visit(self, visitor): class QAPISchemaInclude(QAPISchemaEntity): - def __init__(self, sub_module, info): + def __init__(self, sub_module: QAPISchemaModule, info: QAPISourceInfo): super().__init__(None, info, None) self._sub_module = sub_module - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_include(self._sub_module.name, self.info) @@ -181,17 +244,17 @@ def c_type(self) -> str: raise NotImplementedError() # Return the C type to be used in a parameter list. - def c_param_type(self): + def c_param_type(self) -> str: return self.c_type() # Return the C type to be used where we suppress boxing. - def c_unboxed_type(self): + def c_unboxed_type(self) -> str: return self.c_type() def json_type(self) -> str: raise NotImplementedError() - def alternate_qtype(self): + def alternate_qtype(self) -> str: json2qtype = { 'null': 'QTYPE_QNULL', 'string': 'QTYPE_QSTRING', @@ -202,24 +265,24 @@ def alternate_qtype(self): } return json2qtype[self.json_type()] - def doc_type(self): + def doc_type(self) -> Optional[str]: if self.is_implicit(): return None return self.name - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: QAPISchemaEntity.check(self, schema) if 'deprecated' in [f.name for f in self.features]: raise QAPISemError( self.info, "feature 'deprecated' is not supported for types") - def describe(self): + def describe(self) -> str: assert self.meta return "%s type '%s'" % (self.meta, self.name) class QAPISchemaBuiltinType(QAPISchemaType): - def __init__(self, name, json_type, c_type): + def __init__(self, name: str, json_type: str, c_type: str): super().__init__(name, None, None) assert not c_type or isinstance(c_type, str) assert json_type in ('string', 'number', 'int', 'boolean', 'null', @@ -228,30 +291,37 @@ def __init__(self, name, json_type, c_type): self._c_type_name = c_type self._meta = 'built-in' - def c_name(self): + def c_name(self) -> str: return self.name - def c_type(self): + def c_type(self) -> str: return self._c_type_name - def c_param_type(self): + def c_param_type(self) -> str: if self.name == 'str': return 'const ' + self._c_type_name return self._c_type_name - def json_type(self): + def json_type(self) -> str: return self._json_type_name - def doc_type(self): + def doc_type(self) -> str: return self.json_type() - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_builtin_type(self.name, self.info, self.json_type()) class QAPISchemaEnumType(QAPISchemaType): - def __init__(self, name, info, doc, ifcond, features, members, prefix): + def __init__(self, + name: str, + info: Optional[QAPISourceInfo], + doc: Optional[QAPIDoc], + ifcond: Optional[List[str]], + features: Optional[List['QAPISchemaFeature']], + members: List['QAPISchemaEnumMember'], + prefix: Optional[str]): super().__init__(name, info, doc, ifcond, features) for m in members: assert isinstance(m, QAPISchemaEnumMember) @@ -261,32 +331,32 @@ def __init__(self, name, info, doc, ifcond, features, members, prefix): self.prefix = prefix self._meta = 'enum' - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: super().check(schema) - seen = {} + seen: Dict[str, 'QAPISchemaMember'] = {} for m in self.members: m.check_clash(self.info, seen) - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc for m in self.members: m.connect_doc(doc) - def is_implicit(self): + def is_implicit(self) -> bool: # See QAPISchema._make_implicit_enum_type() and ._def_predefineds() return self.name.endswith('Kind') or self.name == 'QType' - def c_type(self): + def c_type(self) -> str: return c_name(self.name) - def member_names(self): + def member_names(self) -> List[str]: return [m.name for m in self.members] - def json_type(self): + def json_type(self) -> str: return 'string' - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_enum_type( self.name, self.info, self.ifcond, self.features, @@ -294,56 +364,65 @@ def visit(self, visitor): class QAPISchemaArrayType(QAPISchemaType): - def __init__(self, name, info, element_type): + def __init__(self, name: str, + info: Optional[QAPISourceInfo], + element_type: str): super().__init__(name, info, None) assert isinstance(element_type, str) self._element_type_name = element_type - self.element_type = None + self.element_type: Optional[QAPISchemaType] = None self._meta = 'array' - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: super().check(schema) self.element_type = schema.resolve_type( self._element_type_name, self.info, self.info.defn_meta if self.info else None) assert not isinstance(self.element_type, QAPISchemaArrayType) - def set_module(self, schema): + def set_module(self, schema: 'QAPISchema') -> None: self._set_module(schema, self.element_type.info) @property - def ifcond(self): + def ifcond(self) -> List[str]: assert self._checked return self.element_type.ifcond - def is_implicit(self): + def is_implicit(self) -> bool: return True - def c_type(self): + def c_type(self) -> str: return c_name(self.name) + POINTER_SUFFIX - def json_type(self): + def json_type(self) -> str: return 'array' - def doc_type(self): + def doc_type(self) -> Optional[str]: elt_doc_type = self.element_type.doc_type() if not elt_doc_type: return None return 'array of ' + elt_doc_type - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_array_type(self.name, self.info, self.ifcond, self.element_type) - def describe(self): + def describe(self) -> str: assert self.meta return "%s type ['%s']" % (self.meta, self._element_type_name) class QAPISchemaObjectType(QAPISchemaType): - def __init__(self, name, info, doc, ifcond, features, - base, local_members, variants): + def __init__(self, + name: str, + info: Optional[QAPISourceInfo], + doc: Optional[QAPIDoc], + ifcond: Optional['QAPISchemaType'], + features: Optional[List['QAPISchemaFeature']], + base: Optional[str], + local_members: List['QAPISchemaObjectTypeMember'], + variants: Optional['QAPISchemaVariants']): # struct has local_members, optional base, and no variants # flat union has base, variants, and no local_members # simple union has local_members, variants, and no base @@ -357,12 +436,12 @@ def __init__(self, name, info, doc, ifcond, features, assert isinstance(variants, QAPISchemaVariants) variants.set_defined_in(name) self._base_name = base - self.base = None + self.base: Optional[QAPISchemaObjectType] = None self.local_members = local_members self.variants = variants - self.members = None + self.members: Optional[List[QAPISchemaObjectTypeMember]] = None - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: # This calls another type T's .check() exactly when the C # struct emitted by gen_object() contains that T's C struct # (pointers don't count). @@ -377,7 +456,7 @@ def check(self, schema): super().check(schema) assert self._checked and self.members is None - seen = OrderedDict() + seen: Dict[str, 'QAPISchemaMember'] = OrderedDict() if self._base_name: base = schema.resolve_type(self._base_name, self.info, "'base'") if (not isinstance(base, QAPISchemaObjectType) @@ -406,13 +485,15 @@ def check(self, schema): # Check that the members of this type do not cause duplicate JSON members, # and update seen to track the members seen so far. Report any errors # on behalf of info, which is not necessarily self.info - def check_clash(self, info, seen): + def check_clash(self, + info: QAPISourceInfo, + seen: Dict[str, 'QAPISchemaMember']) -> None: assert self._checked assert not self.variants # not implemented for m in self.members: m.check_clash(info, seen) - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc if self.base and self.base.is_implicit(): @@ -421,7 +502,7 @@ def connect_doc(self, doc=None): m.connect_doc(doc) @property - def ifcond(self): + def ifcond(self) -> List[str]: assert self._checked if isinstance(self._ifcond, QAPISchemaType): # Simple union wrapper type inherits from wrapped type; @@ -429,30 +510,30 @@ def ifcond(self): return self._ifcond.ifcond return self._ifcond - def is_implicit(self): + def is_implicit(self) -> bool: # See QAPISchema._make_implicit_object_type(), as well as # _def_predefineds() return self.name.startswith('q_') - def is_empty(self): + def is_empty(self) -> bool: assert self.members is not None return not self.members and not self.variants - def c_name(self): + def c_name(self) -> str: assert self.name != 'q_empty' return super().c_name() - def c_type(self): + def c_type(self) -> str: assert not self.is_implicit() return c_name(self.name) + POINTER_SUFFIX - def c_unboxed_type(self): + def c_unboxed_type(self) -> str: return c_name(self.name) - def json_type(self): + def json_type(self) -> str: return 'object' - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_object_type( self.name, self.info, self.ifcond, self.features, @@ -463,7 +544,13 @@ def visit(self, visitor): class QAPISchemaAlternateType(QAPISchemaType): - def __init__(self, name, info, doc, ifcond, features, variants): + def __init__(self, + name: str, + info: QAPISourceInfo, + doc: QAPIDoc, + ifcond: Optional[List[str]], + features: List['QAPISchemaFeature'], + variants: 'QAPISchemaVariants'): super().__init__(name, info, doc, ifcond, features) assert isinstance(variants, QAPISchemaVariants) assert variants.tag_member @@ -472,7 +559,7 @@ def __init__(self, name, info, doc, ifcond, features, variants): self.variants = variants self._meta = 'alternate' - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: super().check(schema) self.variants.tag_member.check(schema) # Not calling self.variants.check_clash(), because there's nothing @@ -480,8 +567,8 @@ def check(self, schema): self.variants.check(schema, {}) # Alternate branch names have no relation to the tag enum values; # so we have to check for potential name collisions ourselves. - seen = {} - types_seen = {} + seen: Dict[str, QAPISchemaMember] = {} + types_seen: Dict[str, str] = {} for v in self.variants.variants: v.check_clash(self.info, seen) try: @@ -511,26 +598,30 @@ def check(self, schema): % (v.describe(self.info), types_seen[qt])) types_seen[qt] = v.name - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc for v in self.variants.variants: v.connect_doc(doc) - def c_type(self): + def c_type(self) -> str: return c_name(self.name) + POINTER_SUFFIX - def json_type(self): + def json_type(self) -> str: return 'value' - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_alternate_type( self.name, self.info, self.ifcond, self.features, self.variants) class QAPISchemaVariants: - def __init__(self, tag_name, info, tag_member, variants): + def __init__(self, + tag_name: Optional[str], + info: QAPISourceInfo, + tag_member: Optional['QAPISchemaObjectTypeMember'], + variants: List['QAPISchemaVariant']): # Flat unions pass tag_name but not tag_member. # Simple unions and alternates pass tag_member but not tag_name. # After check(), tag_member is always set, and tag_name remains @@ -545,11 +636,13 @@ def __init__(self, tag_name, info, tag_member, variants): self.tag_member = tag_member self.variants = variants - def set_defined_in(self, name): + def set_defined_in(self, name: str) -> None: for v in self.variants: v.set_defined_in(name) - def check(self, schema, seen): + def check(self, + schema: 'QAPISchema', + seen: Dict[str, 'QAPISchemaMember']) -> None: if not self.tag_member: # flat union tag_member = seen.get(c_name(self._tag_name)) base = "'base'" @@ -616,7 +709,9 @@ def check(self, schema, seen): % (v.describe(self.info), v.type.describe())) v.type.check(schema) - def check_clash(self, info, seen): + def check_clash(self, + info: QAPISourceInfo, + seen: Dict[str, 'QAPISchemaMember']) -> None: for v in self.variants: # Reset seen map for each variant, since qapi names from one # branch do not affect another branch @@ -628,18 +723,22 @@ class QAPISchemaMember: """ Represents object members, enum members and features """ role = 'member' - def __init__(self, name, info, ifcond=None): + def __init__(self, name: str, + info: Optional[QAPISourceInfo], + ifcond: Optional[List[str]] = None): assert isinstance(name, str) self.name = name self.info = info self.ifcond = ifcond or [] - self.defined_in = None + self.defined_in: Optional[str] = None - def set_defined_in(self, name): + def set_defined_in(self, name: str) -> None: assert not self.defined_in self.defined_in = name - def check_clash(self, info, seen): + def check_clash(self, + info: Optional[QAPISourceInfo], + seen: Dict[str, 'QAPISchemaMember']) -> None: cname = c_name(self.name) if cname in seen: raise QAPISemError( @@ -648,11 +747,11 @@ def check_clash(self, info, seen): % (self.describe(info), seen[cname].describe(info))) seen[cname] = self - def connect_doc(self, doc): + def connect_doc(self, doc: Optional[QAPIDoc]) -> None: if doc: doc.connect_member(self) - def describe(self, info): + def describe(self, info: QAPISourceInfo) -> str: role = self.role defined_in = self.defined_in assert defined_in @@ -692,7 +791,13 @@ class QAPISchemaFeature(QAPISchemaMember): class QAPISchemaObjectTypeMember(QAPISchemaMember): - def __init__(self, name, info, typ, optional, ifcond=None, features=None): + def __init__(self, + name: str, + info: QAPISourceInfo, + typ: str, + optional: bool, + ifcond: Optional[List[str]] = None, + features: Optional[List[QAPISchemaFeature]] = None): super().__init__(name, info, ifcond) assert isinstance(typ, str) assert isinstance(optional, bool) @@ -700,19 +805,19 @@ def __init__(self, name, info, typ, optional, ifcond=None, features=None): assert isinstance(f, QAPISchemaFeature) f.set_defined_in(name) self._type_name = typ - self.type = None + self.type: Optional[QAPISchemaType] = None self.optional = optional self.features = features or [] - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: assert self.defined_in self.type = schema.resolve_type(self._type_name, self.info, self.describe) - seen = {} + seen: Dict[str, QAPISchemaMember] = {} for f in self.features: f.check_clash(self.info, seen) - def connect_doc(self, doc): + def connect_doc(self, doc: Optional[QAPIDoc]) -> None: super().connect_doc(doc) if doc: for f in self.features: @@ -722,21 +827,35 @@ def connect_doc(self, doc): class QAPISchemaVariant(QAPISchemaObjectTypeMember): role = 'branch' - def __init__(self, name, info, typ, ifcond=None): + def __init__(self, + name: str, + info: QAPISourceInfo, + typ: str, + ifcond: Optional[List[str]] = None): super().__init__(name, info, typ, False, ifcond) class QAPISchemaCommand(QAPISchemaEntity): - def __init__(self, name, info, doc, ifcond, features, - arg_type, ret_type, - gen, success_response, boxed, allow_oob, allow_preconfig): + def __init__(self, + name: str, + info: QAPISourceInfo, + doc: QAPIDoc, + ifcond: Optional[List[str]], + features: List[QAPISchemaFeature], + arg_type: str, + ret_type: Optional[str], + gen: bool, + success_response: bool, + boxed: bool, + allow_oob: bool, + allow_preconfig: bool): super().__init__(name, info, doc, ifcond, features) assert not arg_type or isinstance(arg_type, str) assert not ret_type or isinstance(ret_type, str) self._arg_type_name = arg_type - self.arg_type = None + self.arg_type: Optional[QAPISchemaObjectType] = None self._ret_type_name = ret_type - self.ret_type = None + self.ret_type: Optional[QAPISchemaType] = None self.gen = gen self.success_response = success_response self.boxed = boxed @@ -744,7 +863,7 @@ def __init__(self, name, info, doc, ifcond, features, self.allow_preconfig = allow_preconfig self._meta = 'command' - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: super().check(schema) if self._arg_type_name: arg_type = schema.resolve_type( @@ -774,14 +893,14 @@ def check(self, schema): "command's 'returns' cannot take %s" % self.ret_type.describe()) - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc if doc: if self.arg_type and self.arg_type.is_implicit(): self.arg_type.connect_doc(doc) - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_command( self.name, self.info, self.ifcond, self.features, @@ -790,15 +909,22 @@ def visit(self, visitor): class QAPISchemaEvent(QAPISchemaEntity): - def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): + def __init__(self, + name: str, + info: QAPISourceInfo, + doc: QAPIDoc, + ifcond: Optional[List[str]], + features: List[QAPISchemaFeature], + arg_type: str, + boxed: bool): super().__init__(name, info, doc, ifcond, features) assert not arg_type or isinstance(arg_type, str) self._arg_type_name = arg_type - self.arg_type = None + self.arg_type: Optional[QAPISchemaObjectType] = None self.boxed = boxed self._meta = 'event' - def check(self, schema): + def check(self, schema: 'QAPISchema') -> None: super().check(schema) if self._arg_type_name: arg_type = schema.resolve_type( @@ -815,29 +941,32 @@ def check(self, schema): "event's 'data' can take %s only with 'boxed': true" % self.arg_type.describe()) - def connect_doc(self, doc=None): + def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc if doc: if self.arg_type and self.arg_type.is_implicit(): self.arg_type.connect_doc(doc) - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_event( self.name, self.info, self.ifcond, self.features, self.arg_type, self.boxed) +_EntityType = TypeVar('_EntityType', bound=QAPISchemaEntity) + + class QAPISchema(Visitable): - def __init__(self, fname): + def __init__(self, fname: str): self.fname = fname parser = QAPISchemaParser(fname) exprs = check_exprs(parser.exprs) self.docs = parser.docs - self._entity_list = [] - self._entity_dict = {} - self._module_dict = OrderedDict() + self._entity_list: List[QAPISchemaEntity] = [] + self._entity_dict: Dict[str, QAPISchemaEntity] = {} + self._module_dict: Dict[str, QAPISchemaModule] = OrderedDict() self._schema_dir = os.path.dirname(fname) self._make_module(None) # built-ins self._make_module(fname) @@ -847,7 +976,7 @@ def __init__(self, fname): self._def_exprs(exprs) self.check() - def _def_entity(self, ent): + def _def_entity(self, ent: QAPISchemaEntity) -> None: # Only the predefined types are allowed to not have info assert ent.info or self._predefining self._entity_list.append(ent) @@ -866,16 +995,33 @@ def _def_entity(self, ent): ent.info, "%s is already defined" % other_ent.describe()) self._entity_dict[ent.name] = ent - def lookup_entity(self, name, typ=None): + @overload + def lookup_entity(self, name: str, + typ: None = None) -> Optional[QAPISchemaEntity]: ... + + @overload + def lookup_entity(self, name: str, + typ: Type[_EntityType]) -> Optional[_EntityType]: ... + + def lookup_entity(self, + name: str, + typ: Optional[Type[QAPISchemaEntity]] = None, + ) -> Optional[QAPISchemaEntity]: ent = self._entity_dict.get(name) if typ and not isinstance(ent, typ): return None return ent - def lookup_type(self, name): + def lookup_type(self, name: str) -> QAPISchemaType: return self.lookup_entity(name, QAPISchemaType) - def resolve_type(self, name, info, what): + def resolve_type(self, + name: str, + info: Optional[QAPISourceInfo], + what: Optional[ + Union[str, Callable[[Optional[QAPISourceInfo]], str]] + ], + ) -> QAPISchemaType: typ = self.lookup_type(name) if not typ: if callable(what): @@ -884,27 +1030,33 @@ def resolve_type(self, name, info, what): info, "%s uses unknown type '%s'" % (what, name)) return typ - def _module_name(self, fname): + def _module_name(self, fname: Optional[str]) -> Optional[str]: if fname is None: return None return os.path.relpath(fname, self._schema_dir) - def _make_module(self, fname): + def _make_module(self, fname: Optional[str]) -> QAPISchemaModule: name = self._module_name(fname) if name not in self._module_dict: self._module_dict[name] = QAPISchemaModule(name) return self._module_dict[name] - def module_by_fname(self, fname): + def module_by_fname(self, fname: Optional[str]) -> QAPISchemaModule: name = self._module_name(fname) return self._module_dict[name] - def _def_include(self, expr, info, doc): + def _def_include(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: Optional[QAPIDoc]) -> None: include = expr['include'] assert doc is None self._def_entity(QAPISchemaInclude(self._make_module(include), info)) - def _def_builtin_type(self, name, json_type, c_type): + def _def_builtin_type(self, + name: str, + json_type: str, + c_type: str) -> None: self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type)) # Instantiating only the arrays that are actually used would # be nice, but we can't as long as their generated code @@ -912,7 +1064,7 @@ def _def_builtin_type(self, name, json_type, c_type): # schema. self._make_array_type(name, None) - def _def_predefineds(self): + def _def_predefineds(self) -> None: for t in [('str', 'string', 'char' + POINTER_SUFFIX), ('number', 'number', 'double'), ('int', 'int', 'int64_t'), @@ -941,17 +1093,26 @@ def _def_predefineds(self): self._def_entity(QAPISchemaEnumType('QType', None, None, None, None, qtype_values, 'QTYPE')) - def _make_features(self, features, info): + def _make_features(self, + features: Optional[List[Dict[str, Any]]], + info: QAPISourceInfo) -> List[QAPISchemaFeature]: if features is None: return [] return [QAPISchemaFeature(f['name'], info, f.get('if')) for f in features] - def _make_enum_members(self, values, info): + def _make_enum_members(self, + values: List[Dict[str, Any]], + info: Optional[QAPISourceInfo], + ) -> List[QAPISchemaEnumMember]: return [QAPISchemaEnumMember(v['name'], info, v.get('if')) for v in values] - def _make_implicit_enum_type(self, name, info, ifcond, values): + def _make_implicit_enum_type(self, + name: str, + info: QAPISourceInfo, + ifcond: Optional[List[str]], + values: List[Dict[str, Any]]) -> str: # See also QAPISchemaObjectTypeMember.describe() name = name + 'Kind' # reserved by check_defn_name_str() self._def_entity(QAPISchemaEnumType( @@ -960,13 +1121,21 @@ def _make_implicit_enum_type(self, name, info, ifcond, values): None)) return name - def _make_array_type(self, element_type, info): + def _make_array_type(self, + element_type: str, + info: QAPISourceInfo) -> str: name = element_type + 'List' # reserved by check_defn_name_str() if not self.lookup_type(name): self._def_entity(QAPISchemaArrayType(name, info, element_type)) return name - def _make_implicit_object_type(self, name, info, ifcond, role, members): + def _make_implicit_object_type(self, + name: str, + info: QAPISourceInfo, + ifcond: Optional[QAPISchemaType], + role: str, + members: List[QAPISchemaObjectTypeMember], + ) -> Optional[str]: if not members: return None # See also QAPISchemaObjectTypeMember.describe() @@ -990,7 +1159,10 @@ 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: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['enum'] data = expr['data'] prefix = expr.get('prefix') @@ -1000,7 +1172,12 @@ def _def_enum_type(self, expr, info, doc): name, info, doc, ifcond, features, self._make_enum_members(data, info), prefix)) - def _make_member(self, name, typ, ifcond, features, info): + def _make_member(self, + name: str, + typ: str, + ifcond: Optional[List[str]], + features: Optional[List[Dict[str, Any]]], + info: QAPISourceInfo) -> QAPISchemaObjectTypeMember: optional = False if name.startswith('*'): name = name[1:] @@ -1011,12 +1188,18 @@ def _make_member(self, name, typ, ifcond, features, info): return QAPISchemaObjectTypeMember(name, info, typ, optional, ifcond, self._make_features(features, info)) - def _make_members(self, data, info): + def _make_members(self, + data: Dict[str, Dict[str, Any]], + info: QAPISourceInfo, + ) -> List[QAPISchemaObjectTypeMember]: return [self._make_member(key, value['type'], value.get('if'), value.get('features'), info) for (key, value) in data.items()] - def _def_struct_type(self, expr, info, doc): + def _def_struct_type(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['struct'] base = expr.get('base') data = expr['data'] @@ -1027,10 +1210,18 @@ def _def_struct_type(self, expr, info, doc): self._make_members(data, info), None)) - def _make_variant(self, case, typ, ifcond, info): + def _make_variant(self, + case: str, + typ: str, + ifcond: Optional[List[str]], + info: QAPISourceInfo) -> QAPISchemaVariant: return QAPISchemaVariant(case, info, typ, ifcond) - def _make_simple_variant(self, case, typ, ifcond, info): + def _make_simple_variant(self, + case: str, + typ: str, + ifcond: Optional[List[str]], + info: QAPISourceInfo) -> QAPISchemaVariant: if isinstance(typ, list): assert len(typ) == 1 typ = self._make_array_type(typ[0], info) @@ -1039,7 +1230,10 @@ def _make_simple_variant(self, case, typ, ifcond, info): 'wrapper', [self._make_member('data', typ, None, None, info)]) return QAPISchemaVariant(case, info, typ, ifcond) - def _def_union_type(self, expr, info, doc): + def _def_union_type(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['union'] data = expr['data'] base = expr.get('base') @@ -1070,7 +1264,10 @@ def _def_union_type(self, expr, info, doc): QAPISchemaVariants( tag_name, info, tag_member, variants))) - def _def_alternate_type(self, expr, info, doc): + def _def_alternate_type(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['alternate'] data = expr['data'] ifcond = expr.get('if') @@ -1084,7 +1281,10 @@ def _def_alternate_type(self, expr, info, doc): QAPISchemaVariants( None, info, tag_member, variants))) - def _def_command(self, expr, info, doc): + def _def_command(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['command'] data = expr.get('data') rets = expr.get('returns') @@ -1107,7 +1307,10 @@ def _def_command(self, expr, info, doc): gen, success_response, boxed, allow_oob, allow_preconfig)) - def _def_event(self, expr, info, doc): + def _def_event(self, + expr: Dict[str, Any], + info: QAPISourceInfo, + doc: QAPIDoc) -> None: name = expr['event'] data = expr.get('data') boxed = expr.get('boxed', False) @@ -1120,7 +1323,7 @@ def _def_event(self, expr, info, doc): self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, features, data, boxed)) - def _def_exprs(self, exprs): + def _def_exprs(self, exprs: List[ParsedExpression]) -> None: for expr_elem in exprs: expr = expr_elem.expr info = expr_elem.info @@ -1142,7 +1345,7 @@ def _def_exprs(self, exprs): else: assert False - def check(self): + def check(self) -> None: for ent in self._entity_list: ent.check(self) ent.connect_doc() @@ -1150,7 +1353,7 @@ def check(self): for ent in self._entity_list: ent.set_module(self) - def visit(self, visitor): + def visit(self, visitor: QAPISchemaVisitor) -> None: visitor.visit_begin(self) for mod in self._module_dict.values(): mod.visit(visitor) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 16/25] qapi/schema.py: enable checking 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (14 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 15/25] qapi/schema.py: add type hint annotations John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 17/25] qapi: Disable similarity checks in pylint entirely John Snow ` (9 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/mypy.ini | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini index 20ab509946..df60c18de1 100644 --- a/scripts/qapi/mypy.ini +++ b/scripts/qapi/mypy.ini @@ -1,10 +1,4 @@ [mypy] strict = True strict_optional = False -disallow_untyped_calls = False python_version = 3.6 - -[mypy-qapi.schema] -disallow_untyped_defs = False -disallow_incomplete_defs = False -check_untyped_defs = False -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 17/25] qapi: Disable similarity checks in pylint entirely 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (15 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 16/25] qapi/schema.py: enable checking John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 18/25] qapi/schema.py: Add pylint warning suppressions John Snow ` (8 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa The pylint similarity checks cannot distinguish parameter lists from other code; with the QAPISchemaVisitor interface having long lists of parameters, these similarity checks fire off in a way that's difficult to disable in a targeted way without littering the code with pylint pragmas. There is a change request filed to be able to ignore parameter lists, see: https://github.com/PyCQA/pylint/issues/3619 Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/pylintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc index 5091a08f12..fb444e93bb 100644 --- a/scripts/qapi/pylintrc +++ b/scripts/qapi/pylintrc @@ -18,6 +18,7 @@ ignore-patterns=schema.py, # --disable=W". disable=fixme, missing-docstring, + similarities, # See https://github.com/PyCQA/pylint/issues/3619 too-many-arguments, too-many-branches, too-many-statements, -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 18/25] qapi/schema.py: Add pylint warning suppressions 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (16 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 17/25] qapi: Disable similarity checks in pylint entirely John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 19/25] qapi/schema.py: Convert several methods to classmethods John Snow ` (7 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 8907bec0b5..61238c0686 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -39,6 +39,8 @@ class Visitable: """Abstract duck that suggests a class is visitable.""" + # pylint: disable=too-few-public-methods + def visit(self, visitor: 'QAPISchemaVisitor') -> None: raise NotImplementedError @@ -133,6 +135,7 @@ def visit_module(self, name: Optional[str]) -> None: pass def visit_needed(self, entity: QAPISchemaEntity) -> bool: + # pylint: disable=unused-argument, no-self-use # Default to visiting everything return True -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 19/25] qapi/schema.py: Convert several methods to classmethods 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (17 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 18/25] qapi/schema.py: Add pylint warning suppressions John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 20/25] qapi/schema.py: Replace one-letter variable names John Snow ` (6 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa If they don't use self and nothing that extends them needs self either, they can be classmethods. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 61238c0686..2d23ce04eb 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -1096,7 +1096,8 @@ def _def_predefineds(self) -> None: self._def_entity(QAPISchemaEnumType('QType', None, None, None, None, qtype_values, 'QTYPE')) - def _make_features(self, + @classmethod + def _make_features(cls, features: Optional[List[Dict[str, Any]]], info: QAPISourceInfo) -> List[QAPISchemaFeature]: if features is None: @@ -1104,7 +1105,8 @@ def _make_features(self, return [QAPISchemaFeature(f['name'], info, f.get('if')) for f in features] - def _make_enum_members(self, + @classmethod + def _make_enum_members(cls, values: List[Dict[str, Any]], info: Optional[QAPISourceInfo], ) -> List[QAPISchemaEnumMember]: @@ -1213,7 +1215,8 @@ def _def_struct_type(self, self._make_members(data, info), None)) - def _make_variant(self, + @classmethod + def _make_variant(cls, case: str, typ: str, ifcond: Optional[List[str]], -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 20/25] qapi/schema.py: Replace one-letter variable names 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (18 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 19/25] qapi/schema.py: Convert several methods to classmethods John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 21/25] qapi/schema.py: disable pylint line limit John Snow ` (5 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa I hope you like butter, because here comes the churn! Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 184 +++++++++++++++++++++-------------------- 1 file changed, 95 insertions(+), 89 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 2d23ce04eb..a0e047c735 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -53,9 +53,11 @@ def __init__(self, ifcond: Optional[Union[List[str], 'QAPISchemaType']] = None, features: Optional[List['QAPISchemaFeature']] = None): assert name is None or isinstance(name, str) - for f in features or []: - assert isinstance(f, QAPISchemaFeature) - f.set_defined_in(name) + + for feature in features or []: + assert isinstance(feature, QAPISchemaFeature) + feature.set_defined_in(name) + self.name = name self._module: Optional[QAPISchemaModule] = None # For explicitly defined entities, info points to the (explicit) @@ -84,15 +86,15 @@ def c_name(self) -> str: def check(self, schema: 'QAPISchema') -> None: assert not self._checked seen: Dict[str, 'QAPISchemaMember'] = {} - for f in self.features: - f.check_clash(self.info, seen) + for feature in self.features: + feature.check_clash(self.info, seen) self._checked = True def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: doc = doc or self.doc if doc: - for f in self.features: - doc.connect_feature(f) + for feature in self.features: + doc.connect_feature(feature) def check_doc(self) -> None: if self.doc: @@ -326,9 +328,9 @@ def __init__(self, members: List['QAPISchemaEnumMember'], prefix: Optional[str]): super().__init__(name, info, doc, ifcond, features) - for m in members: - assert isinstance(m, QAPISchemaEnumMember) - m.set_defined_in(name) + for member in members: + assert isinstance(member, QAPISchemaEnumMember) + member.set_defined_in(name) assert prefix is None or isinstance(prefix, str) self.members = members self.prefix = prefix @@ -337,14 +339,14 @@ def __init__(self, def check(self, schema: 'QAPISchema') -> None: super().check(schema) seen: Dict[str, 'QAPISchemaMember'] = {} - for m in self.members: - m.check_clash(self.info, seen) + for member in self.members: + member.check_clash(self.info, seen) def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc - for m in self.members: - m.connect_doc(doc) + for member in self.members: + member.connect_doc(doc) def is_implicit(self) -> bool: # See QAPISchema._make_implicit_enum_type() and ._def_predefineds() @@ -432,9 +434,9 @@ def __init__(self, super().__init__(name, info, doc, ifcond, features) self._meta = 'union' if variants else 'struct' assert base is None or isinstance(base, str) - for m in local_members: - assert isinstance(m, QAPISchemaObjectTypeMember) - m.set_defined_in(name) + for member in local_members: + assert isinstance(member, QAPISchemaObjectTypeMember) + member.set_defined_in(name) if variants is not None: assert isinstance(variants, QAPISchemaVariants) variants.set_defined_in(name) @@ -471,9 +473,9 @@ def check(self, schema: 'QAPISchema') -> None: self.base = base self.base.check(schema) self.base.check_clash(self.info, seen) - for m in self.local_members: - m.check(schema) - m.check_clash(self.info, seen) + for member in self.local_members: + member.check(schema) + member.check_clash(self.info, seen) # check_clash is abstract, but local_members is asserted to be # Sequence[QAPISchemaObjectTypeMember]. Cast to the narrower type. @@ -493,16 +495,16 @@ def check_clash(self, seen: Dict[str, 'QAPISchemaMember']) -> None: assert self._checked assert not self.variants # not implemented - for m in self.members: - m.check_clash(info, seen) + for member in self.members: + member.check_clash(info, seen) def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc if self.base and self.base.is_implicit(): self.base.connect_doc(doc) - for m in self.local_members: - m.connect_doc(doc) + for member in self.local_members: + member.connect_doc(doc) @property def ifcond(self) -> List[str]: @@ -572,40 +574,43 @@ def check(self, schema: 'QAPISchema') -> None: # so we have to check for potential name collisions ourselves. seen: Dict[str, QAPISchemaMember] = {} types_seen: Dict[str, str] = {} - for v in self.variants.variants: - v.check_clash(self.info, seen) + + for variant in self.variants.variants: + variant.check_clash(self.info, seen) + try: - qtype = v.type.alternate_qtype() + qtype = variant.type.alternate_qtype() except KeyError: - raise QAPISemError( - self.info, - "%s cannot use %s" - % (v.describe(self.info), v.type.describe())) + msg = "{} cannot use {}".format( + variant.describe(self.info), variant.type.describe()) + raise QAPISemError(self.info, msg) from None + conflicting = set([qtype]) if qtype == 'QTYPE_QSTRING': - if isinstance(v.type, QAPISchemaEnumType): - for m in v.type.members: - if m.name in ['on', 'off']: + if isinstance(variant.type, QAPISchemaEnumType): + for member in variant.type.members: + if member.name in ['on', 'off']: conflicting.add('QTYPE_QBOOL') - if re.match(r'[-+0-9.]', m.name): + if re.match(r'[-+0-9.]', member.name): # lazy, could be tightened conflicting.add('QTYPE_QNUM') else: conflicting.add('QTYPE_QNUM') conflicting.add('QTYPE_QBOOL') - for qt in conflicting: - if qt in types_seen: - raise QAPISemError( - self.info, - "%s can't be distinguished from '%s'" - % (v.describe(self.info), types_seen[qt])) - types_seen[qt] = v.name + + for qtype in conflicting: + if qtype in types_seen: + msg = "{} can't be distinguished from '{}'".format( + variant.describe(self.info), types_seen[qtype]) + raise QAPISemError(self.info, msg) + + types_seen[qtype] = variant.name def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc - for v in self.variants.variants: - v.connect_doc(doc) + for variant in self.variants.variants: + variant.connect_doc(doc) def c_type(self) -> str: return c_name(self.name) + POINTER_SUFFIX @@ -632,16 +637,16 @@ def __init__(self, assert bool(tag_member) != bool(tag_name) assert (isinstance(tag_name, str) or isinstance(tag_member, QAPISchemaObjectTypeMember)) - for v in variants: - assert isinstance(v, QAPISchemaVariant) + for variant in variants: + assert isinstance(variant, QAPISchemaVariant) self._tag_name = tag_name self.info = info self.tag_member = tag_member self.variants = variants def set_defined_in(self, name: str) -> None: - for v in self.variants: - v.set_defined_in(name) + for variant in self.variants: + variant.set_defined_in(name) def check(self, schema: 'QAPISchema', @@ -686,40 +691,41 @@ def check(self, if self._tag_name: # flat union # branches that are not explicitly covered get an empty type cases = {v.name for v in self.variants} - for m in self.tag_member.type.members: - if m.name not in cases: - v = QAPISchemaVariant(m.name, self.info, - 'q_empty', m.ifcond) - v.set_defined_in(self.tag_member.defined_in) - self.variants.append(v) + for member in self.tag_member.type.members: + if member.name not in cases: + variant = QAPISchemaVariant(member.name, self.info, + 'q_empty', member.ifcond) + variant.set_defined_in(self.tag_member.defined_in) + self.variants.append(variant) if not self.variants: raise QAPISemError(self.info, "union has no branches") - for v in self.variants: - v.check(schema) + for variant in self.variants: + variant.check(schema) # Union names must match enum values; alternate names are # checked separately. Use 'seen' to tell the two apart. if seen: - if v.name not in self.tag_member.type.member_names(): + if variant.name not in self.tag_member.type.member_names(): raise QAPISemError( self.info, "branch '%s' is not a value of %s" - % (v.name, self.tag_member.type.describe())) - if (not isinstance(v.type, QAPISchemaObjectType) - or v.type.variants): + % (variant.name, self.tag_member.type.describe())) + if (not isinstance(variant.type, QAPISchemaObjectType) + or variant.type.variants): raise QAPISemError( self.info, - "%s cannot use %s" - % (v.describe(self.info), v.type.describe())) - v.type.check(schema) + "%s cannot use %s" % ( + variant.describe(self.info), + variant.type.describe())) + variant.type.check(schema) def check_clash(self, info: QAPISourceInfo, seen: Dict[str, 'QAPISchemaMember']) -> None: - for v in self.variants: + for variant in self.variants: # Reset seen map for each variant, since qapi names from one # branch do not affect another branch - assert isinstance(v.type, QAPISchemaObjectType) - v.type.check_clash(info, dict(seen)) + assert isinstance(variant.type, QAPISchemaObjectType) + variant.type.check_clash(info, dict(seen)) class QAPISchemaMember: @@ -804,9 +810,9 @@ def __init__(self, super().__init__(name, info, ifcond) assert isinstance(typ, str) assert isinstance(optional, bool) - for f in features or []: - assert isinstance(f, QAPISchemaFeature) - f.set_defined_in(name) + for feature in features or []: + assert isinstance(feature, QAPISchemaFeature) + feature.set_defined_in(name) self._type_name = typ self.type: Optional[QAPISchemaType] = None self.optional = optional @@ -817,14 +823,14 @@ def check(self, schema: 'QAPISchema') -> None: self.type = schema.resolve_type(self._type_name, self.info, self.describe) seen: Dict[str, QAPISchemaMember] = {} - for f in self.features: - f.check_clash(self.info, seen) + for feature in self.features: + feature.check_clash(self.info, seen) def connect_doc(self, doc: Optional[QAPIDoc]) -> None: super().connect_doc(doc) if doc: - for f in self.features: - doc.connect_feature(f) + for feature in self.features: + doc.connect_feature(feature) class QAPISchemaVariant(QAPISchemaObjectTypeMember): @@ -1068,22 +1074,22 @@ def _def_builtin_type(self, self._make_array_type(name, None) def _def_predefineds(self) -> None: - for t in [('str', 'string', 'char' + POINTER_SUFFIX), - ('number', 'number', 'double'), - ('int', 'int', 'int64_t'), - ('int8', 'int', 'int8_t'), - ('int16', 'int', 'int16_t'), - ('int32', 'int', 'int32_t'), - ('int64', 'int', 'int64_t'), - ('uint8', 'int', 'uint8_t'), - ('uint16', 'int', 'uint16_t'), - ('uint32', 'int', 'uint32_t'), - ('uint64', 'int', 'uint64_t'), - ('size', 'int', 'uint64_t'), - ('bool', 'boolean', 'bool'), - ('any', 'value', 'QObject' + POINTER_SUFFIX), - ('null', 'null', 'QNull' + POINTER_SUFFIX)]: - self._def_builtin_type(*t) + for args in (('str', 'string', 'char' + POINTER_SUFFIX), + ('number', 'number', 'double'), + ('int', 'int', 'int64_t'), + ('int8', 'int', 'int8_t'), + ('int16', 'int', 'int16_t'), + ('int32', 'int', 'int32_t'), + ('int64', 'int', 'int64_t'), + ('uint8', 'int', 'uint8_t'), + ('uint16', 'int', 'uint16_t'), + ('uint32', 'int', 'uint32_t'), + ('uint64', 'int', 'uint64_t'), + ('size', 'int', 'uint64_t'), + ('bool', 'boolean', 'bool'), + ('any', 'value', 'QObject' + POINTER_SUFFIX), + ('null', 'null', 'QNull' + POINTER_SUFFIX)): + self._def_builtin_type(*args) self.the_empty_object_type = QAPISchemaObjectType( 'q_empty', None, None, None, None, None, [], None) self._def_entity(self.the_empty_object_type) -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 21/25] qapi/schema.py: disable pylint line limit 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (19 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 20/25] qapi/schema.py: Replace one-letter variable names John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 22/25] qapi/schema.py: Ignore unused argument for check() John Snow ` (4 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa It's a big file, but there's really nothing in here that doesn't belong in here -- and I don't think it's large enough to justify the one-module-per-class approach. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index a0e047c735..271befea1c 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -12,6 +12,8 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. +# pylint: disable=too-many-lines ¯\_(ツ)_/¯ + # TODO catching name collisions in generated code would be nice from collections import OrderedDict -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 22/25] qapi/schema.py: Ignore unused argument for check() 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (20 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 21/25] qapi/schema.py: disable pylint line limit John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:44 ` [PATCH 23/25] qapi/schema.py: enable pylint checks John Snow ` (3 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa This is an interface with a default implementation. Pylint doesn't have enough context to be aware of this. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 271befea1c..6ecbc2aa6b 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -86,6 +86,7 @@ def c_name(self) -> str: return c_name(self.name) def check(self, schema: 'QAPISchema') -> None: + # pylint: disable=unused-argument assert not self._checked seen: Dict[str, 'QAPISchemaMember'] = {} for feature in self.features: -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 23/25] qapi/schema.py: enable pylint checks 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (21 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 22/25] qapi/schema.py: Ignore unused argument for check() John Snow @ 2020-09-22 22:44 ` John Snow 2020-09-22 22:45 ` [PATCH 24/25] qapi/schema.py: Add module docstring John Snow ` (2 subsequent siblings) 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:44 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/pylintrc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc index fb444e93bb..539e5f65a0 100644 --- a/scripts/qapi/pylintrc +++ b/scripts/qapi/pylintrc @@ -1,10 +1,5 @@ [MASTER] -# Add files or directories matching the regex patterns to the ignore list. -# The regex matches against base names, not paths. -ignore-patterns=schema.py, - - [MESSAGES CONTROL] # Disable the message, report, category or checker with the given id(s). You -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 24/25] qapi/schema.py: Add module docstring 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (22 preceding siblings ...) 2020-09-22 22:44 ` [PATCH 23/25] qapi/schema.py: enable pylint checks John Snow @ 2020-09-22 22:45 ` John Snow 2020-09-22 22:45 ` [PATCH 25/25] qapi/schema.py: Use python3 style super() John Snow 2020-10-22 14:51 ` [PATCH 00/25] qapi: static typing conversion, pt6 John Snow 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:45 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Add some microdocumentation that gives a nice file-level overview of this 1300+ line file. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 6ecbc2aa6b..baafe3babf 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -1,7 +1,37 @@ # -*- coding: utf-8 -*- -# -# QAPI schema internal representation -# +""" +QAPI Schema internal representation. + +The `QAPISchema` represents the fully realized QAPI schema. It builds a +listing of `QAPISchemaEntity` objects that are each associated with a +`QAPISchemaModule`. + +`QAPISchemaMember` represents a single member of a collection of +members, see the subclasses thereof for users. `QAPISchemaVariants` is a +simple collection of `QAPISchemaVariant`. + +The `QAPISchemaVisitor` can be extended and passed to QAPISchema.visit +to iterate over a schema and perform code generation tasks. + +The Python class hierarchy at a glance: + +`QAPISchemaEntity` + `QAPISchemaInclude` + `QAPISchemaCommand` + `QAPISchemaEvent` + `QAPISchemaType` + `QAPISchemaBuiltinType` + `QAPISchemaEnumType` + `QAPISchemaArrayType` + `QAPISchemaObjectType` + `QAPISchemaAlternateType` + +`QAPISchemaMember` + `QAPISchemaEnumMember` + `QAPISchemaFeature` + `QAPISchemaObjectTypeMember` + `QAPISchemaVariant` +""" # Copyright (c) 2015-2019 Red Hat Inc. # # Authors: -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 25/25] qapi/schema.py: Use python3 style super() 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (23 preceding siblings ...) 2020-09-22 22:45 ` [PATCH 24/25] qapi/schema.py: Add module docstring John Snow @ 2020-09-22 22:45 ` John Snow 2020-10-22 14:51 ` [PATCH 00/25] qapi: static typing conversion, pt6 John Snow 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-09-22 22:45 UTC (permalink / raw) To: Markus Armbruster Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index baafe3babf..6b47ca26e0 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -309,7 +309,7 @@ def doc_type(self) -> Optional[str]: return self.name def check(self, schema: 'QAPISchema') -> None: - QAPISchemaEntity.check(self, schema) + super().check(schema) if 'deprecated' in [f.name for f in self.features]: raise QAPISemError( self.info, "feature 'deprecated' is not supported for types") -- 2.26.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 00/25] qapi: static typing conversion, pt6 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow ` (24 preceding siblings ...) 2020-09-22 22:45 ` [PATCH 25/25] qapi/schema.py: Use python3 style super() John Snow @ 2020-10-22 14:51 ` John Snow 25 siblings, 0 replies; 27+ messages in thread From: John Snow @ 2020-10-22 14:51 UTC (permalink / raw) To: Markus Armbruster; +Cc: Michael Roth, qemu-devel, Eduardo Habkost, Cleber Rosa On 9/22/20 6:44 PM, John Snow wrote: > based-on: <20200922223525.4085762-1-jsnow@redhat.com> > [PATCH 00/26] qapi: static typing conversion, pt5 > > Hi, this series adds static type hints to the QAPI module. > This is the final part, part six! > > Part 6 (Everything): > https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6 > Note: I have rebased and updated the pt6 branch. I am currently working on respinning pt2, which now contains the introspect.py patches that were formerly in pt1. Thanks, --js > - Requires Python 3.6+ > - Requires mypy 0.770 or newer (for type analysis only) > - Requires pylint 2.6.0 or newer (for lint checking only) > > This part of the series focuses on schema.py. > > Type hints are added in patches that add *only* type hints and change no > other behavior. Any necessary changes to behavior to accommodate typing > are split out into their own tiny patches. > > Every commit should pass with: > - flake8 qapi/ > - pylint --rcfile=qapi/pylintrc qapi/ > - mypy --config-file=qapi/mypy.ini qapi/ > > John Snow (25): > qapi/schema: add Visitable mixin > qapi/schema.py: Move meta-type into class instances > qapi/schema.py: add assert in stub methods > qapi/schema.py: constrain QAPISchemaObjectType base type > qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type > qapi/schema.py: constrain QAPISchemaEvent arg_type type > qapi/schema.py: constrain tag_member type > qapi/schema.py: Allow alternate_type to assert > qapi/schema.py: remove superfluous assert > qapi/schema.py: Add assertion to ifcond property > qapi/schema.py: Constrain type of QAPISchemaObjectType members field > qapi/schema.py: remove 'and' from non-bool rvalue expressions > qapi/schema.py: Test type of self.ret_type instead of local temp > qapi/schema.py: Assert variants of an object are also objects > qapi/schema.py: add type hint annotations > qapi/schema.py: enable checking > qapi: Disable similarity checks in pylint entirely > qapi/schema.py: Add pylint warning suppressions > qapi/schema.py: Convert several methods to classmethods > qapi/schema.py: Replace one-letter variable names > qapi/schema.py: disable pylint line limit > qapi/schema.py: Ignore unused argument for check() > qapi/schema.py: enable pylint checks > qapi/schema.py: Add module docstring > qapi/schema.py: Use python3 style super() > > scripts/qapi/mypy.ini | 6 - > scripts/qapi/pylintrc | 6 +- > scripts/qapi/schema.py | 848 +++++++++++++++++++++++++++-------------- > 3 files changed, 557 insertions(+), 303 deletions(-) > ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-10-22 14:53 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-22 22:44 [PATCH 00/25] qapi: static typing conversion, pt6 John Snow 2020-09-22 22:44 ` [PATCH 01/25] qapi/schema: add Visitable mixin John Snow 2020-09-22 22:44 ` [PATCH 02/25] qapi/schema.py: Move meta-type into class instances John Snow 2020-09-22 22:44 ` [PATCH 03/25] qapi/schema.py: add assert in stub methods John Snow 2020-09-22 22:44 ` [PATCH 04/25] qapi/schema.py: constrain QAPISchemaObjectType base type John Snow 2020-09-22 22:44 ` [PATCH 05/25] qapi/schema.py: constrain QAPISchemaObjectTypeMember arg_type type John Snow 2020-09-22 22:44 ` [PATCH 06/25] qapi/schema.py: constrain QAPISchemaEvent " John Snow 2020-09-22 22:44 ` [PATCH 07/25] qapi/schema.py: constrain tag_member type John Snow 2020-09-22 22:44 ` [PATCH 08/25] qapi/schema.py: Allow alternate_type to assert John Snow 2020-09-22 22:44 ` [PATCH 09/25] qapi/schema.py: remove superfluous assert John Snow 2020-09-22 22:44 ` [PATCH 10/25] qapi/schema.py: Add assertion to ifcond property John Snow 2020-09-22 22:44 ` [PATCH 11/25] qapi/schema.py: Constrain type of QAPISchemaObjectType members field John Snow 2020-09-22 22:44 ` [PATCH 12/25] qapi/schema.py: remove 'and' from non-bool rvalue expressions John Snow 2020-09-22 22:44 ` [PATCH 13/25] qapi/schema.py: Test type of self.ret_type instead of local temp John Snow 2020-09-22 22:44 ` [PATCH 14/25] qapi/schema.py: Assert variants of an object are also objects John Snow 2020-09-22 22:44 ` [PATCH 15/25] qapi/schema.py: add type hint annotations John Snow 2020-09-22 22:44 ` [PATCH 16/25] qapi/schema.py: enable checking John Snow 2020-09-22 22:44 ` [PATCH 17/25] qapi: Disable similarity checks in pylint entirely John Snow 2020-09-22 22:44 ` [PATCH 18/25] qapi/schema.py: Add pylint warning suppressions John Snow 2020-09-22 22:44 ` [PATCH 19/25] qapi/schema.py: Convert several methods to classmethods John Snow 2020-09-22 22:44 ` [PATCH 20/25] qapi/schema.py: Replace one-letter variable names John Snow 2020-09-22 22:44 ` [PATCH 21/25] qapi/schema.py: disable pylint line limit John Snow 2020-09-22 22:44 ` [PATCH 22/25] qapi/schema.py: Ignore unused argument for check() John Snow 2020-09-22 22:44 ` [PATCH 23/25] qapi/schema.py: enable pylint checks John Snow 2020-09-22 22:45 ` [PATCH 24/25] qapi/schema.py: Add module docstring John Snow 2020-09-22 22:45 ` [PATCH 25/25] qapi/schema.py: Use python3 style super() John Snow 2020-10-22 14:51 ` [PATCH 00/25] qapi: static typing conversion, pt6 John Snow
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).