060b5a93 | 15-Mar-2024 |
Markus Armbruster <armbru@redhat.com> |
qapi: Dumb down QAPISchema.lookup_entity()
QAPISchema.lookup_entity() takes an optional type argument, a subtype of QAPISchemaDefinition, and returns that type or None. Callers can use this to save
qapi: Dumb down QAPISchema.lookup_entity()
QAPISchema.lookup_entity() takes an optional type argument, a subtype of QAPISchemaDefinition, and returns that type or None. Callers can use this to save themselves an isinstance() test.
The only remaining user of this convenience feature is .lookup_type(). But we don't actually save anything anymore there: we still need the isinstance() to help mypy over the hump.
Drop the .lookup_entity() argument, and adjust .lookup_type().
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-26-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> [Commit message typo fixed]
show more ...
|
99e75d8c | 15-Mar-2024 |
Markus Armbruster <armbru@redhat.com> |
qapi: Tighten check whether implicit object type already exists
Entities with names starting with q_obj_ are implicit object types. Therefore, QAPISchema._make_implicit_object_type()'s .lookup_entit
qapi: Tighten check whether implicit object type already exists
Entities with names starting with q_obj_ are implicit object types. Therefore, QAPISchema._make_implicit_object_type()'s .lookup_entity() can only return a QAPISchemaObjectType. Assert that.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-25-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: John Snow <jsnow@redhat.com>
show more ...
|
8d413dbd | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: remove unnecessary asserts
With strict typing enabled, these runtime statements aren't necessary anymore; we can prove them statically.
Signed-off-by: John Snow <jsnow@redhat.com> Revi
qapi/schema: remove unnecessary asserts
With strict typing enabled, these runtime statements aren't necessary anymore; we can prove them statically.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-24-armbru@redhat.com>
show more ...
|
aa1fed9f | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: turn on mypy strictness
This patch can be rolled in with the previous one once the series is ready for merge, but for work-in-progress' sake, it's separate here.
Signed-off-by: John Sn
qapi/schema: turn on mypy strictness
This patch can be rolled in with the previous one once the series is ready for merge, but for work-in-progress' sake, it's separate here.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-23-armbru@redhat.com>
show more ...
|
4ed3fe08 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: add type hints
This patch only adds type hints, which aren't utilized at runtime and don't change the behavior of this module in any way.
In a scant few locations, type hints are remov
qapi/schema: add type hints
This patch only adds type hints, which aren't utilized at runtime and don't change the behavior of this module in any way.
In a scant few locations, type hints are removed where no longer necessary due to inference power from typing all of the rest of creation; and any type hints that no longer need string quotes are changed.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-22-armbru@redhat.com>
show more ...
|
d5e2f3d0 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/parser.py: assert member.info is present in connect_member
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbr
qapi/parser.py: assert member.info is present in connect_member
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-21-armbru@redhat.com>
show more ...
|
7c6e4464 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/parser: demote QAPIExpression to Dict[str, Any]
Dict[str, object] is a stricter type, but with the way that code is currently arranged, it is infeasible to enforce this strictness.
In particul
qapi/parser: demote QAPIExpression to Dict[str, Any]
Dict[str, object] is a stricter type, but with the way that code is currently arranged, it is infeasible to enforce this strictness.
In particular, although expr.py's entire raison d'être is normalization and type-checking of QAPI Expressions, that type information is not "remembered" in any meaningful way by mypy because each individual expression is not downcast to a specific expression type that holds all the details of each expression's unique form.
As a result, all of the code in schema.py that deals with actually creating type-safe specialized structures has no guarantee (myopically) that the data it is being passed is correct.
There are two ways to solve this:
(1) Re-assert that the incoming data is in the shape we expect it to be, or (2) Disable type checking for this data.
(1) is appealing to my sense of strictness, but I gotta concede that it is asinine to re-check the shape of a QAPIExpression in schema.py when expr.py has just completed that work at length. The duplication of code and the nightmare thought of needing to update both locations if and when we change the shape of these structures makes me extremely reluctant to go down this route.
(2) allows us the chance to miss updating types in the case that types are updated in expr.py, but it *is* an awful lot simpler and, importantly, gets us closer to type checking schema.py *at all*. Something is better than nothing, I'd argue.
So, do the simpler dumber thing and worry about future strictness improvements later.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-20-armbru@redhat.com>
show more ...
|
7e09dd68 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: assert inner type of QAPISchemaVariants in check_clash()
QAPISchemaVariant's "variants" field is typed as List[QAPISchemaVariant], where the typing for QAPISchemaVariant allows its type
qapi/schema: assert inner type of QAPISchemaVariants in check_clash()
QAPISchemaVariant's "variants" field is typed as List[QAPISchemaVariant], where the typing for QAPISchemaVariant allows its type field to be any QAPISchemaType.
However, QAPISchemaVariant expects that all of its variants contain the narrower QAPISchemaObjectType. This relationship is enforced at runtime in QAPISchemaVariants.check(). This relationship is not embedded in the type system though, so QAPISchemaVariants.check_clash() needs to re-assert this property in order to call QAPISchemaVariant.type.check_clash().
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-19-armbru@redhat.com>
show more ...
|
583f4d6f | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: fix typing for QAPISchemaVariants.tag_member
There are two related changes here:
(1) We need to perform type narrowing for resolving the type of tag_member during check(), and
(2)
qapi/schema: fix typing for QAPISchemaVariants.tag_member
There are two related changes here:
(1) We need to perform type narrowing for resolving the type of tag_member during check(), and
(2) tag_member is a delayed initialization field, but we can hide it behind a property that raises an Exception if it's called too early. This simplifies the typing in quite a few places and avoids needing to assert that the "tag_member is not None" at a dozen callsites, which can be confusing and suggest the wrong thing to a drive-by contributor.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-18-armbru@redhat.com>
show more ...
|
9beda22d | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: Don't initialize "members" with `None`
Declare, but don't initialize the "members" field with type List[QAPISchemaObjectTypeMember].
This simplifies the typing from what would otherwis
qapi/schema: Don't initialize "members" with `None`
Declare, but don't initialize the "members" field with type List[QAPISchemaObjectTypeMember].
This simplifies the typing from what would otherwise be Optional[List[T]] to merely List[T]. This removes the need to add assertions to several callsites that this value is not None - which it never will be after the delayed initialization in check() anyway.
The type declaration without initialization trick will cause accidental uses of this field prior to full initialization to raise an AttributeError.
(Note that it is valid to have an empty members list, see the internal q_empty object as an example. For this reason, we cannot use the empty list as a replacement test for full initialization and instead rely on the _checked/_check_complete fields.)
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-17-armbru@redhat.com>
show more ...
|
875f6242 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: add _check_complete flag
Instead of using the None value for the members field, use a dedicated flag to detect recursive misconfigurations.
This is intended to assist with subsequent p
qapi/schema: add _check_complete flag
Instead of using the None value for the members field, use a dedicated flag to detect recursive misconfigurations.
This is intended to assist with subsequent patches that seek to remove the "None" value from the members field (which can never hold that value after the final call to check()) in order to simplify the static typing of that field; avoiding the need of assertions littered at many callsites to eliminate the possibility of the None value.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-16-armbru@redhat.com>
show more ...
|
8b9e7fd3 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: assert info is present when necessary
QAPISchemaInfo arguments can often be None because built-in definitions don't have such information. The type hint can only be Optional[QAPISchema
qapi/schema: assert info is present when necessary
QAPISchemaInfo arguments can often be None because built-in definitions don't have such information. The type hint can only be Optional[QAPISchemaInfo] then. But, 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 type hints in a forthcoming commit.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-15-armbru@redhat.com>
show more ...
|
8c91329f | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type
Adjust the expression at the callsite to work around mypy's weak type introspection that believes this expression can resolve to QAP
qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type
Adjust the expression at the callsite to work around mypy's weak type introspection that believes this expression can resolve to QAPISourceInfo; it cannot.
(Fundamentally: self.info only resolves to false in a boolean expression when it is None; therefore this expression may only ever produce Optional[str]. mypy does not know that 'info', when it is a QAPISourceInfo object, cannot ever be false.)
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-14-armbru@redhat.com>
show more ...
|
7191400a | 15-Mar-2024 |
Markus Armbruster <armbru@redhat.com> |
qapi: Assert built-in types exist
QAPISchema.lookup_type('FOO') returns a QAPISchemaType when type 'FOO' exists, else None. It won't return None for built-in types like 'int'.
Since mypy can't see
qapi: Assert built-in types exist
QAPISchema.lookup_type('FOO') returns a QAPISchemaType when type 'FOO' exists, else None. It won't return None for built-in types like 'int'.
Since mypy can't see that, it'll complain that we assign the Optional[QAPISchemaType] returned by .lookup_type() to QAPISchemaType variables.
Add assertions to help it over the hump.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-13-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com>
show more ...
|
802a3e3f | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: assert resolve_type has 'info' and 'what' args on error
resolve_type() is generally used to resolve configuration-provided type names into type objects, and generally requires valid 'in
qapi/schema: assert resolve_type has 'info' and 'what' args on error
resolve_type() is generally used to resolve configuration-provided type names into type objects, and generally requires valid 'info' and 'what' parameters.
In some cases, such as with QAPISchemaArrayType.check(), resolve_type may be used to resolve built-in types and as such will not have an 'info' argument, but also must not fail in this scenario.
Use an assertion to sate mypy that we will indeed have 'info' and 'what' parameters for the error pathway in resolve_type.
Note: there are only three callsites to resolve_type at present where "info" is perceived by mypy to be possibly None:
1) QAPISchemaArrayType.check() 2) QAPISchemaObjectTypeMember.check() 3) QAPISchemaEvent.check()
Of those three, only the first actually ever passes None; the other two are limited by their base class initializers which accept info=None, but neither subclass actually use a None value in practice, currently.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-12-armbru@redhat.com>
show more ...
|
10755a95 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: add type narrowing to lookup_type()
This function is a bit hard to type as-is; mypy needs some assertions to assist with the type narrowing.
Signed-off-by: John Snow <jsnow@redhat.com>
qapi/schema: add type narrowing to lookup_type()
This function is a bit hard to type as-is; mypy needs some assertions to assist with the type narrowing.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-11-armbru@redhat.com>
show more ...
|
9bda6c7d | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: adjust type narrowing for mypy's benefit
We already take care to perform some type narrowing for arg_type and ret_type, but not in a way where mypy can utilize the result once we add ty
qapi/schema: adjust type narrowing for mypy's benefit
We already take care to perform some type narrowing for arg_type and ret_type, but not in a way where mypy can utilize the result once we add type hints, e.g.:
qapi/schema.py:833: error: Incompatible types in assignment (expression has type "QAPISchemaType", variable has type "Optional[QAPISchemaObjectType]") [assignment]
qapi/schema.py:893: error: Incompatible types in assignment (expression has type "QAPISchemaType", variable has type "Optional[QAPISchemaObjectType]") [assignment]
A simple change to use a temporary variable helps the medicine go down.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-10-armbru@redhat.com>
show more ...
|
d150be3d | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: make c_type() and json_type() abstract methods
These methods should always return a str, it's only the default abstract implementation that doesn't. They can be marked "abstract", which
qapi/schema: make c_type() and json_type() abstract methods
These methods should always return a str, it's only the default abstract implementation that doesn't. They can be marked "abstract", which requires subclasses to override the method with the proper return type.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-9-armbru@redhat.com>
show more ...
|
578cd932 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: declare type for QAPISchemaArrayType.element_type
A QAPISchemaArrayType's element type gets resolved only during .check(). We have QAPISchemaArrayType.__init__() initialize self.element
qapi/schema: declare type for QAPISchemaArrayType.element_type
A QAPISchemaArrayType's element type gets resolved only during .check(). We have QAPISchemaArrayType.__init__() initialize self.element_type = None, and .check() assign the actual type. Using .element_type before .check() is wrong, and hopefully crashes due to the value being None. Works.
However, it makes for awkward typing. With .element_type: Optional[QAPISchemaType], mypy is of course unable to see that it's None before .check(), and a QAPISchemaType after. To help it over the hump, we'd have to assert self.element_type is not None before all the (valid) uses. The assertion catches invalid uses, but only at run time; mypy can't flag them.
Instead, declare .element_type in .__init__() as QAPISchemaType *without* initializing it. Using .element_type before .check() now certainly crashes, which is an improvement. Mypy still can't flag invalid uses, but that's okay.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-8-armbru@redhat.com>
show more ...
|
ec103961 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: declare type for QAPISchemaObjectTypeMember.type
A QAPISchemaObjectTypeMember's type gets resolved only during .check(). We have QAPISchemaObjectTypeMember.__init__() initialize self.ty
qapi/schema: declare type for QAPISchemaObjectTypeMember.type
A QAPISchemaObjectTypeMember's type gets resolved only during .check(). We have QAPISchemaObjectTypeMember.__init__() initialize self.type = None, and .check() assign the actual type. Using .type before .check() is wrong, and hopefully crashes due to the value being None. Works.
However, it makes for awkward typing. With .type: Optional[QAPISchemaType], mypy is of course unable to see that it's None before .check(), and a QAPISchemaType after. To help it over the hump, we'd have to assert self.type is not None before all the (valid) uses. The assertion catches invalid uses, but only at run time; mypy can't flag them.
Instead, declare .type in .__init__() as QAPISchemaType *without* initializing it. Using .type before .check() now certainly crashes, which is an improvement. Mypy still can't flag invalid uses, but that's okay.
Addresses typing errors such as these:
qapi/schema.py:657: error: "None" has no attribute "alternate_qtype" [attr-defined] qapi/schema.py:662: error: "None" has no attribute "describe" [attr-defined]
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-7-armbru@redhat.com>
show more ...
|
2418d1c4 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi: create QAPISchemaDefinition
Include entities don't have names, but we generally expect "entities" to have names. Reclassify all entities with names as *definitions*, leaving the nameless inclu
qapi: create QAPISchemaDefinition
Include entities don't have names, but we generally expect "entities" to have names. Reclassify all entities with names as *definitions*, leaving the nameless include entities as QAPISchemaEntity instances.
This is primarily to help simplify typing around expectations of what callers expect for properties of an "entity".
Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-6-armbru@redhat.com>
show more ...
|
ce7fde06 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/schema: add pylint suppressions
With this patch, pylint is happy with the file, so enable it in the configuration.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <a
qapi/schema: add pylint suppressions
With this patch, pylint is happy with the file, so enable it in the configuration.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-5-armbru@redhat.com>
show more ...
|
cebc1881 | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi: sort pylint suppressions
Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Marku
qapi: sort pylint suppressions
Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-4-armbru@redhat.com>
show more ...
|
2daf52df | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/parser: shush up pylint
Shhh!
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <2
qapi/parser: shush up pylint
Shhh!
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-3-armbru@redhat.com>
show more ...
|
4f8f199f | 15-Mar-2024 |
John Snow <jsnow@redhat.com> |
qapi/parser: fix typo - self.returns.info => self.errors.info
Small copy-pasto. The correct info field to use in this conditional block is self.errors.info.
Fixes: 3a025d3d1ffa Signed-off-by: John
qapi/parser: fix typo - self.returns.info => self.errors.info
Small copy-pasto. The correct info field to use in this conditional block is self.errors.info.
Fixes: 3a025d3d1ffa Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240315152301.3621858-2-armbru@redhat.com>
show more ...
|