Merge pull request #83 from ISISComputingGroup/Ticket_8527_Fix_Ruff_Pyright

Ticket 8527 fix ruff pyright

Author: LowriJenkins

check_version.py

@@ -3,7 +3,7 @@
 import upgrade
 
 
-def compare_version_number(version_to_check):
+def compare_version_number(version_to_check: str) -> int:
     latest_version, _ = upgrade.UPGRADE_STEPS[-1]
 
     if latest_version == version_to_check:

requirements.txt

@@ -1,2 +1,4 @@
 gitpython
 mysql-connector-python
+PyHamcrest
+

src/common_upgrades/add_to_base_iocs.py

@@ -1,7 +1,10 @@
 from xml.dom import minidom
 from xml.parsers.expat import ExpatError
 
-IOC_FILENAME = "configurations\components\_base\iocs.xml"
+from src.file_access import FileAccess
+from src.local_logger import LocalLogger
+
+IOC_FILENAME = r"configurations\components\_base\iocs.xml"
 
 FILE_TO_CHECK_STR = "IOC default component file"
 ALREADY_CONTAINS = "{} already contains {} ioc."
@@ -12,12 +15,14 @@
 class AddToBaseIOCs:
     """Add the ioc autostart to _base ioc so that it autostarts"""
 
-    def __init__(self, ioc_to_add, add_after_ioc, xml_to_add):
+    def __init__(
+        self, ioc_to_add: str | None, add_after_ioc: str | None, xml_to_add: str | None
+    ) -> None:
         self._ioc_to_add = ioc_to_add
         self._add_after_ioc = add_after_ioc
         self._xml_to_add = xml_to_add
 
-    def perform(self, file_access, logger):
+    def perform(self, file_access: FileAccess, logger: LocalLogger) -> int:
         """Add the autostart of the given.
 
         Args:
@@ -54,7 +59,7 @@ def perform(self, file_access, logger):
         return 0
 
     @staticmethod
-    def _get_ioc_names(xml):
+    def _get_ioc_names(xml: minidom.Document) -> list[str]:
         """Gets the names of all the iocs in the xml.
 
         Args:
@@ -65,7 +70,9 @@ def _get_ioc_names(xml):
         """
         return [ioc.getAttribute("name") for ioc in xml.getElementsByTagName("ioc")]
 
-    def _check_final_file_contains_one_of_added_ioc(self, logger, xml):
+    def _check_final_file_contains_one_of_added_ioc(
+        self, logger: LocalLogger, xml: minidom.Document
+    ) -> bool:
         """Check the file to make sure it now contains one and only one ioc added entry.
 
         Args:
@@ -77,14 +84,15 @@ def _check_final_file_contains_one_of_added_ioc(self, logger, xml):
         """
         ioc_names = AddToBaseIOCs._get_ioc_names(xml)
 
+        assert self._ioc_to_add is not None
         node_count = ioc_names.count(self._ioc_to_add)
         if node_count != 1:
             # I can not see how to generate this error but it is here because it is important
             logger.error(INCORRECT_ADDING.format(FILE_TO_CHECK_STR, node_count, self._ioc_to_add))
             return False
         return True
 
-    def _check_prerequistes_for_file(self, xml, logger):
+    def _check_prerequistes_for_file(self, xml: minidom.Document, logger: LocalLogger) -> bool:
         """Check the file can be modified.
 
         Args:
@@ -95,11 +103,11 @@ def _check_prerequistes_for_file(self, xml, logger):
             True if everything is ok, else False.
         """
         ioc_names = AddToBaseIOCs._get_ioc_names(xml)
-
+        assert self._ioc_to_add is not None
         if ioc_names.count(self._ioc_to_add) != 0:
             logger.error(ALREADY_CONTAINS.format(FILE_TO_CHECK_STR, self._ioc_to_add))
             return False
-
+        assert self._add_after_ioc is not None
         node_count = ioc_names.count(self._add_after_ioc)
         if node_count != 1:
             logger.error(
@@ -108,7 +116,7 @@ def _check_prerequistes_for_file(self, xml, logger):
             return False
         return True
 
-    def _add_ioc(self, ioc_xml, logger):
+    def _add_ioc(self, ioc_xml: minidom.Document, logger: LocalLogger) -> minidom.Document:
         """Add IOC entry after add after ioc specified if it exists.
 
         Args:
