[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 03/16] qapi/expr.py: constrain incoming expression types
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 03/16] qapi/expr.py: constrain incoming expression types |
Date: |
Wed, 18 Nov 2020 15:58:58 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> mypy does not know the types of values stored in Dicts that masquerade
> as objects. Help the type checker out by constraining the type.
I'm not sure I understand what you mean by "dict masquerading as
object".
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> ---
> scripts/qapi/expr.py | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 5694c501fa38..f7c7f91326ef 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -15,9 +15,17 @@
> # See the COPYING file in the top-level directory.
>
> import re
> +from typing import MutableMapping, Optional
>
> from .common import c_name
> from .error import QAPISemError
> +from .parser import QAPIDoc
> +from .source import QAPISourceInfo
> +
> +
> +# Expressions in their raw form are JSON-like structures with arbitrary
> forms.
> +# Minimally, their top-level form must be a mapping of strings to values.
PEP 8: "For flowing long blocks of text with fewer structural
restrictions (docstrings or comments), the line length should be limited
to 72 characters."
> +Expression = MutableMapping[str, object]
>
>
> # Names must be letters, numbers, -, and _. They must start with letter,
> @@ -287,9 +295,20 @@ def check_event(expr, info):
>
> def check_exprs(exprs):
> for expr_elem in exprs:
> - expr = expr_elem['expr']
> - info = expr_elem['info']
> - doc = expr_elem.get('doc')
dict is arguably a rather awkward choice here. I figure a named tuple
would be neater. A class feels like overkill. Observation, not demand.
> + # Expression
> + assert isinstance(expr_elem['expr'], dict)
> + expr: Expression = expr_elem['expr']
> + for key in expr.keys():
> + assert isinstance(key, str)
> +
> + # QAPISourceInfo
> + assert isinstance(expr_elem['info'], QAPISourceInfo)
> + info: QAPISourceInfo = expr_elem['info']
> +
> + # Optional[QAPIDoc]
> + tmp = expr_elem.get('doc')
> + assert tmp is None or isinstance(tmp, QAPIDoc)
> + doc: Optional[QAPIDoc] = tmp
>
> if 'include' in expr:
> continue
I hope further typing work will reduce the isinstance() assertions
again.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 03/16] qapi/expr.py: constrain incoming expression types,
Markus Armbruster <=