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]
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
We currently call QAPIDoc.check() only for definition documentation.
Calling it for free-form documentation as well is simpler. No change,
because it doesn't actually do anything there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20240216145841.2099240-13-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If printing a QAPI schema object for debugging we get the classname and
a hex value for the instance:
<qapi.schema.QAPISchemaEnumType object at 0x7f0ab4c2dad0>
<qapi.schema.QAPISchemaObjectType object at 0x7f0ab4c2dd90>
<qapi.schema.QAPISchemaArrayType object at 0x7f0ab4c2df90>
With this change we instead get the classname and the human friendly
name of the QAPI type instance:
<QAPISchemaEnumType:CpuS390State at 0x7f0ab4c2dad0>
<QAPISchemaObjectType:CpuInfoS390 at 0x7f0ab4c2dd90>
<QAPISchemaArrayType:CpuInfoFastList at 0x7f0ab4c2df90>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20231018120500.2028642-1-berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Conditional swapped to avoid negation]
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[Tweaked to mollify pylint]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Error messages describe object members, enumeration values, features,
and variants like ROLE 'NAME', where ROLE is "member", "value",
"feature", or "branch", respectively. When the member is defined in
another type, e.g. inherited from a base type, we add "of type
'TYPE'". Example: test case struct-base-clash-deep reports a member
of type 'Sub' clashing with a member of its base type 'Base' as
struct-base-clash-deep.json: In struct 'Sub':
struct-base-clash-deep.json:10: member 'name' collides with member 'name' of type 'Base'
Members of implicitly defined types need special treatment. We don't
want to add "of type 'TYPE'" for them, because their named are made up
and mean nothing to the user. Instead, we describe members of an
implicitly defined base type as "base member 'NAME'", and command and
event parameters as "parameter 'NAME'". Example: test case
union-bad-base reports member of a variant's type clashing with a
member of its implicitly defined base type as
union-bad-base.json: In union 'TestUnion':
union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string'
The next commit will permit unions as variant types. "base member
'NAME' would then be ambigious: is it the union's base, or is it the
union's variant's base? One of its test cases would report a clash
between two such bases as "base member 'type' collides with base
member 'type'". Confusing.
Refine the special treatment: add "of TYPE" even for implicitly
defined types, but massage TYPE and ROLE so they make sense for the
user.
Message-Id: <20230420102619.348173-3-berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The C code generator fails to honor 'if' conditions of command and
event arguments.
For instance, tests/qapi-schema/qapi-schema-test.json has
{ 'event': 'TEST_IF_EVENT',
'data': { 'foo': 'TestIfStruct',
'bar': { 'type': ['str'], 'if': 'TEST_IF_EVT_ARG' } },
'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
Generated tests/test-qapi-events.h fails to honor the TEST_IF_EVT_ARG
condition:
#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
void qapi_event_send_test_if_event(TestIfStruct *foo, strList *bar);
#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */
Only uses so far are in tests/.
We could fix the generator to emit something like
#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
void qapi_event_send_test_if_event(TestIfStruct *foo
#if defined(TEST_IF_EVT_ARG)
, strList *bar
#endif
);
#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */
Ugly. Calls become similarly ugly. Not worth fixing.
Conditional arguments work fine with 'boxed': true, simply because
complex types with conditional members work fine. Not worth breaking.
Reject conditional arguments unless boxed.
Move the tests cases covering unboxed conditional arguments out of
tests/qapi-schema/qapi-schema-test.json. Cover boxed conditional
arguments there instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230316071325.492471-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
