[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/19] qapi/schema: assert info is present when necessary
|
From: |
Markus Armbruster |
|
Subject: |
Re: [PATCH v2 12/19] qapi/schema: assert info is present when necessary |
|
Date: |
Tue, 16 Jan 2024 14:37:00 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) |
John Snow <jsnow@redhat.com> writes:
> QAPISchemaInfo instances are sometimes defined as an Optional
> field/argument because built-in definitions don't *have* a source
> definition. As a consequence, there are a few places where we need to
> assert that it's present because the root entity definition can only
> enforce that it is "Optional".
Well, we need to assert to help mypy over the hump. But that's later in
this series. Just like "enforce that is is 'Optional'". My point is:
the commit message is less than clear unless the reader already knows
where we're going.
Here's my try:
QAPISchemaInfo arguments can often be None because build-in
definitions don't have such information. The type hint can only be
Optional[QAPISchemaInfo] then. But then mypy gets upset about all the
places where we exploit that it can't actually be None there. Add
assertions that will help mypy over the hump, to enable adding the
type hints.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> scripts/qapi/schema.py | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 43af756ed47..eefa58a798b 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -758,6 +758,7 @@ def describe(self, info):
> else:
> assert False
>
> + assert info is not None
> if defined_in != info.defn_name:
> return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in)
> return "%s '%s'" % (role, self.name)
> @@ -848,6 +849,7 @@ def __init__(self, name, info, doc, ifcond, features,
> self.coroutine = coroutine
>
> def check(self, schema):
> + assert self.info is not None
> super().check(schema)
> if self._arg_type_name:
> arg_type = schema.resolve_type(
- [PATCH v2 13/19] qapi/schema: split "checked" field into "checking" and "checked", (continued)
- [PATCH v2 14/19] qapi/schema: fix typing for QAPISchemaVariants.tag_member, John Snow, 2024/01/12
- [PATCH v2 15/19] qapi/schema: assert inner type of QAPISchemaVariants in check_clash(), John Snow, 2024/01/12
- [PATCH v2 12/19] qapi/schema: assert info is present when necessary, John Snow, 2024/01/12
- Re: [PATCH v2 12/19] qapi/schema: assert info is present when necessary,
Markus Armbruster <=
- [PATCH v2 18/19] qapi/schema: turn on mypy strictness, John Snow, 2024/01/12
- [PATCH v2 19/19] qapi/schema: remove unnecessary asserts, John Snow, 2024/01/12
- [PATCH v2 10/19] qapi: use schema.resolve_type instead of schema.lookup_type, John Snow, 2024/01/12
- [PATCH v2 07/19] qapi/schema: adjust type narrowing for mypy's benefit, John Snow, 2024/01/12
- [PATCH v2 16/19] qapi/parser: demote QAPIExpression to Dict[str, Any], John Snow, 2024/01/12
- [PATCH v2 09/19] qapi/schema: allow resolve_type to be used for built-in types, John Snow, 2024/01/12