diff options
author | Richard Henderson <richard.henderson@linaro.org> | 2021-10-02 09:03:55 -0400 |
---|---|---|
committer | Richard Henderson <richard.henderson@linaro.org> | 2021-10-02 09:03:55 -0400 |
commit | f50ecf548c7313c27037f7b7fb8ecc5a5e89249c (patch) | |
tree | 02bbb52a12da86caa459b321507cef2f241be486 | |
parent | 5f992102383ed8ed97076548e1c897c7034ed8a4 (diff) | |
parent | d183e0481b1510b253ac94e702c76115f3bb6450 (diff) |
Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-10-02' into staging
QAPI patches patches for 2021-10-02
# gpg: Signature made Sat 02 Oct 2021 01:37:11 AM EDT
# gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653
# gpg: issuer "armbru@redhat.com"
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
# gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full]
* remotes/armbru/tags/pull-qapi-2021-10-02:
qapi/parser: enable pylint checks
qapi/parser: Silence too-few-public-methods warning
qapi/parser: enable mypy checks
qapi/parser: Add FIXME for consolidating JSON-related types
qapi/parser: add type hint annotations (QAPIDoc)
qapi/parser: add import cycle workaround
qapi/parser: Introduce NullSection
qapi/parser: clarify _end_section() logic
qapi/parser: remove FIXME comment from _append_body_line
qapi: Add spaces after symbol declaration for consistency
qapi/parser: fix unused check_args_section arguments
qapi/gen: use dict.items() to iterate over _modules
qapi/pylintrc: ignore 'consider-using-f-string' warning
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
-rw-r--r-- | qapi/block-core.json | 1 | ||||
-rw-r--r-- | qga/qapi-schema.json | 3 | ||||
-rw-r--r-- | scripts/qapi/gen.py | 3 | ||||
-rw-r--r-- | scripts/qapi/mypy.ini | 5 | ||||
-rw-r--r-- | scripts/qapi/parser.py | 152 | ||||
-rw-r--r-- | scripts/qapi/pylintrc | 4 | ||||
-rw-r--r-- | tests/qapi-schema/doc-bad-feature.err | 2 | ||||
-rw-r--r-- | tests/qapi-schema/doc-empty-symbol.err | 2 | ||||
-rw-r--r-- | tests/qapi-schema/doc-good.json | 8 |
9 files changed, 114 insertions, 66 deletions
diff --git a/qapi/block-core.json b/qapi/block-core.json index 623a4f4a3f..6d3217abb6 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3132,6 +3132,7 @@ ## # @BlockdevQcow2EncryptionFormat: +# # @aes: AES-CBC with plain64 initialization vectors # # Since: 2.10 diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index c60f5e669d..94e4aacdcc 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1140,6 +1140,7 @@ ## # @GuestExec: +# # @pid: pid of child process in guest OS # # Since: 2.5 @@ -1171,6 +1172,7 @@ ## # @GuestHostName: +# # @host-name: Fully qualified domain name of the guest OS # # Since: 2.10 @@ -1197,6 +1199,7 @@ ## # @GuestUser: +# # @user: Username # @domain: Logon domain (windows only) # @login-time: Time of login of this user on the computer. If multiple diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index ab26d5c937..2ec1e7b3b6 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -296,10 +296,9 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._current_module = old_module def write(self, output_dir: str, opt_builtins: bool = False) -> None: - for name in self._module: + for name, (genc, genh) in self._module.items(): if QAPISchemaModule.is_builtin_module(name) and not opt_builtins: continue - (genc, genh) = self._module[name] genc.write(output_dir) genh.write(output_dir) diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini index 54ca4483d6..6625356429 100644 --- a/scripts/qapi/mypy.ini +++ b/scripts/qapi/mypy.ini @@ -3,11 +3,6 @@ strict = True disallow_untyped_calls = False python_version = 3.6 -[mypy-qapi.parser] -disallow_untyped_defs = False -disallow_incomplete_defs = False -check_untyped_defs = False - [mypy-qapi.schema] disallow_untyped_defs = False disallow_incomplete_defs = False diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index f03ba2cfec..1b006cdc13 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -18,6 +18,7 @@ from collections import OrderedDict import os import re from typing import ( + TYPE_CHECKING, Dict, List, Optional, @@ -30,9 +31,22 @@ from .error import QAPISemError, QAPISourceError from .source import QAPISourceInfo +if TYPE_CHECKING: + # pylint: disable=cyclic-import + # TODO: Remove cycle. [schema -> expr -> parser -> schema] + from .schema import QAPISchemaFeature, QAPISchemaMember + + +#: Represents a single Top Level QAPI schema expression. +TopLevelExpr = Dict[str, object] + # Return value alias for get_expr(). _ExprValue = Union[List[object], Dict[str, object], str, bool] +# FIXME: Consolidate and centralize definitions for TopLevelExpr, +# _ExprValue, _JSONValue, and _JSONObject; currently scattered across +# several modules. + class QAPIParseError(QAPISourceError): """Error class for all QAPI schema parsing errors.""" @@ -447,7 +461,10 @@ class QAPIDoc: """ class Section: - def __init__(self, parser, name=None, indent=0): + # pylint: disable=too-few-public-methods + def __init__(self, parser: QAPISchemaParser, + name: Optional[str] = None, indent: int = 0): + # parser, for error messages about indentation self._parser = parser # optional section name (argument/member or section name) @@ -456,7 +473,7 @@ class QAPIDoc: # the expected indent level of the text of this section self._indent = indent - def append(self, line): + def append(self, line: str) -> None: # Strip leading spaces corresponding to the expected indent level # Blank lines are always OK. if line: @@ -471,39 +488,47 @@ class QAPIDoc: self.text += line.rstrip() + '\n' class ArgSection(Section): - def __init__(self, parser, name, indent=0): + def __init__(self, parser: QAPISchemaParser, + name: str, indent: int = 0): super().__init__(parser, name, indent) - self.member = None + self.member: Optional['QAPISchemaMember'] = None - def connect(self, member): + def connect(self, member: 'QAPISchemaMember') -> None: self.member = member - def __init__(self, parser, info): + class NullSection(Section): + """ + Immutable dummy section for use at the end of a doc block. + """ + # pylint: disable=too-few-public-methods + def append(self, line: str) -> None: + assert False, "Text appended after end_comment() called." + + def __init__(self, parser: QAPISchemaParser, info: QAPISourceInfo): # self._parser is used to report errors with QAPIParseError. The # resulting error position depends on the state of the parser. # It happens to be the beginning of the comment. More or less # servicable, but action at a distance. self._parser = parser self.info = info - self.symbol = None + self.symbol: Optional[str] = None self.body = QAPIDoc.Section(parser) - # dict mapping parameter name to ArgSection - self.args = OrderedDict() - self.features = OrderedDict() - # a list of Section - self.sections = [] + # dicts mapping parameter/feature names to their ArgSection + self.args: Dict[str, QAPIDoc.ArgSection] = OrderedDict() + self.features: Dict[str, QAPIDoc.ArgSection] = OrderedDict() + self.sections: List[QAPIDoc.Section] = [] # the current section self._section = self.body self._append_line = self._append_body_line - def has_section(self, name): + def has_section(self, name: str) -> bool: """Return True if we have a section with this name.""" for i in self.sections: if i.name == name: return True return False - def append(self, line): + def append(self, line: str) -> None: """ Parse a comment line and add it to the documentation. @@ -524,18 +549,18 @@ class QAPIDoc: line = line[1:] self._append_line(line) - def end_comment(self): - self._end_section() + def end_comment(self) -> None: + self._switch_section(QAPIDoc.NullSection(self._parser)) @staticmethod - def _is_section_tag(name): + def _is_section_tag(name: str) -> bool: return name in ('Returns:', 'Since:', # those are often singular or plural 'Note:', 'Notes:', 'Example:', 'Examples:', 'TODO:') - def _append_body_line(self, line): + def _append_body_line(self, line: str) -> None: """ Process a line of documentation text in the body section. @@ -556,9 +581,11 @@ class QAPIDoc: if not line.endswith(':'): raise QAPIParseError(self._parser, "line should end with ':'") self.symbol = line[1:-1] - # FIXME invalid names other than the empty string aren't flagged + # Invalid names are not checked here, but the name provided MUST + # match the following definition, which *is* validated in expr.py. if not self.symbol: - raise QAPIParseError(self._parser, "invalid name") + raise QAPIParseError( + self._parser, "name required after '@'") elif self.symbol: # This is a definition documentation block if name.startswith('@') and name.endswith(':'): @@ -575,7 +602,7 @@ class QAPIDoc: # This is a free-form documentation block self._append_freeform(line) - def _append_args_line(self, line): + def _append_args_line(self, line: str) -> None: """ Process a line of documentation text in an argument section. @@ -621,7 +648,7 @@ class QAPIDoc: self._append_freeform(line) - def _append_features_line(self, line): + def _append_features_line(self, line: str) -> None: name = line.split(' ', 1)[0] if name.startswith('@') and name.endswith(':'): @@ -653,7 +680,7 @@ class QAPIDoc: self._append_freeform(line) - def _append_various_line(self, line): + def _append_various_line(self, line: str) -> None: """ Process a line of documentation text in an additional section. @@ -689,7 +716,11 @@ class QAPIDoc: self._append_freeform(line) - def _start_symbol_section(self, symbols_dict, name, indent): + def _start_symbol_section( + self, + symbols_dict: Dict[str, 'QAPIDoc.ArgSection'], + name: str, + indent: int) -> None: # FIXME invalid names other than the empty string aren't flagged if not name: raise QAPIParseError(self._parser, "invalid parameter name") @@ -697,34 +728,41 @@ class QAPIDoc: raise QAPIParseError(self._parser, "'%s' parameter name duplicated" % name) assert not self.sections - self._end_section() - self._section = QAPIDoc.ArgSection(self._parser, name, indent) - symbols_dict[name] = self._section + new_section = QAPIDoc.ArgSection(self._parser, name, indent) + self._switch_section(new_section) + symbols_dict[name] = new_section - def _start_args_section(self, name, indent): + def _start_args_section(self, name: str, indent: int) -> None: self._start_symbol_section(self.args, name, indent) - def _start_features_section(self, name, indent): + def _start_features_section(self, name: str, indent: int) -> None: self._start_symbol_section(self.features, name, indent) - def _start_section(self, name=None, indent=0): + def _start_section(self, name: Optional[str] = None, + indent: int = 0) -> None: if name in ('Returns', 'Since') and self.has_section(name): raise QAPIParseError(self._parser, "duplicated '%s' section" % name) - self._end_section() - self._section = QAPIDoc.Section(self._parser, name, indent) - self.sections.append(self._section) - - def _end_section(self): - if self._section: - text = self._section.text = self._section.text.strip() - if self._section.name and (not text or text.isspace()): - raise QAPIParseError( - self._parser, - "empty doc section '%s'" % self._section.name) - self._section = None + new_section = QAPIDoc.Section(self._parser, name, indent) + self._switch_section(new_section) + self.sections.append(new_section) + + def _switch_section(self, new_section: 'QAPIDoc.Section') -> None: + text = self._section.text = self._section.text.strip() + + # Only the 'body' section is allowed to have an empty body. + # All other sections, including anonymous ones, must have text. + if self._section != self.body and not text: + # We do not create anonymous sections unless there is + # something to put in them; this is a parser bug. + assert self._section.name + raise QAPIParseError( + self._parser, + "empty doc section '%s'" % self._section.name) + + self._section = new_section - def _append_freeform(self, line): + def _append_freeform(self, line: str) -> None: match = re.match(r'(@\S+:)', line) if match: raise QAPIParseError(self._parser, @@ -732,37 +770,41 @@ class QAPIDoc: % match.group(1)) self._section.append(line) - def connect_member(self, member): + def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self.args: # Undocumented TODO outlaw self.args[member.name] = QAPIDoc.ArgSection(self._parser, member.name) self.args[member.name].connect(member) - def connect_feature(self, feature): + def connect_feature(self, feature: 'QAPISchemaFeature') -> None: if feature.name not in self.features: raise QAPISemError(feature.info, "feature '%s' lacks documentation" % feature.name) self.features[feature.name].connect(feature) - def check_expr(self, expr): + def check_expr(self, expr: TopLevelExpr) -> None: if self.has_section('Returns') and 'command' not in expr: raise QAPISemError(self.info, "'Returns:' is only valid for commands") - def check(self): + def check(self) -> None: - def check_args_section(args, info, what): + def check_args_section( + args: Dict[str, QAPIDoc.ArgSection], what: str + ) -> None: bogus = [name for name, section in args.items() if not section.member] if bogus: raise QAPISemError( self.info, - "documented member%s '%s' %s not exist" - % ("s" if len(bogus) > 1 else "", - "', '".join(bogus), - "do" if len(bogus) > 1 else "does")) - - check_args_section(self.args, self.info, 'members') - check_args_section(self.features, self.info, 'features') + "documented %s%s '%s' %s not exist" % ( + what, + "s" if len(bogus) > 1 else "", + "', '".join(bogus), + "do" if len(bogus) > 1 else "does" + )) + + check_args_section(self.args, 'member') + check_args_section(self.features, 'feature') diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc index c5275d5f59..b259531a72 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=parser.py, - schema.py, +ignore-patterns=schema.py, [MESSAGES CONTROL] @@ -23,6 +22,7 @@ disable=fixme, too-many-branches, too-many-statements, too-many-instance-attributes, + consider-using-f-string, [REPORTS] diff --git a/tests/qapi-schema/doc-bad-feature.err b/tests/qapi-schema/doc-bad-feature.err index e4c62adfa3..49d1746c3d 100644 --- a/tests/qapi-schema/doc-bad-feature.err +++ b/tests/qapi-schema/doc-bad-feature.err @@ -1 +1 @@ -doc-bad-feature.json:3: documented member 'a' does not exist +doc-bad-feature.json:3: documented feature 'a' does not exist diff --git a/tests/qapi-schema/doc-empty-symbol.err b/tests/qapi-schema/doc-empty-symbol.err index 81b90e882a..aa51be41b2 100644 --- a/tests/qapi-schema/doc-empty-symbol.err +++ b/tests/qapi-schema/doc-empty-symbol.err @@ -1 +1 @@ -doc-empty-symbol.json:4:1: invalid name +doc-empty-symbol.json:4:1: name required after '@' diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index a20acffd8b..86dc25d2bd 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -53,6 +53,7 @@ ## # @Enum: +# # @one: The _one_ {and only} # # Features: @@ -67,6 +68,7 @@ ## # @Base: +# # @base1: # the first member ## @@ -75,6 +77,7 @@ ## # @Variant1: +# # A paragraph # # Another paragraph (but no @var: line) @@ -91,11 +94,13 @@ ## # @Variant2: +# ## { 'struct': 'Variant2', 'data': {} } ## # @Object: +# # Features: # @union-feat1: a feature ## @@ -109,6 +114,7 @@ ## # @Alternate: +# # @i: an integer # @b is undocumented # @@ -126,6 +132,7 @@ ## # @cmd: +# # @arg1: the first argument # # @arg2: the second @@ -175,6 +182,7 @@ ## # @EVT_BOXED: +# # Features: # @feat3: a feature ## |