@@ -119,7 +127,9 @@ def _add_ioc(self, ioc_xml, logger):
         """
         for ioc in ioc_xml.getElementsByTagName("ioc"):
             if ioc.getAttribute("name") == self._add_after_ioc:
+                assert self._xml_to_add is not None
                 new_ioc_node = minidom.parseString(self._xml_to_add).firstChild
+                assert ioc_xml.firstChild is not None
                 ioc_xml.firstChild.insertBefore(new_ioc_node, ioc.nextSibling)
                 # add some formatting to make it look nice
                 ioc_xml.firstChild.insertBefore(ioc_xml.createTextNode("\n    "), new_ioc_node)

src/common_upgrades/change_macro_in_globals.py

@@ -1,12 +1,16 @@
 import re
+from typing import Generator
 
 from src.common_upgrades.utils.constants import GLOBALS_FILENAME
+from src.common_upgrades.utils.macro import Macro
+from src.file_access import FileAccess
+from src.local_logger import LocalLogger
 
 
 class ChangeMacroInGlobals(object):
     """An interface to replace arbitrary macros in a globals.txt file"""
 
-    def __init__(self, file_access, logger):
+    def __init__(self, file_access: FileAccess, logger: LocalLogger) -> None:
         """Initialise.
 
         Args:
@@ -17,7 +21,7 @@ def __init__(self, file_access, logger):
         self._logger = logger
         self._loaded_file = self.load_globals_file()
 
-    def load_globals_file(self):
+    def load_globals_file(self) -> list:
         """Loads in a globals file as a list of strings.
 
         Returns:
@@ -29,7 +33,7 @@ def load_globals_file(self):
         else:
             return []
 
-    def change_macros(self, ioc_name, macros_to_change):
+    def change_macros(self, ioc_name: str, macros_to_change: list[tuple[Macro, Macro]]) -> None:
         """Changes a list of macros in the globals.txt file for a specific IOC.
 
         Args:
@@ -46,7 +50,7 @@ def change_macros(self, ioc_name, macros_to_change):
 
         self.write_modified_globals_file()
 
-    def change_ioc_name(self, old_ioc_name, new_ioc_name):
+    def change_ioc_name(self, old_ioc_name: str, new_ioc_name: str) -> None:
         """Changes the name of an IOC in a globals.txt file.
 
         Args:
@@ -62,7 +66,7 @@ def change_ioc_name(self, old_ioc_name, new_ioc_name):
 
         self.write_modified_globals_file()
 
-    def _globals_filter_generator(self, ioc_to_change):
+    def _globals_filter_generator(self, ioc_to_change: str) -> Generator[int, None, None]:
         """Returns lines containing specified IOCs from globals.txt
 
         Generator that gives all the lines for a given IOC in globals.txt.
@@ -80,7 +84,7 @@ def _globals_filter_generator(self, ioc_to_change):
                 self._logger.info("Found line '{}' in {}".format(line, GLOBALS_FILENAME))
                 yield index
 
-    def _determine_replacement_values(self, old_macro, new_macro):
+    def _determine_replacement_values(self, old_macro: Macro, new_macro: Macro) -> dict[str, str]:
         """Determines the strings to search for and replace.
 
         Args:
@@ -110,7 +114,9 @@ def _determine_replacement_values(self, old_macro, new_macro):
 
         return regex_changes
 
-    def _apply_regex_macro_change(self, ioc_name, old_macro, new_macro, line_number):
+    def _apply_regex_macro_change(
+        self, ioc_name: str, old_macro: Macro, new_macro: Macro, line_number: int
+    ) -> None:
         """Applies a regular expression to modify a macro.
 
         Args:
