Skip to content

Commit

Permalink
qapi/schema: fix typing for QAPISchemaVariants.tag_member
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jnsnow authored and Markus Armbruster committed Apr 24, 2024
1 parent 9beda22 commit 583f4d6
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions scripts/qapi/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,25 +627,39 @@ def __init__(self, tag_name, info, tag_member, variants):
assert isinstance(v, QAPISchemaVariant)
self._tag_name = tag_name
self.info = info
self.tag_member = tag_member
self._tag_member: Optional[QAPISchemaObjectTypeMember] = tag_member
self.variants = variants

@property
def tag_member(self) -> 'QAPISchemaObjectTypeMember':
if self._tag_member is None:
raise RuntimeError(
"QAPISchemaVariants has no tag_member property until "
"after check() has been run."
)
return self._tag_member

def set_defined_in(self, name):
for v in self.variants:
v.set_defined_in(name)

def check(self, schema, seen):
if self._tag_name: # union
self.tag_member = seen.get(c_name(self._tag_name))
# We need to narrow the member type:
tmp = seen.get(c_name(self._tag_name))
assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember)
self._tag_member = tmp

base = "'base'"
# Pointing to the base type when not implicit would be
# nice, but we don't know it here
if not self.tag_member or self._tag_name != self.tag_member.name:
if not self._tag_member or self._tag_name != self._tag_member.name:
raise QAPISemError(
self.info,
"discriminator '%s' is not a member of %s"
% (self._tag_name, base))
# Here we do:
assert self.tag_member.defined_in
base_type = schema.lookup_type(self.tag_member.defined_in)
assert base_type
if not base_type.is_implicit():
Expand All @@ -666,11 +680,13 @@ def check(self, schema, seen):
"discriminator member '%s' of %s must not be conditional"
% (self._tag_name, base))
else: # alternate
assert self._tag_member
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
assert not self.tag_member.optional
assert not self.tag_member.ifcond.is_present()
if self._tag_name: # union
# branches that are not explicitly covered get an empty type
assert self.tag_member.defined_in
cases = {v.name for v in self.variants}
for m in self.tag_member.type.members:
if m.name not in cases:
Expand Down

0 comments on commit 583f4d6

Please sign in to comment.