From 420110591c54f5fd38e065d5bddac73b3076bf9e Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 14 Feb 2023 19:00:09 -0500 Subject: qapi/parser: add QAPIExpression type This patch creates a new type, QAPIExpression, which represents a parsed expression complete with QAPIDoc and QAPISourceInfo. This patch turns parser.exprs into a list of QAPIExpression instead, and adjusts expr.py to match. This allows the types we specify in parser.py to be "remembered" all the way through expr.py and into schema.py. Several assertions around packing and unpacking this data can be removed as a result. It also corrects a harmless typing error. Before the patch, check_exprs() allegedly takes a List[_JSONObject]. It actually takes a list of dicts of the form {'expr': E, 'info': I, 'doc': D} where E is of type _ExprValue, I is of type QAPISourceInfo, and D is of type QAPIDoc. Key 'doc' is optional. This is not a _JSONObject! Passes type checking anyway, because _JSONObject is Dict[str, object]. Signed-off-by: John Snow Message-Id: <20230215000011.1725012-5-jsnow@redhat.com> Reviewed-by: Markus Armbruster [Commit message amended to point out the typing fix] --- scripts/qapi/parser.py | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 1b006cdc13..50906e27d4 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -21,6 +21,7 @@ from typing import ( TYPE_CHECKING, Dict, List, + Mapping, Optional, Set, Union, @@ -37,15 +38,24 @@ if TYPE_CHECKING: 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. + +# FIXME: Consolidate and centralize definitions for _ExprValue, +# JSONValue, and _JSONObject; currently scattered across several +# modules. + + +class QAPIExpression(Dict[str, object]): + # pylint: disable=too-few-public-methods + def __init__(self, + data: Mapping[str, object], + info: QAPISourceInfo, + doc: Optional['QAPIDoc'] = None): + super().__init__(data) + self.info = info + self.doc: Optional['QAPIDoc'] = doc class QAPIParseError(QAPISourceError): @@ -100,7 +110,7 @@ class QAPISchemaParser: self.line_pos = 0 # Parser output: - self.exprs: List[Dict[str, object]] = [] + self.exprs: List[QAPIExpression] = [] self.docs: List[QAPIDoc] = [] # Showtime! @@ -147,8 +157,7 @@ class QAPISchemaParser: "value of 'include' must be a string") incl_fname = os.path.join(os.path.dirname(self._fname), include) - self.exprs.append({'expr': {'include': incl_fname}, - 'info': info}) + self._add_expr(OrderedDict({'include': incl_fname}), info) exprs_include = self._include(include, info, incl_fname, self._included) if exprs_include: @@ -165,17 +174,18 @@ class QAPISchemaParser: for name, value in pragma.items(): self._pragma(name, value, info) else: - expr_elem = {'expr': expr, - 'info': info} - if cur_doc: - if not cur_doc.symbol: - raise QAPISemError( - cur_doc.info, "definition documentation required") - expr_elem['doc'] = cur_doc - self.exprs.append(expr_elem) + if cur_doc and not cur_doc.symbol: + raise QAPISemError( + cur_doc.info, "definition documentation required") + self._add_expr(expr, info, cur_doc) cur_doc = None self.reject_expr_doc(cur_doc) + def _add_expr(self, expr: Mapping[str, object], + info: QAPISourceInfo, + doc: Optional['QAPIDoc'] = None) -> None: + self.exprs.append(QAPIExpression(expr, info, doc)) + @staticmethod def reject_expr_doc(doc: Optional['QAPIDoc']) -> None: if doc and doc.symbol: @@ -784,7 +794,7 @@ class QAPIDoc: % feature.name) self.features[feature.name].connect(feature) - def check_expr(self, expr: TopLevelExpr) -> None: + def check_expr(self, expr: QAPIExpression) -> None: if self.has_section('Returns') and 'command' not in expr: raise QAPISemError(self.info, "'Returns:' is only valid for commands") -- cgit 1.4.1 From 67a81f9fb78d73398051fed0df7a0aac5b61b7c5 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 14 Feb 2023 19:00:10 -0500 Subject: qapi: remove _JSONObject We can remove this alias as it only has two usages now, and no longer pays for the confusion of "yet another type". Signed-off-by: John Snow Message-Id: <20230215000011.1725012-6-jsnow@redhat.com> Reviewed-by: Markus Armbruster --- scripts/qapi/expr.py | 13 +++---------- scripts/qapi/parser.py | 5 ++--- 2 files changed, 5 insertions(+), 13 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index b8f905543e..ca01ea6f4a 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -47,14 +47,6 @@ from .parser import QAPIExpression from .source import QAPISourceInfo -# Deserialized JSON objects as returned by the parser. -# The values of this mapping are not necessary to exhaustively type -# here (and also not practical as long as mypy lacks recursive -# types), because the purpose of this module is to interrogate that -# type. -_JSONObject = Dict[str, object] - - # See check_name_str(), below. valid_name = re.compile(r'(__[a-z0-9.-]+_)?' r'(x-)?' @@ -191,7 +183,7 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None: info, "%s name should not end in 'List'" % meta) -def check_keys(value: _JSONObject, +def check_keys(value: Dict[str, object], info: QAPISourceInfo, source: str, required: List[str], @@ -255,7 +247,8 @@ def check_flags(expr: QAPIExpression) -> None: expr.info, "flags 'allow-oob' and 'coroutine' are incompatible") -def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None: +def check_if(expr: Dict[str, object], + info: QAPISourceInfo, source: str) -> None: """ Validate the ``if`` member of an object. diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 50906e27d4..d570086e1a 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -42,9 +42,8 @@ if TYPE_CHECKING: _ExprValue = Union[List[object], Dict[str, object], str, bool] -# FIXME: Consolidate and centralize definitions for _ExprValue, -# JSONValue, and _JSONObject; currently scattered across several -# modules. +# FIXME: Consolidate and centralize definitions for _ExprValue and +# JSONValue; currently scattered across several modules. class QAPIExpression(Dict[str, object]): -- cgit 1.4.1 From c7b7a7ded9376ad936cfedd843752789ca61bc3b Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 14 Feb 2023 19:00:11 -0500 Subject: qapi: remove JSON value FIXME With the two major JSON-ish type hierarchies clarified for distinct purposes; QAPIExpression for parsed expressions and JSONValue for introspection data, remove this FIXME as no longer an action item. A third JSON-y data type, _ExprValue, is not meant to represent JSON in the abstract but rather only the possible legal return values from a single function, get_expr(). It isn't appropriate to attempt to merge it with either of the above two types. In theory, it may be possible to define a completely agnostic one-size-fits-all JSON type hierarchy that any other user could borrow - in practice, it's tough to wrangle the differences between invariant, covariant and contravariant types: input and output parameters demand different properties of such a structure. However, QAPIExpression serves to authoritatively type user input to the QAPI parser, while JSONValue serves to authoritatively type qapi generator *output* to be served back to client users at runtime via QMP. The AST for these two types are different and cannot be wholly merged into a unified syntax. They could, in theory, share some JSON primitive definitions. In practice, this is currently more trouble than it's worth with mypy's current expressive power. As such, declare this "done enough for now". Signed-off-by: John Snow Message-Id: <20230215000011.1725012-7-jsnow@redhat.com> Reviewed-by: Markus Armbruster --- scripts/qapi/parser.py | 4 ---- 1 file changed, 4 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index d570086e1a..878f90b458 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -42,10 +42,6 @@ if TYPE_CHECKING: _ExprValue = Union[List[object], Dict[str, object], str, bool] -# FIXME: Consolidate and centralize definitions for _ExprValue and -# JSONValue; currently scattered across several modules. - - class QAPIExpression(Dict[str, object]): # pylint: disable=too-few-public-methods def __init__(self, -- cgit 1.4.1