@@ -135,7 +141,7 @@ def _apply_regex_macro_change(self, ioc_name, old_macro, new_macro, line_number)
             self._loaded_file[line_number],
         )
 
-    def _change_ioc_name(self, ioc_name, new_ioc_name, line_number):
+    def _change_ioc_name(self, ioc_name: str, new_ioc_name: str, line_number: int) -> None:
         """If a new name is supplied, changes the name of the IOC
 
         Args:
@@ -150,7 +156,7 @@ def _change_ioc_name(self, ioc_name, new_ioc_name, line_number):
                 ioc_name, new_ioc_name.upper()
             )
 
-    def write_modified_globals_file(self):
+    def write_modified_globals_file(self) -> None:
         """Writes the modified globals file if it has been loaded.
 
         Returns:

src/common_upgrades/change_macros_in_xml.py

@@ -1,10 +1,15 @@
 import re
+from typing import Generator
+from xml.dom.minidom import Document, Element, Text
 from xml.parsers.expat import ExpatError
 
 from src.common_upgrades.utils.constants import FILTER_REGEX, IOC_FILE, SYNOPTIC_FOLDER
+from src.common_upgrades.utils.macro import Macro
+from src.file_access import FileAccess
+from src.local_logger import LocalLogger
 
 
-def change_macro_name(macro, old_macro_name, new_macro_name):
+def change_macro_name(macro: Element, old_macro_name: str, new_macro_name: str) -> None:
     """Changes the macro name of a macro xml node.
 
     Args:
@@ -17,7 +22,9 @@ def change_macro_name(macro, old_macro_name, new_macro_name):
         macro.setAttribute("name", new_macro_name)
 
 
-def change_macro_value(macro, old_macro_value, new_macro_value):
+def change_macro_value(
+    macro: Element, old_macro_value: str | None, new_macro_value: str | None
+) -> None:
     """Changes the macros in the given xml if a new macro value is given.
 
     Args:
@@ -33,7 +40,7 @@ def change_macro_value(macro, old_macro_value, new_macro_value):
             macro.setAttribute("value", new_macro_value)
 
 
-def find_macro_with_name(macros, name_to_find):
+def find_macro_with_name(macros: Element, name_to_find: str) -> bool:
     """Find whether macro with name attribute equal to argument name_to_find exists
 
     Args:
@@ -51,7 +58,9 @@ def find_macro_with_name(macros, name_to_find):
 class ChangeMacrosInXML(object):
     """Changes macros in XML files."""
 
-    def __init__(self, file_access, logger):
+    _ioc_file_generator: object
+
+    def __init__(self, file_access: FileAccess, logger: LocalLogger) -> None:
         """Initialise.
 
         Args:
@@ -62,20 +71,27 @@ def __init__(self, file_access, logger):
         self._logger = logger
 
     def add_macro(
-        self, ioc_name, macro_to_add, pattern, description="No description", default_value=None
-    ):
-        """Add a macro with a specified name and value to all IOCs whose name begins with ioc_name, unless a macro
-        with that name already exists
+        self,
+        ioc_name: str,
+        macro_to_add: Macro,
+        pattern: str,
+        description: str = "No description",
+        default_value: str | None = None,
+    ) -> None:
+        """Add a macro with a specified name and value to all IOCs whose name begins with ioc_name,
+         unless a macro with that name already exists
 
         Args:
