Fixup type hinting

Author: amykyta3

.github/workflows/build.yml

@@ -47,7 +47,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          python -m pip install pytest
+          python -m pip install -r tests/requirements.txt
           sudo apt update
           sudo apt install libxml2-utils
           tests/schema/1685-2014/download_schema.sh
@@ -82,7 +82,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          python -m pip install pylint
+          python -m pip install -r tests/requirements.txt
 
       - name: Install
         run: |
@@ -104,7 +104,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          python -m pip install mypy
+          python -m pip install -r tests/requirements.txt
 
       - name: Install
         run: |

src/peakrdl_ipxact/__peakrdl__.py

@@ -17,7 +17,7 @@ class Exporter(ExporterSubcommandPlugin):
     short_desc = "Export the register model to IP-XACT"
 
 
-    def add_exporter_arguments(self, arg_group: 'argparse.ArgumentParser') -> None:
+    def add_exporter_arguments(self, arg_group: 'argparse._ActionsContainer') -> None:
         arg_group.add_argument(
             "--vendor",
             default="example.org",
@@ -73,9 +73,9 @@ def is_compatible(self, path: str) -> bool:
         with open(path, "r", encoding="utf-8") as f:
             if re.search(r"<(spirit|ipxact):component\b", f.read()):
                 return True
-        return False # type: ignore [unreachable]
+        return False
 
-    def add_importer_arguments(self, arg_group: 'argparse.ArgumentParser') -> None:
+    def add_importer_arguments(self, arg_group: 'argparse._ActionsContainer') -> None:
         arg_group.add_argument(
             "--remap-state",
             metavar="STATE",

src/peakrdl_ipxact/exporter.py

@@ -30,6 +30,9 @@ def supports_isPresent(self) -> bool:
 
 #===============================================================================
 class IPXACTExporter:
+    msg: 'MessageHandler'
+    doc: minidom.Document
+
     def __init__(self, **kwargs: Any) -> None:
         """
         Constructor for the exporter object.
@@ -51,15 +54,13 @@ def __init__(self, **kwargs: Any) -> None:
             String to use for line breaks. Defaults to a newline (``\\n``).
         """
 
-        self.msg = None # type: MessageHandler
 
         self.vendor = kwargs.pop("vendor", None) or "example.org"
         self.library = kwargs.pop("library", None) or "mylibrary"
         self.version = kwargs.pop("version", None) or "1.0"
         self.standard = kwargs.pop("standard", None) or Standard.IEEE_1685_2014
         self.xml_indent = kwargs.pop("xml_indent", None) or "  "
         self.xml_newline = kwargs.pop("xml_newline", None) or "\n"
-        self.doc = None # type: minidom.Document
         self._max_width = None # type: Optional[int]
 
         # Check for stray kwargs
@@ -222,7 +223,7 @@ def add_nameGroup(self, parent: minidom.Element, name: str, displayName: Optiona
             self.add_value(parent, self.ns + "description", description)
 
     #---------------------------------------------------------------------------
-    def add_registerData(self, parent: minidom.Element, node: RegNode) -> None:
+    def add_registerData(self, parent: minidom.Element, node: AddressableNode) -> None:
         if self.standard == Standard.IEEE_1685_2009:
             # registers must all be listed before register files
             for child in node.children(skip_not_present=self.skip_not_present):
@@ -339,7 +340,7 @@ def add_registerFile(self, parent: minidom.Element, node: Union[RegfileNode, Add
         if self.standard.supports_isPresent and not node.get_property("ispresent"):
             self.add_value(registerFile, self.ns + "isPresent", "0")
 
-        if node.is_array:
+        if node.array_dimensions:
             for dim in node.array_dimensions:
                 self.add_value(registerFile, self.ns + "dim", "%d" % dim)
 
@@ -350,6 +351,7 @@ def add_registerFile(self, parent: minidom.Element, node: Union[RegfileNode, Add
         if node.is_array:
             # For arrays, ipxact:range also defines the increment between indexes
             # Must use stride instead
+            assert node.array_stride is not None
             self.add_value(registerFile, self.ns + "range", self.hex_str(node.array_stride))
         else:
             self.add_value(registerFile, self.ns + "range", self.hex_str(node.size))
@@ -377,7 +379,7 @@ def add_register(self, parent: minidom.Element, node: RegNode) -> None:
         if self.standard.supports_isPresent and not node.get_property("ispresent"):
             self.add_value(register, self.ns + "isPresent", "0")
 
-        if node.is_array:
+        if node.array_dimensions:
             if node.array_stride != (node.get_property("regwidth") / 8):
                 self.msg.fatal(
                     "IP-XACT does not support register arrays whose stride is larger then the register's size",

src/peakrdl_ipxact/importer.py

@@ -1,4 +1,4 @@
-from typing import Optional, List, Dict, Any, Type, Union, Set
+from typing import Optional, List, Dict, Any, Type, Union, Set, TypeVar
 import re
 
 from xml.etree import ElementTree
@@ -10,6 +10,7 @@
 
 from . import typemaps
 
+CT = TypeVar("CT", bound=comp.Component)
 
 # Expected IP-XACT namespaces. This parser is not strict about the exact version.
 VALID_NS_REGEXES = [
@@ -19,8 +20,9 @@
 ]
 
 class IPXACTImporter(RDLImporter):
+    ns: str
 
-    def __init__(self, compiler: RDLCompiler):
+    def __init__(self, compiler: RDLCompiler) -> None:
         """
         Parameters
         ----------
@@ -29,10 +31,9 @@ def __init__(self, compiler: RDLCompiler):
         """
 
         super().__init__(compiler)
-        self.ns = None # type: str
         self._addressUnitBits = 8
         self._current_addressBlock_access = rdltypes.AccessType.rw
-        self.remap_states_seen = set() # type: Set[str]
+        self.remap_states_seen: Set[str] = set()
 
     @property
     def src_ref(self) -> SourceRefBase:
@@ -58,18 +59,23 @@ def import_file(self, path: str, remap_state: Optional[str] = None) -> None:
 
         tree = ElementTree.parse(path)
 
-        component = self.get_component(tree)
+        component = self.get_component(tree) # type: ignore
 
         memoryMaps = self.get_all_memoryMap(component)
 
+        comp_name = self.get_sanitized_element_name(component)
+        if not comp_name:
+            self.msg.fatal("component is missing required tag 'name'", self.src_ref)
+
         for memoryMap in memoryMaps:
-            comp_name = self.get_sanitized_element_name(component)
             self.import_memoryMap(memoryMap, comp_name, remap_state)
 
 
     def get_component(self, tree: ElementTree.ElementTree) -> ElementTree.Element:
         # Find <component> and determine namespace prefix
         root = tree.getroot()
+        assert root is not None
+
         if get_local_name(root) == "component":
             component = root
             namespace = get_namespace(root)
@@ -116,6 +122,8 @@ def get_all_address_blocks(self, memoryMap: ElementTree.Element, remap_state: Op
         memoryRemaps = memoryMap.findall(self.ns+"memoryRemap")
         for memoryRemap in memoryRemaps:
             this_remapState = memoryRemap.get(self.ns+"state")
+            if this_remapState is None:
+                continue
             self.remap_states_seen.add(this_remapState)
             if this_remapState == remap_state:
                 remap_addressBlocks = memoryRemap.findall(self.ns+"addressBlock")
@@ -239,6 +247,8 @@ def parse_addressBlock(self, addressBlock: ElementTree.Element, name_prefix:str)
 
         d = self.flatten_element_values(addressBlock)
         name = self.get_sanitized_element_name(addressBlock)
+        if not name:
+            self.msg.fatal("addressBlock is missing required tag 'name'", self.src_ref)
 
         if d.get('usage', None) == "reserved":
             # 1685-2014 6.9.4.2-a.1.iii: defines the entire range of the
@@ -257,6 +267,7 @@ def parse_addressBlock(self, addressBlock: ElementTree.Element, name_prefix:str)
         # Create named component definition
         is_memory = d.get('usage', None) == "memory"
         type_name = name_prefix + name
+        C_def: Union[comp.Mem, comp.Addrmap]
         if is_memory:
             C_def = self.create_mem_definition(type_name)
         else:
@@ -295,9 +306,9 @@ def parse_addressBlock(self, addressBlock: ElementTree.Element, name_prefix:str)
                 if R:
                     self.add_child(C_def, R)
             elif local_name == "registerFile" and not is_memory:
-                R = self.parse_registerFile(child_el)
-                if R:
-                    self.add_child(C_def, R)
+                RF = self.parse_registerFile(child_el)
+                if RF:
+                    self.add_child(C_def, RF)
             else:
                 self.msg.error(
                     "Invalid child element <%s> found in <%s:addressBlock>"
@@ -324,17 +335,19 @@ def parse_addressBlock(self, addressBlock: ElementTree.Element, name_prefix:str)
         # Also convert to an instance, since this will be instantiated into the
         # wrapper that represents the memoryMap
         if is_memory:
-            C = self.instantiate_mem(
+            assert isinstance(C_def, comp.Mem)
+            mem = self.instantiate_mem(
                 C_def,
                 name, self.AU_to_bytes(d['baseAddress'])
             )
+            return mem
         else:
-            C = self.instantiate_addrmap(
+            assert isinstance(C_def, comp.Addrmap)
+            am = self.instantiate_addrmap(
                 C_def,
                 name, self.AU_to_bytes(d['baseAddress'])
             )
-
-        return C
+            return am
 
 
     def parse_registerFile(self, registerFile: ElementTree.Element) -> Optional[comp.Regfile]:
@@ -361,6 +374,8 @@ def parse_registerFile(self, registerFile: ElementTree.Element) -> Optional[comp
 
         d = self.flatten_element_values(registerFile)
         name = self.get_sanitized_element_name(registerFile)
+        if not name:
+            self.msg.fatal("registerFile is missing required tag 'name'", self.src_ref)
 
         # Check for required values
         required = {'addressOffset', 'range'}
@@ -402,9 +417,9 @@ def parse_registerFile(self, registerFile: ElementTree.Element) -> Optional[comp
                 if R:
                     self.add_child(C, R)
             elif local_name == "registerFile":
-                R = self.parse_registerFile(child_el)
-                if R:
-                    self.add_child(C, R)
+                RF = self.parse_registerFile(child_el)
+                if RF:
+                    self.add_child(C, RF)
             else:
                 self.msg.error(
                     "Invalid child element <%s> found in <%s:registerFile>"
@@ -456,6 +471,8 @@ def parse_register(self, register: ElementTree.Element) -> Optional[comp.Reg]:
 
         d = self.flatten_element_values(register)
         name = self.get_sanitized_element_name(register)
+        if not name:
+            self.msg.fatal("register is missing required tag 'name'", self.src_ref)
 
         # Check for required values
         required = {'addressOffset', 'size'}
@@ -511,6 +528,8 @@ def parse_register(self, register: ElementTree.Element) -> Optional[comp.Reg]:
             if local_name == "field":
                 # This XML element is a field
                 field_name = self.get_sanitized_element_name(child_el)
+                if not field_name:
+                    self.msg.fatal("field is missing required tag 'name'", self.src_ref)
                 field_tuples.append((field_name, child_el))
                 if field_name in field_names:
                     field_name_collisions.add(field_name)
@@ -650,6 +669,7 @@ def parse_field(
             self.assign_property(C, "onwrite", d['modifiedWriteValue'])
 
         if 'enum_el' in d:
+            assert C.inst_name is not None
             enum_type = self.parse_enumeratedValues(d['enum_el'], C.inst_name + "_enum_t")
             self.assign_property(C, "encode", enum_type)
 
@@ -700,7 +720,7 @@ def parse_integer(self, s: str) -> int:
                 v *= multiplier[m.group(2).upper()]
             return v
 
-        m = re.fullmatch(r"\d*'h([0-9a-f]+)", s, re.I) # type: ignore [unreachable]
+        m = re.fullmatch(r"\d*'h([0-9a-f]+)", s, re.I)
         if m:
             return int(m.group(1), 16)
 
@@ -890,6 +910,8 @@ def parse_enumeratedValues(self, enumeratedValues: ElementTree.Element, type_nam
                     d[local_name] = self.parse_integer(get_text(child))
 
             name = self.get_sanitized_element_name(enumeratedValue)
+            if not name:
+                self.msg.fatal("enumeratedValue is missing required tag 'name'", self.src_ref)
 
             # Check for required values
             required = {'value'}
@@ -946,23 +968,23 @@ def AU_to_bytes(self, au: int) -> int:
         return byte_units
 
 
-    def memoryMap_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: comp.Component) -> comp.Component:
+    def memoryMap_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: CT) -> CT:
         #pylint: disable=unused-argument
         return component
 
-    def addressBlock_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: comp.Component) -> comp.Component:
+    def addressBlock_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: CT) -> CT:
         #pylint: disable=unused-argument
         return component
 
-    def registerFile_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: comp.Component) -> comp.Component:
+    def registerFile_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: CT) -> CT:
         #pylint: disable=unused-argument
         return component
 
-    def register_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: comp.Component) -> comp.Component:
+    def register_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: CT) -> CT:
         #pylint: disable=unused-argument
         return component
 
-    def field_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: comp.Component) -> comp.Component:
+    def field_vendorExtensions(self, vendorExtensions: ElementTree.Element, component: CT) -> CT:
         #pylint: disable=unused-argument
         return component
 

src/peakrdl_ipxact/typemaps.py

@@ -1,3 +1,4 @@
+from typing import Optional
 from systemrdl import rdltypes
 
 # sw <--> ipxact:access
@@ -13,9 +14,9 @@ def access_from_sw(sw: rdltypes.AccessType) -> str:
     for sw_entry, access_entry in ACCESS_MAP:
         if sw == sw_entry:
             return access_entry
-    return None
+    raise RuntimeError("Impossible")
 
-def sw_from_access(access: str) -> rdltypes.AccessType:
+def sw_from_access(access: str) -> Optional[rdltypes.AccessType]:
     for sw_entry, access_entry in ACCESS_MAP:
         if access == access_entry:
             return sw_entry
@@ -39,9 +40,9 @@ def mwv_from_onwrite(onwrite: rdltypes.OnWriteType) -> str:
     for onwrite_entry, mwv_entry in MWV_MAP:
         if onwrite == onwrite_entry:
             return mwv_entry
-    return None
+    raise RuntimeError("Impossible")
 
-def onwrite_from_mwv(mwv: str) -> rdltypes.OnWriteType:
+def onwrite_from_mwv(mwv: str) -> Optional[rdltypes.OnWriteType]:
     for onwrite_entry, mwv_entry in MWV_MAP:
         if mwv == mwv_entry:
             return onwrite_entry
@@ -59,9 +60,9 @@ def readaction_from_onread(onread: rdltypes.OnReadType) -> str:
     for onread_entry, read_action_entry in READ_ACTION_MAP:
         if onread == onread_entry:
             return read_action_entry
-    return None
+    raise RuntimeError("Impossible")
 
-def onread_from_readaction(readaction: str) -> rdltypes.OnReadType:
+def onread_from_readaction(readaction: str) -> Optional[rdltypes.OnReadType]:
     for onread_entry, read_action_entry in READ_ACTION_MAP:
         if readaction == read_action_entry:
             return onread_entry

tests/mypy.ini

@@ -1,9 +1,9 @@
 [mypy]
-ignore_missing_imports = True
-strict_optional = False
+#ignore_missing_imports = True
+#strict_optional = False
 disallow_incomplete_defs = True
 disallow_untyped_defs = True
 warn_unused_configs = True
 warn_unused_ignores = True
 warn_unreachable = True
-disallow_untyped_calls = False
+disallow_untyped_calls = True

tests/requirements.txt

@@ -0,0 +1,5 @@
+pytest
+pylint
+mypy
+pytest-cov
+coveralls>=3.0.0