[v2,3/7] qapi/expr: Use TopLevelExpr where appropriate

Commit Message

John Snow Feb. 8, 2023, 2:13 a.m. UTC

Remove most usages of _JSONObject with a more semantically meaningful
alias. Note that this is only a semantic alias; the distinction is not
enforced by the type system. This is merely a benefit for the human:
instead of check_xyz functions operating on a representation of some
"JSON Object", we can document them as operating on QAPI's Top Level
Expressions directly; it's more semantically meaningful.

Signed-off-by: John Snow <[e-mail]>
---
 scripts/qapi/expr.py | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Markus Armbruster Feb. 8, 2023, 4:22 p.m. UTC | #1

John Snow <[e-mail]> writes:

> Remove most usages of _JSONObject with a more semantically meaningful
> alias. Note that this is only a semantic alias; the distinction is not
> enforced by the type system. This is merely a benefit for the human:
> instead of check_xyz functions operating on a representation of some
> "JSON Object", we can document them as operating on QAPI's Top Level
> Expressions directly; it's more semantically meaningful.
>
> Signed-off-by: John Snow <[e-mail]>
> ---
>  scripts/qapi/expr.py | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 293f830fe9d..338c9ea4131 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -44,7 +44,7 @@
>  
>  from .common import c_name
>  from .error import QAPISemError
> -from .parser import ParsedExpression
> +from .parser import ParsedExpression, TopLevelExpr
>  from .source import QAPISourceInfo
>  
>  
> @@ -229,11 +229,11 @@ def pprint(elems: Iterable[str]) -> str:
>                 pprint(unknown), pprint(allowed)))
>  
>  
> -def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
> +def check_flags(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
>      """
>      Ensure flag members (if present) have valid values.
>  
> -    :param expr: The expression to validate.
> +    :param expr: The `TopLevelExpr` to validate.
>      :param info: QAPI schema source file information.
>  
>      :raise QAPISemError:
> @@ -447,9 +447,9 @@ def check_features(features: Optional[object],
>          check_if(feat, info, source)
>  
>  
> -def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
> +def check_enum(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
>      """
> -    Normalize and validate this expression as an ``enum`` definition.
> +    Normalize and validate this `TopLevelExpr` as an ``enum`` definition.

Doc comment style question raised in review of PATCH 2 applies.

More of the same below.

>  
>      :param expr: The expression to validate.
>      :param info: QAPI schema source file information.

[...]

Patch

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 293f830fe9d..338c9ea4131 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -44,7 +44,7 @@ 
 
 from .common import c_name
 from .error import QAPISemError
-from .parser import ParsedExpression
+from .parser import ParsedExpression, TopLevelExpr
 from .source import QAPISourceInfo
 
 
@@ -229,11 +229,11 @@  def pprint(elems: Iterable[str]) -> str:
                pprint(unknown), pprint(allowed)))
 
 
-def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_flags(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
     Ensure flag members (if present) have valid values.
 
-    :param expr: The expression to validate.
+    :param expr: The `TopLevelExpr` to validate.
     :param info: QAPI schema source file information.
 
     :raise QAPISemError:
@@ -447,9 +447,9 @@  def check_features(features: Optional[object],
         check_if(feat, info, source)
 
 
-def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_enum(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as an ``enum`` definition.
+    Normalize and validate this `TopLevelExpr` as an ``enum`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.
@@ -486,9 +486,9 @@  def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_features(member.get('features'), info)
 
 
-def check_struct(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_struct(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as a ``struct`` definition.
+    Normalize and validate this `TopLevelExpr` as a ``struct`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.
@@ -503,9 +503,9 @@  def check_struct(expr: _JSONObject, info: QAPISourceInfo) -> None:
     check_type(expr.get('base'), info, "'base'")
 
 
-def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_union(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as a ``union`` definition.
+    Normalize and validate this `TopLevelExpr` as a ``union`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.
@@ -531,9 +531,9 @@  def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_type(value['type'], info, source, allow_array=not base)
 
 
-def check_alternate(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_alternate(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as an ``alternate`` definition.
+    Normalize and validate this `TopLevelExpr` as an ``alternate`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.
@@ -557,9 +557,9 @@  def check_alternate(expr: _JSONObject, info: QAPISourceInfo) -> None:
         check_type(value['type'], info, source, allow_array=True)
 
 
-def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_command(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as a ``command`` definition.
+    Normalize and validate this `TopLevelExpr` as a ``command`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.
@@ -577,9 +577,9 @@  def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
     check_type(rets, info, "'returns'", allow_array=True)
 
 
-def check_event(expr: _JSONObject, info: QAPISourceInfo) -> None:
+def check_event(expr: TopLevelExpr, info: QAPISourceInfo) -> None:
     """
-    Normalize and validate this expression as an ``event`` definition.
+    Normalize and validate this `TopLevelExpr` as an ``event`` definition.
 
     :param expr: The expression to validate.
     :param info: QAPI schema source file information.