-            ioc_name: Name of the IOC to add the macro to (e.g. DFKPS would add macros to DFKPS_01 and DFKPS_02)
-            macro_to_add: Macro class with desired name and value
-            pattern: Regex pattern describing what values the macro accepts e.g. "^(0|1)$" for 0 or 1
+            ioc_name: Name of the IOC to add the macro to (e.g. DFKPS would add macros to DFKPS_01
+            and DFKPS_02) macro_to_add: Macro class with desired name and value
+            pattern: Regex pattern describing what values the macro accepts e.g. "^(0|1)$" for 0/1
             description: Description of macro purpose
             default_value: An optional default value for the macro
         Returns:
             None
         """
+        assert macro_to_add.name is not None
+        assert macro_to_add.value is not None
         for path, ioc_xml in self._file_access.get_config_files(IOC_FILE):
             for ioc in self.ioc_tag_generator(path, ioc_xml, ioc_name):
                 macros = ioc.getElementsByTagName("macros")[0]
@@ -88,7 +104,7 @@ def add_macro(
 
             self._file_access.write_xml_file(path, ioc_xml)
 
-    def change_macros(self, ioc_name, macros_to_change):
+    def change_macros(self, ioc_name: str, macros_to_change: list[tuple[Macro, Macro]]) -> None:
         """Changes macros in all xml files that contain the correct macros for a specified ioc.
 
         Args:
@@ -103,15 +119,16 @@ def change_macros(self, ioc_name, macros_to_change):
                 macros = ioc.getElementsByTagName("macros")[0]
                 for macro in macros.getElementsByTagName("macro"):
                     name = macro.getAttribute("name")
-                    for old_macro, new_macro in macros_to_change:
-                        # Check if current macro name starts with name of macro to be changed
-                        if re.match(old_macro.name, name) is not None:
-                            change_macro_name(macro, old_macro.name, new_macro.name)
-                            change_macro_value(macro, old_macro.value, new_macro.value)
+                    if name is not None:
+                        for old_macro, new_macro in macros_to_change:
+                            # Check if current macro name starts with name of macro to be changed
+                            if re.match(old_macro.name, name) is not None:
+                                change_macro_name(macro, old_macro.name, new_macro.name)
+                                change_macro_value(macro, old_macro.value, new_macro.value)
 
             self._file_access.write_xml_file(path, ioc_xml)
 
-    def change_ioc_name(self, old_ioc_name, new_ioc_name):
+    def change_ioc_name(self, old_ioc_name: str, new_ioc_name: str) -> None:
         """Replaces all instances of old_ioc_name with new_ioc_name in an XML tree
         Args:
             old_ioc_name: String, the old ioc prefix (without _XX number suffix)
@@ -131,7 +148,7 @@ def change_ioc_name(self, old_ioc_name, new_ioc_name):
 
             self._file_access.write_xml_file(path, ioc_xml)
 
-    def change_ioc_name_in_synoptics(self, old_ioc_name, new_ioc_name):
+    def change_ioc_name_in_synoptics(self, old_ioc_name: str, new_ioc_name: str) -> None:
         """Replaces instances of old_ioc_name with new_ioc_name
 
         Args:
@@ -155,7 +172,10 @@ def change_ioc_name_in_synoptics(self, old_ioc_name, new_ioc_name):
             for element in synoptic_xml.getElementsByTagName("value"):
                 # Obtain text between the <value> tags (https://stackoverflow.com/a/317494 and https://stackoverflow.com/a/13591742)
                 if element.firstChild is not None:
-                    if element.firstChild.nodeType == element.TEXT_NODE:
+                    if (
+                        isinstance(element.firstChild, Text)
+                        and element.firstChild.nodeType == element.TEXT_NODE
+                    ):
                         ioc_name_with_suffix = element.firstChild.nodeValue
 
                         if old_ioc_name in ioc_name_with_suffix:
@@ -166,7 +186,9 @@ def change_ioc_name_in_synoptics(self, old_ioc_name, new_ioc_name):
 
             self._file_access.write_xml_file(xml_path, synoptic_xml)
 
-    def ioc_tag_generator(self, path, ioc_xml, ioc_to_change):
+    def ioc_tag_generator(
+        self, path: str, ioc_xml: Document, ioc_to_change: str
+    ) -> Generator[Element, None, None]:
         """Generator giving all the IOC tags in all configurations.
 
         Args: