[PATCH 00/14] qapi: static typing conversion, pt3

[PATCH 00/14] qapi: static typing conversion, pt3

[John Snow]

based-on: <20200922211313.4082880-1-jsnow@redhat.com>
          [PATCH 00/16] qapi: static typing conversion, pt2

Hi, this series adds static type hints to the QAPI module.
This is part three!

Part 3: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt3
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 just doc.py ... which, err, Peter
Maydell is getting rid of soon. Oh well. This is posted for reference;
and simply as context and pre-requisite for later patches.

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 (14):
  qapi/doc.py: stash long temporary locals in named locals
  qapi/doc.py: avoid unnecessary keyword arguments
  qapi/doc.py: Add assertion on section.member
  qapi/doc.py: assert correct types in member_func callbacks
  qapi/doc.py: Assert no suffix given for enum members
  qapi/doc.py: Add type hint annotations
  qapi/doc.py: enable mypy checks
  qapi/doc.py: Add generic texi_member callback
  qapi/doc.py: Remove one-letter variables
  qapi/gen.py: Add __bool__ dunder method to QAPIGen
  qapi/doc.py: Don't use private attributes of QAPIGen property
  qapi/doc.py: Assert tag member is Enum type
  qapi/doc.py: Assert type of object variant
  qapi/doc.py: enable pylint checks

 scripts/qapi/doc.py   | 189 ++++++++++++++++++++++++++++--------------
 scripts/qapi/gen.py   |   3 +
 scripts/qapi/mypy.ini |   5 --
 scripts/qapi/pylintrc |   3 +-
 4 files changed, 133 insertions(+), 67 deletions(-)

-- 
2.26.2

[PATCH 01/14] qapi/doc.py: stash long temporary locals in named locals

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 70f7cdfaa6..4743beb89a 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -267,15 +267,13 @@ def visit_command(self, name, info, ifcond, features,
                       arg_type, ret_type, gen, success_response, boxed,
                       allow_oob, allow_preconfig):
         doc = self.cur_doc
-        self._gen.add(texi_msg('Command', doc, ifcond,
-                               texi_arguments(doc,
-                                              arg_type if boxed else None)))
+        members = texi_arguments(doc, arg_type if boxed else None)
+        self._gen.add(texi_msg('Command', doc, ifcond, members))
 
     def visit_event(self, name, info, ifcond, features, arg_type, boxed):
         doc = self.cur_doc
-        self._gen.add(texi_msg('Event', doc, ifcond,
-                               texi_arguments(doc,
-                                              arg_type if boxed else None)))
+        members = texi_arguments(doc, arg_type if boxed else None)
+        self._gen.add(texi_msg('Event', doc, ifcond, members))
 
     def symbol(self, doc, entity):
         if self._gen._body:
-- 
2.26.2

[PATCH 02/14] qapi/doc.py: avoid unnecessary keyword arguments

[John Snow]

Keyword Callables are hard to type in Python 3.6, avoid them if there's
no urgent need to use them.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 4743beb89a..66333629d6 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -164,7 +164,8 @@ def texi_members(doc, what, base=None, variants=None,
             desc = 'One of ' + members_text + '\n'
         else:
             desc = 'Not documented\n'
-        items += member_func(section.member, desc, suffix='')
+
+        items += member_func(section.member, desc, '')
     if base:
         items += '@item The members of @code{%s}\n' % base.doc_type()
     if variants:
@@ -174,7 +175,7 @@ def texi_members(doc, what, base=None, variants=None,
             if v.type.is_implicit():
                 assert not v.type.base and not v.type.variants
                 for m in v.type.local_members:
-                    items += member_func(m, desc='', suffix=when)
+                    items += member_func(m, '', when)
             else:
                 items += '@item The members of @code{%s}%s\n' % (
                     v.type.doc_type(), when)
-- 
2.26.2

[PATCH 03/14] qapi/doc.py: Add assertion on section.member

[John Snow]

Similarly to other cases, we lack the power at the moment to express
that a specific member is constrained to a certain containing type. Add
an assertion before we use properties specific to that type.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 66333629d6..c645876b24 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -5,9 +5,10 @@
 """This script produces the documentation of a qapi schema in texinfo format"""
 
 import re
+from typing import Optional
+
 from .gen import QAPIGenDoc
-from .schema import QAPISchemaVisitor
-
+from .schema import QAPISchemaVisitor, QAPISchemaObjectTypeMember
 
 MSG_FMT = """
 @deftypefn {type} {{}} {name}
@@ -155,14 +156,17 @@ def texi_members(doc, what, base=None, variants=None,
     items = ''
     for section in doc.args.values():
         # TODO Drop fallbacks when undocumented members are outlawed
+        desc: Optional[str] = None
+
         if section.text:
             desc = texi_format(section.text)
-        elif (variants and variants.tag_member == section.member
-              and not section.member.type.doc_type()):
-            values = section.member.type.member_names()
-            members_text = ', '.join(['@t{"%s"}' % v for v in values])
-            desc = 'One of ' + members_text + '\n'
-        else:
+        elif variants and variants.tag_member == section.member:
+            assert isinstance(section.member, QAPISchemaObjectTypeMember)
+            if not section.member.type.doc_type():
+                values = section.member.type.member_names()
+                members_text = ', '.join(['@t{"%s"}' % v for v in values])
+                desc = 'One of ' + members_text + '\n'
+        if desc is None:
             desc = 'Not documented\n'
 
         items += member_func(section.member, desc, '')
-- 
2.26.2

[PATCH 04/14] qapi/doc.py: assert correct types in member_func callbacks

[John Snow]

These each take a specific subtype; assert that they got that correct
subtype.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index c645876b24..62b8f34707 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -8,7 +8,11 @@
 from typing import Optional
 
 from .gen import QAPIGenDoc
-from .schema import QAPISchemaVisitor, QAPISchemaObjectTypeMember
+from .schema import (
+    QAPISchemaEnumMember,
+    QAPISchemaObjectTypeMember,
+    QAPISchemaVisitor,
+)
 
 MSG_FMT = """
 @deftypefn {type} {{}} {name}
@@ -136,12 +140,14 @@ def texi_if(ifcond, prefix='\n', suffix='\n'):
 
 def texi_enum_value(value, desc, suffix):
     """Format a table of members item for an enumeration value"""
+    assert isinstance(value, QAPISchemaEnumMember)
     return '@item @code{%s}\n%s%s' % (
         value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
 
 def texi_member(member, desc, suffix):
     """Format a table of members item for an object type member"""
+    assert isinstance(member, QAPISchemaObjectTypeMember)
     typ = member.type.doc_type()
     membertype = ': ' + typ if typ else ''
     return '@item @code{%s%s}%s%s\n%s%s' % (
-- 
2.26.2

[PATCH 05/14] qapi/doc.py: Assert no suffix given for enum members

[John Snow]

We don't do anything with this argument. If something shows up here,
something wrong has happened.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 62b8f34707..9a02cf2f27 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -141,6 +141,7 @@ def texi_if(ifcond, prefix='\n', suffix='\n'):
 def texi_enum_value(value, desc, suffix):
     """Format a table of members item for an enumeration value"""
     assert isinstance(value, QAPISchemaEnumMember)
+    assert suffix == '', "Ignored suffix for texi_enum_value"
     return '@item @code{%s}\n%s%s' % (
         value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
-- 
2.26.2

[PATCH 06/14] qapi/doc.py: Add type hint annotations

[John Snow]

Annotations do not change runtime behavior.
This commit *only* adds annotations.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 119 ++++++++++++++++++++++++++++++++------------
 1 file changed, 87 insertions(+), 32 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 9a02cf2f27..43943575da 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -5,14 +5,24 @@
 """This script produces the documentation of a qapi schema in texinfo format"""
 
 import re
-from typing import Optional
+from typing import Callable, List, Optional
 
 from .gen import QAPIGenDoc
+from .parser import QAPIDoc
 from .schema import (
+    QAPISchema,
+    QAPISchemaEntity,
     QAPISchemaEnumMember,
+    QAPISchemaFeature,
+    QAPISchemaMember,
+    QAPISchemaObjectType,
     QAPISchemaObjectTypeMember,
+    QAPISchemaType,
+    QAPISchemaVariants,
     QAPISchemaVisitor,
 )
+from .source import QAPISourceInfo
+
 
 MSG_FMT = """
 @deftypefn {type} {{}} {name}
@@ -36,27 +46,27 @@
 """.format
 
 
-def subst_strong(doc):
+def subst_strong(doc: str) -> str:
     """Replaces *foo* by @strong{foo}"""
     return re.sub(r'\*([^*\n]+)\*', r'@strong{\1}', doc)
 
 
-def subst_emph(doc):
+def subst_emph(doc: str) -> str:
     """Replaces _foo_ by @emph{foo}"""
     return re.sub(r'\b_([^_\n]+)_\b', r'@emph{\1}', doc)
 
 
-def subst_vars(doc):
+def subst_vars(doc: str) -> str:
     """Replaces @var by @code{var}"""
     return re.sub(r'@([\w-]+)', r'@code{\1}', doc)
 
 
-def subst_braces(doc):
+def subst_braces(doc: str) -> str:
     """Replaces {} with @{ @}"""
     return doc.replace('{', '@{').replace('}', '@}')
 
 
-def texi_example(doc):
+def texi_example(doc: str) -> str:
     """Format @example"""
     # TODO: Neglects to escape @ characters.
     # We should probably escape them in subst_braces(), and rename the
@@ -66,7 +76,7 @@ def texi_example(doc):
     return EXAMPLE_FMT(code=doc)
 
 
-def texi_format(doc):
+def texi_format(doc: str) -> str:
     """
     Format documentation
 
@@ -126,19 +136,26 @@ def texi_format(doc):
     return ret
 
 
-def texi_body(doc):
+def texi_body(doc: QAPIDoc) -> str:
     """Format the main documentation body"""
     return texi_format(doc.body.text)
 
 
-def texi_if(ifcond, prefix='\n', suffix='\n'):
+def texi_if(ifcond: Optional[List[str]],
+            prefix: str = '\n',
+            suffix: str = '\n') -> str:
     """Format the #if condition"""
     if not ifcond:
         return ''
     return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix)
 
 
-def texi_enum_value(value, desc, suffix):
+TexiMemberCallback = Callable[[QAPISchemaMember, str, str], str]
+
+
+def texi_enum_value(value: QAPISchemaMember,
+                    desc: str,
+                    suffix: str) -> str:
     """Format a table of members item for an enumeration value"""
     assert isinstance(value, QAPISchemaEnumMember)
     assert suffix == '', "Ignored suffix for texi_enum_value"
@@ -146,7 +163,9 @@ def texi_enum_value(value, desc, suffix):
         value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
 
-def texi_member(member, desc, suffix):
+def texi_member(member: QAPISchemaMember,
+                desc: str,
+                suffix: str) -> str:
     """Format a table of members item for an object type member"""
     assert isinstance(member, QAPISchemaObjectTypeMember)
     typ = member.type.doc_type()
@@ -157,8 +176,11 @@ def texi_member(member, desc, suffix):
         suffix, desc, texi_if(member.ifcond, prefix='@*'))
 
 
-def texi_members(doc, what, base=None, variants=None,
-                 member_func=texi_member):
+def texi_members(doc: QAPIDoc,
+                 what: str,
+                 base: Optional[QAPISchemaObjectType] = None,
+                 variants: Optional[QAPISchemaVariants] = None,
+                 member_func: TexiMemberCallback = texi_member) -> str:
     """Format the table of members"""
     items = ''
     for section in doc.args.values():
@@ -195,7 +217,8 @@ def texi_members(doc, what, base=None, variants=None,
     return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
 
 
-def texi_arguments(doc, boxed_arg_type):
+def texi_arguments(doc: QAPIDoc,
+                   boxed_arg_type: Optional[QAPISchemaObjectType]) -> str:
     if boxed_arg_type:
         assert not doc.args
         return ('\n@b{Arguments:} the members of @code{%s}\n'
@@ -203,7 +226,7 @@ def texi_arguments(doc, boxed_arg_type):
     return texi_members(doc, 'Arguments')
 
 
-def texi_features(doc):
+def texi_features(doc: QAPIDoc) -> str:
     """Format the table of features"""
     items = ''
     for section in doc.features.values():
@@ -214,7 +237,7 @@ def texi_features(doc):
     return '\n@b{Features:}\n@table @asis\n%s@end table\n' % (items)
 
 
-def texi_sections(doc, ifcond):
+def texi_sections(doc: QAPIDoc, ifcond: Optional[List[str]]) -> str:
     """Format additional sections following arguments"""
     body = ''
     for section in doc.sections:
@@ -229,7 +252,7 @@ def texi_sections(doc, ifcond):
     return body
 
 
-def texi_type(typ, doc, ifcond, members):
+def texi_type(typ: str, doc: QAPIDoc, ifcond: List[str], members: str) -> str:
     return TYPE_FMT(type=typ,
                     name=doc.symbol,
                     body=texi_body(doc),
@@ -238,7 +261,7 @@ def texi_type(typ, doc, ifcond, members):
                     sections=texi_sections(doc, ifcond))
 
 
-def texi_msg(typ, doc, ifcond, members):
+def texi_msg(typ: str, doc: QAPIDoc, ifcond: List[str], members: str) -> str:
     return MSG_FMT(type=typ,
                    name=doc.symbol,
                    body=texi_body(doc),
@@ -248,60 +271,92 @@ def texi_msg(typ, doc, ifcond, members):
 
 
 class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
-    def __init__(self, prefix):
+    def __init__(self, prefix: str):
         self._prefix = prefix
         self._gen = QAPIGenDoc(self._prefix + 'qapi-doc.texi')
-        self.cur_doc = None
+        self.cur_doc: Optional[QAPIDoc] = None
 
-    def write(self, output_dir):
+    def write(self, output_dir: str) -> None:
         self._gen.write(output_dir)
 
-    def visit_enum_type(self, name, info, ifcond, features, members, prefix):
+    def visit_enum_type(self,
+                        name: str,
+                        info: QAPISourceInfo,
+                        ifcond: List[str],
+                        features: List[QAPISchemaFeature],
+                        members: List[QAPISchemaEnumMember],
+                        prefix: Optional[str]) -> None:
         doc = self.cur_doc
         self._gen.add(texi_type('Enum', doc, ifcond,
                                 texi_members(doc, 'Values',
                                              member_func=texi_enum_value)))
 
-    def visit_object_type(self, name, info, ifcond, features,
-                          base, members, variants):
+    def visit_object_type(self,
+                          name: str,
+                          info: QAPISourceInfo,
+                          ifcond: List[str],
+                          features: List[QAPISchemaFeature],
+                          base: Optional[QAPISchemaObjectType],
+                          members: List[QAPISchemaObjectTypeMember],
+                          variants: Optional[QAPISchemaVariants]) -> None:
         doc = self.cur_doc
         if base and base.is_implicit():
             base = None
         self._gen.add(texi_type('Object', doc, ifcond,
                                 texi_members(doc, 'Members', base, variants)))
 
-    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:
         doc = self.cur_doc
         self._gen.add(texi_type('Alternate', doc, ifcond,
                                 texi_members(doc, 'Members')))
 
-    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: QAPISchemaType,
+                      gen: bool,
+                      success_response: bool,
+                      boxed: bool,
+                      allow_oob: bool,
+                      allow_preconfig: bool) -> None:
         doc = self.cur_doc
         members = texi_arguments(doc, arg_type if boxed else None)
         self._gen.add(texi_msg('Command', doc, ifcond, members))
 
-    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:
         doc = self.cur_doc
         members = texi_arguments(doc, arg_type if boxed else None)
         self._gen.add(texi_msg('Event', doc, ifcond, members))
 
-    def symbol(self, doc, entity):
+    def symbol(self, doc: QAPIDoc, entity: QAPISchemaEntity) -> None:
         if self._gen._body:
             self._gen.add('\n')
         self.cur_doc = doc
         entity.visit(self)
         self.cur_doc = None
 
-    def freeform(self, doc):
+    def freeform(self, doc: QAPIDoc) -> None:
         assert not doc.args
         if self._gen._body:
             self._gen.add('\n')
         self._gen.add(texi_body(doc) + texi_sections(doc, None))
 
 
-def gen_doc(schema, output_dir, prefix):
+def gen_doc(schema: QAPISchema, output_dir: str, prefix: str) -> None:
     vis = QAPISchemaGenDocVisitor(prefix)
     vis.visit_begin(schema)
     for doc in schema.docs:
-- 
2.26.2

[PATCH 07/14] qapi/doc.py: enable mypy checks

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/mypy.ini | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
index 0d0111930f..df9b05e4ab 100644
--- a/scripts/qapi/mypy.ini
+++ b/scripts/qapi/mypy.ini
@@ -4,11 +4,6 @@ strict_optional = False
 disallow_untyped_calls = False
 python_version = 3.6
 
-[mypy-qapi.doc]
-disallow_subclassing_any = False
-disallow_untyped_defs = False
-disallow_incomplete_defs = False
-
 [mypy-qapi.error]
 disallow_untyped_defs = False
 disallow_incomplete_defs = False
-- 
2.26.2

[PATCH 08/14] qapi/doc.py: Add generic texi_member callback

[John Snow]

We can just use the same callback and dispatch based on type, which
removes the need to type the callback, add downcast assertions, or use
less-specific types in the specialized generators.

---

This patch is optional, it's just demonstrating a different way to
handle the callback typing -- by eliminating the callback.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 43943575da..74d017f60e 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -5,7 +5,7 @@
 """This script produces the documentation of a qapi schema in texinfo format"""
 
 import re
-from typing import Callable, List, Optional
+from typing import List, Optional
 
 from .gen import QAPIGenDoc
 from .parser import QAPIDoc
@@ -150,24 +150,17 @@ def texi_if(ifcond: Optional[List[str]],
     return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix)
 
 
-TexiMemberCallback = Callable[[QAPISchemaMember, str, str], str]
-
-
-def texi_enum_value(value: QAPISchemaMember,
-                    desc: str,
-                    suffix: str) -> str:
+def texi_enum_value(value: QAPISchemaEnumMember,
+                    desc: str, suffix: str) -> str:
     """Format a table of members item for an enumeration value"""
-    assert isinstance(value, QAPISchemaEnumMember)
     assert suffix == '', "Ignored suffix for texi_enum_value"
     return '@item @code{%s}\n%s%s' % (
         value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
 
-def texi_member(member: QAPISchemaMember,
-                desc: str,
-                suffix: str) -> str:
+def texi_object_member(member: QAPISchemaObjectTypeMember,
+                       desc: str, suffix: str) -> str:
     """Format a table of members item for an object type member"""
-    assert isinstance(member, QAPISchemaObjectTypeMember)
     typ = member.type.doc_type()
     membertype = ': ' + typ if typ else ''
     return '@item @code{%s%s}%s%s\n%s%s' % (
@@ -176,11 +169,20 @@ def texi_member(member: QAPISchemaMember,
         suffix, desc, texi_if(member.ifcond, prefix='@*'))
 
 
+def texi_member(member: QAPISchemaMember, desc: str, suffix: str) -> str:
+    """Format a table of members item for an arbitrary member type"""
+
+    if isinstance(member, QAPISchemaObjectTypeMember):
+        return texi_object_member(member, desc, suffix)
+    if isinstance(member, QAPISchemaEnumMember):
+        return texi_enum_value(member, desc, suffix)
+    raise Exception(f"Unhandled member type {type(member).__name__}")
+
+
 def texi_members(doc: QAPIDoc,
                  what: str,
                  base: Optional[QAPISchemaObjectType] = None,
-                 variants: Optional[QAPISchemaVariants] = None,
-                 member_func: TexiMemberCallback = texi_member) -> str:
+                 variants: Optional[QAPISchemaVariants] = None) -> str:
     """Format the table of members"""
     items = ''
     for section in doc.args.values():
@@ -198,7 +200,7 @@ def texi_members(doc: QAPIDoc,
         if desc is None:
             desc = 'Not documented\n'
 
-        items += member_func(section.member, desc, '')
+        items += texi_member(section.member, desc, '')
     if base:
         items += '@item The members of @code{%s}\n' % base.doc_type()
     if variants:
@@ -208,7 +210,7 @@ def texi_members(doc: QAPIDoc,
             if v.type.is_implicit():
                 assert not v.type.base and not v.type.variants
                 for m in v.type.local_members:
-                    items += member_func(m, '', when)
+                    items += texi_member(m, '', when)
             else:
                 items += '@item The members of @code{%s}%s\n' % (
                     v.type.doc_type(), when)
@@ -288,8 +290,7 @@ def visit_enum_type(self,
                         prefix: Optional[str]) -> None:
         doc = self.cur_doc
         self._gen.add(texi_type('Enum', doc, ifcond,
-                                texi_members(doc, 'Values',
-                                             member_func=texi_enum_value)))
+                                texi_members(doc, 'Values')))
 
     def visit_object_type(self,
                           name: str,
-- 
2.26.2

[PATCH 09/14] qapi/doc.py: Remove one-letter variables

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 74d017f60e..5f2b0cd51d 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -203,17 +203,17 @@ def texi_members(doc: QAPIDoc,
         items += texi_member(section.member, desc, '')
     if base:
         items += '@item The members of @code{%s}\n' % base.doc_type()
-    if variants:
-        for v in variants.variants:
-            when = ' when @code{%s} is @t{"%s"}%s' % (
-                variants.tag_member.name, v.name, texi_if(v.ifcond, " (", ")"))
-            if v.type.is_implicit():
-                assert not v.type.base and not v.type.variants
-                for m in v.type.local_members:
-                    items += texi_member(m, '', when)
-            else:
-                items += '@item The members of @code{%s}%s\n' % (
-                    v.type.doc_type(), when)
+    for variant in variants.variants if variants else ():
+        when = ' when @code{%s} is @t{"%s"}%s' % (
+            variants.tag_member.name, variant.name,
+            texi_if(variant.ifcond, " (", ")"))
+        if variant.type.is_implicit():
+            assert not variant.type.base and not variant.type.variants
+            for member in variant.type.local_members:
+                items += texi_member(member, '', when)
+        else:
+            items += '@item The members of @code{%s}%s\n' % (
+                variant.type.doc_type(), when)
     if not items:
         return ''
     return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
-- 
2.26.2

[PATCH 10/14] qapi/gen.py: Add __bool__ dunder method to QAPIGen

[John Snow]

Falseish when there is no body or preamble; Trueish when there is.
Header and footer are excluded for now, because they are assumed to be
dynamic and always present.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/gen.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index ed498397ad..892d7d772e 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -40,6 +40,9 @@ def __init__(self, fname: Optional[str]):
         self._preamble = ''
         self._body = ''
 
+    def __bool__(self) -> bool:
+        return bool(self._preamble or self._body)
+
     def preamble_add(self, text: str) -> None:
         self._preamble += text
 
-- 
2.26.2

[PATCH 11/14] qapi/doc.py: Don't use private attributes of QAPIGen property

[John Snow]

Use the new __bool__ method to do the same without exposing the private
attribute.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 5f2b0cd51d..2914e93b1c 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -344,7 +344,7 @@ def visit_event(self,
         self._gen.add(texi_msg('Event', doc, ifcond, members))
 
     def symbol(self, doc: QAPIDoc, entity: QAPISchemaEntity) -> None:
-        if self._gen._body:
+        if self._gen:
             self._gen.add('\n')
         self.cur_doc = doc
         entity.visit(self)
@@ -352,7 +352,7 @@ def symbol(self, doc: QAPIDoc, entity: QAPISchemaEntity) -> None:
 
     def freeform(self, doc: QAPIDoc) -> None:
         assert not doc.args
-        if self._gen._body:
+        if self._gen:
             self._gen.add('\n')
         self._gen.add(texi_body(doc) + texi_sections(doc, None))
 
-- 
2.26.2

[PATCH 12/14] qapi/doc.py: Assert tag member is Enum type

[John Snow]

The type system can't quite express this constraint natively: members
can envelop any type -- but tag_members may only ever envelop an
enumerated type.

For now, shrug and add an assertion.

Note: These assertions don't appear to be useful yet because schema.py
is not yet typed. Once it is, these assertions will matter.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 2914e93b1c..b96d9046d3 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -13,6 +13,7 @@
     QAPISchema,
     QAPISchemaEntity,
     QAPISchemaEnumMember,
+    QAPISchemaEnumType,
     QAPISchemaFeature,
     QAPISchemaMember,
     QAPISchemaObjectType,
@@ -193,6 +194,7 @@ def texi_members(doc: QAPIDoc,
             desc = texi_format(section.text)
         elif variants and variants.tag_member == section.member:
             assert isinstance(section.member, QAPISchemaObjectTypeMember)
+            assert isinstance(section.member.type, QAPISchemaEnumType)
             if not section.member.type.doc_type():
                 values = section.member.type.member_names()
                 members_text = ', '.join(['@t{"%s"}' % v for v in values])
-- 
2.26.2

[PATCH 13/14] qapi/doc.py: Assert type of object variant

[John Snow]

Objects may have variants, but those variants must themselves be
objects. This is difficult to express with our current type system and
hierarchy, so instead pepper in an assertion.

Note: These assertions don't appear to be useful yet because schema.py
is not yet typed. Once it is, these assertions will matter.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/doc.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index b96d9046d3..3de592e220 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -206,6 +206,7 @@ def texi_members(doc: QAPIDoc,
     if base:
         items += '@item The members of @code{%s}\n' % base.doc_type()
     for variant in variants.variants if variants else ():
+        assert isinstance(variant.type, QAPISchemaObjectType)
         when = ' when @code{%s} is @t{"%s"}%s' % (
             variants.tag_member.name, variant.name,
             texi_if(variant.ifcond, " (", ")"))
-- 
2.26.2

[PATCH 14/14] qapi/doc.py: enable pylint checks

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/pylintrc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
index d7adb2ba33..fb0386d529 100644
--- a/scripts/qapi/pylintrc
+++ b/scripts/qapi/pylintrc
@@ -2,8 +2,7 @@
 
 # Add files or directories matching the regex patterns to the ignore list.
 # The regex matches against base names, not paths.
-ignore-patterns=doc.py,
-                error.py,
+ignore-patterns=error.py,
                 parser.py,
                 schema.py,
 
-- 
2.26.2

Re: [PATCH 01/14] qapi/doc.py: stash long temporary locals in named locals

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:49PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 02/14] qapi/doc.py: avoid unnecessary keyword arguments

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:50PM -0400, John Snow wrote:
> Keyword Callables are hard to type in Python 3.6, avoid them if there's
> no urgent need to use them.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 03/14] qapi/doc.py: Add assertion on section.member

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:51PM -0400, John Snow wrote:
> Similarly to other cases, we lack the power at the moment to express
> that a specific member is constrained to a certain containing type. Add
> an assertion before we use properties specific to that type.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 04/14] qapi/doc.py: assert correct types in member_func callbacks

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:52PM -0400, John Snow wrote:
> These each take a specific subtype; assert that they got that correct
> subtype.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 05/14] qapi/doc.py: Assert no suffix given for enum members

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:53PM -0400, John Snow wrote:
> We don't do anything with this argument. If something shows up here,
> something wrong has happened.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 06/14] qapi/doc.py: Add type hint annotations

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:54PM -0400, John Snow wrote:
> Annotations do not change runtime behavior.
> This commit *only* adds annotations.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 07/14] qapi/doc.py: enable mypy checks

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:55PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 09/14] qapi/doc.py: Remove one-letter variables

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:57PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 10/14] qapi/gen.py: Add __bool__ dunder method to QAPIGen

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:58PM -0400, John Snow wrote:
> Falseish when there is no body or preamble; Trueish when there is.
> Header and footer are excluded for now, because they are assumed to be
> dynamic and always present.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 11/14] qapi/doc.py: Don't use private attributes of QAPIGen property

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:17:59PM -0400, John Snow wrote:
> Use the new __bool__ method to do the same without exposing the private
> attribute.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 12/14] qapi/doc.py: Assert tag member is Enum type

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:18:00PM -0400, John Snow wrote:
> The type system can't quite express this constraint natively: members
> can envelop any type -- but tag_members may only ever envelop an
> enumerated type.
> 
> For now, shrug and add an assertion.
> 
> Note: These assertions don't appear to be useful yet because schema.py
> is not yet typed. Once it is, these assertions will matter.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 13/14] qapi/doc.py: Assert type of object variant

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:18:01PM -0400, John Snow wrote:
> Objects may have variants, but those variants must themselves be
> objects. This is difficult to express with our current type system and
> hierarchy, so instead pepper in an assertion.
> 
> Note: These assertions don't appear to be useful yet because schema.py
> is not yet typed. Once it is, these assertions will matter.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 14/14] qapi/doc.py: enable pylint checks

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:18:02PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>

Tested-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 00/14] qapi: static typing conversion, pt3

[John Snow]

On 9/22/20 5:17 PM, John Snow wrote:
> based-on: <20200922211313.4082880-1-jsnow@redhat.com>
>            [PATCH 00/16] qapi: static typing conversion, pt2
> 
> Hi, this series adds static type hints to the QAPI module.
> This is part three!
> 
> Part 3: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt3
> Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6
> 

This entire series is now deprecated, because of Peter Maydell's work 
removing doc.py!

There will not be a replacement. The series now goes from part 1, 2, ... 
4, 5, 6.

--js