[PATCH 19/19] qapi/schema: refactor entity lookup helpers

[John Snow <jsnow@redhat.com>]

This is not a clear win, but I was confused and I couldn't help myself.

Before:

lookup_entity(self, name: str, typ: Optional[type] = None
              ) -> Optional[QAPISchemaEntity]: ...

lookup_type(self, name: str) -> Optional[QAPISchemaType]: ...

resolve_type(self, name: str, info: Optional[QAPISourceInfo],
             what: Union[str, Callable[[Optional[QAPISourceInfo]], str]]
             ) -> QAPISchemaType: ...

After:

get_entity(self, name: str) -> Optional[QAPISchemaEntity]: ...
get_typed_entity(self, name: str, typ: Type[_EntityType]
                 ) -> Optional[_EntityType]: ...
lookup_type(self, name: str) -> QAPISchemaType: ...
resolve_type(self, name: str, info: Optional[QAPISourceInfo],
             what: Union[str, Callable[[Optional[QAPISourceInfo]], str]]
             ) -> QAPISchemaType: ...

In essence, any function that can return a None value becomes "get ..."
to encourage association with the dict.get() function which has the same
behavior. Any function named "lookup" or "resolve" by contrast is no
longer allowed to return a None value.

This means that any callers to resolve_type or lookup_type don't have to
check that the function worked, they can just assume it did.

Callers to resolve_type will be greeted with a QAPISemError if something
has gone wrong, as they have in the past. Callers to lookup_type will be
greeted with a KeyError if the entity does not exist, or a TypeError if
it does, but is the wrong type.

get_entity and get_typed_entity remain for any callers who are
specifically interested in the negative case. These functions have only
a single caller each.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapidoc.py     |  2 +-
 scripts/qapi/introspect.py |  8 ++----
 scripts/qapi/schema.py     | 52 ++++++++++++++++++++++++--------------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 8f3b9997a15..96deadbf7fc 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -508,7 +508,7 @@ def run(self):
             vis.visit_begin(schema)
             for doc in schema.docs:
                 if doc.symbol:
-                    vis.symbol(doc, schema.lookup_entity(doc.symbol))
+                    vis.symbol(doc, schema.get_entity(doc.symbol))
                 else:
                     vis.freeform(doc)
             return vis.get_document_nodes()
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 42981bce163..67c7d89aae0 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -227,14 +227,10 @@ def _use_type(self, typ: QAPISchemaType) -> str:
 
         # Map the various integer types to plain int
         if typ.json_type() == 'int':
-            tmp = self._schema.lookup_type('int')
-            assert tmp is not None
-            typ = tmp
+            typ = self._schema.lookup_type('int')
         elif (isinstance(typ, QAPISchemaArrayType) and
               typ.element_type.json_type() == 'int'):
-            tmp = self._schema.lookup_type('intList')
-            assert tmp is not None
-            typ = tmp
+            typ = self._schema.lookup_type('intList')
         # Add type to work queue if new
         if typ not in self._used_types:
             self._used_types.append(typ)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index b5f377e68b8..5813136e78b 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -26,6 +26,8 @@
     Dict,
     List,
     Optional,
+    Type,
+    TypeVar,
     Union,
     cast,
 )
@@ -767,7 +769,6 @@ def check(
             # 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():
                 base = "base type '%s'" % self.tag_member.defined_in
             if not isinstance(self.tag_member.type, QAPISchemaEnumType):
@@ -1111,6 +1112,12 @@ def visit(self, visitor: QAPISchemaVisitor) -> None:
             self.arg_type, self.boxed)
 
 
+# Used for type-dependent type lookup return values.
+_EntityType = TypeVar(   # pylint: disable=invalid-name
+    '_EntityType', bound=QAPISchemaEntity
+)
+
+
 class QAPISchema:
     def __init__(self, fname: str):
         self.fname = fname
@@ -1155,22 +1162,28 @@ def _def_entity(self, ent: QAPISchemaEntity) -> None:
                 ent.info, "%s is already defined" % other_ent.describe())
         self._entity_dict[ent.name] = ent
 
-    def lookup_entity(
+    def get_entity(self, name: str) -> Optional[QAPISchemaEntity]:
+        return self._entity_dict.get(name)
+
+    def get_typed_entity(
         self,
         name: str,
-        typ: Optional[type] = None,
-    ) -> Optional[QAPISchemaEntity]:
-        ent = self._entity_dict.get(name)
-        if typ and not isinstance(ent, typ):
-            return None
+        typ: Type[_EntityType]
+    ) -> Optional[_EntityType]:
+        ent = self.get_entity(name)
+        if ent is not None and not isinstance(ent, typ):
+            etype = type(ent).__name__
+            ttype = typ.__name__
+            raise TypeError(
+                f"Entity '{name}' is of type '{etype}', not '{ttype}'."
+            )
         return ent
 
-    def lookup_type(self, name: str) -> Optional[QAPISchemaType]:
-        typ = self.lookup_entity(name, QAPISchemaType)
-        if typ is None:
-            return None
-        assert isinstance(typ, QAPISchemaType)
-        return typ
+    def lookup_type(self, name: str) -> QAPISchemaType:
+        ent = self.get_typed_entity(name, QAPISchemaType)
+        if ent is None:
+            raise KeyError(f"Entity '{name}' is not defined.")
+        return ent
 
     def resolve_type(
         self,
@@ -1178,13 +1191,14 @@ def resolve_type(
         info: Optional[QAPISourceInfo],
         what: Union[str, Callable[[Optional[QAPISourceInfo]], str]],
     ) -> QAPISchemaType:
-        typ = self.lookup_type(name)
-        if not typ:
+        try:
+            return self.lookup_type(name)
+        except (KeyError, TypeError) as err:
             if callable(what):
                 what = what(info)
             raise QAPISemError(
-                info, "%s uses unknown type '%s'" % (what, name))
-        return typ
+                info, "%s uses unknown type '%s'" % (what, name)
+            ) from err
 
     def _module_name(self, fname: str) -> str:
         if QAPISchemaModule.is_system_module(fname):
@@ -1279,7 +1293,7 @@ def _make_array_type(
         self, element_type: str, info: Optional[QAPISourceInfo]
     ) -> str:
         name = element_type + 'List'    # reserved by check_defn_name_str()
-        if not self.lookup_type(name):
+        if not self.get_entity(name):
             self._def_entity(QAPISchemaArrayType(name, info, element_type))
         return name
 
@@ -1295,7 +1309,7 @@ def _make_implicit_object_type(
             return None
         # See also QAPISchemaObjectTypeMember.describe()
         name = 'q_obj_%s-%s' % (name, role)
-        typ = self.lookup_entity(name, QAPISchemaObjectType)
+        typ = self.get_typed_entity(name, QAPISchemaObjectType)
         if typ:
             # The implicit object type has multiple users.  This can
             # only be a duplicate definition, which will be flagged
-- 
2.41.0