Mypy type correctness (#3199)

7 files changed, 66 insertions, 49 deletions

diff --git a/lib/elements/clone.py b/lib/elements/clone.py
index 943428b8..9c3e3e3a 100644
--- a/lib/elements/clone.py
+++ b/lib/elements/clone.py
@@ -5,11 +5,11 @@
 
 from contextlib import contextmanager
 from math import degrees
-from typing import Dict, Generator, List
+from typing import Dict, Generator, List, Optional, Tuple, Any, cast
 
-from inkex import BaseElement, Title, Transform
+from inkex import BaseElement, Title, Transform, Vector2d
 from lxml.etree import _Comment
-from shapely import MultiLineString
+from shapely import Geometry, MultiLineString, Point as ShapelyPoint
 
 from ..commands import (find_commands, is_command_symbol,
                         point_command_symbols_up)
@@ -40,12 +40,12 @@ class Clone(EmbroideryElement):
     name = "Clone"
     element_name = _("Clone")
 
-    def __init__(self, *args, **kwargs):
-        super(Clone, self).__init__(*args, **kwargs)
+    def __init__(self, node: BaseElement) -> None:
+        super(Clone, self).__init__(node)
 
     @property
     @param('clone', _("Clone"), type='toggle', inverse=False, default=True)
-    def clone(self):
+    def clone(self) -> bool:
         return self.get_boolean_param("clone", True)
 
     @property
@@ -55,7 +55,7 @@ class Clone(EmbroideryElement):
            unit='deg',
            type='float')
     @cache
-    def clone_fill_angle(self):
+    def clone_fill_angle(self) -> float:
         return self.get_float_param('angle')
 
     @property
@@ -66,15 +66,15 @@ class Clone(EmbroideryElement):
            type='boolean',
            default=False)
     @cache
-    def flip_angle(self):
+    def flip_angle(self) -> bool:
         return self.get_boolean_param('flip_angle', False)
 
-    def get_cache_key_data(self, previous_stitch, next_element):
+    def get_cache_key_data(self, previous_stitch: Any, next_element: EmbroideryElement) -> List[str]:
         source_node = self.node.href
         source_elements = self.clone_to_elements(source_node)
         return [element.get_cache_key(previous_stitch, next_element) for element in source_elements]
 
-    def clone_to_elements(self, node) -> List[EmbroideryElement]:
+    def clone_to_elements(self, node: BaseElement) -> List[EmbroideryElement]:
         # Only used in get_cache_key_data, actual embroidery uses nodes_to_elements+iterate_nodes
         from .utils import node_to_elements
         elements = []
@@ -85,7 +85,7 @@ class Clone(EmbroideryElement):
                 elements.extend(node_to_elements(child, True))
         return elements
 
-    def to_stitch_groups(self, last_stitch_group=None, next_element=None) -> List[StitchGroup]:
+    def to_stitch_groups(self, last_stitch_group: Optional[StitchGroup], next_element: Optional[EmbroideryElement] = None) -> List[StitchGroup]:
         if not self.clone:
             return []
 
@@ -96,7 +96,7 @@ class Clone(EmbroideryElement):
 
             next_elements = [next_element]
             if len(elements) > 1:
-                next_elements = elements[1:] + next_elements
+                next_elements = cast(List[Optional[EmbroideryElement]], elements[1:]) + next_elements
             for element, next_element in zip(elements, next_elements):
                 # Using `embroider` here to get trim/stop after commands, etc.
                 element_stitch_groups = element.embroider(last_stitch_group, next_element)
@@ -107,26 +107,26 @@ class Clone(EmbroideryElement):
             return stitch_groups
 
     @property
-    def first_stitch(self):
+    def first_stitch(self) -> Optional[ShapelyPoint]:
         first, last = self.first_and_last_element()
         if first:
             return first.first_stitch
         return None
 
-    def uses_previous_stitch(self):
+    def uses_previous_stitch(self) -> bool:
         first, last = self.first_and_last_element()
         if first:
             return first.uses_previous_stitch()
-        return None
+        return False
 
-    def uses_next_element(self):
+    def uses_next_element(self) -> bool:
         first, last = self.first_and_last_element()
         if last:
             return last.uses_next_element()
-        return None
+        return False
 
     @cache
-    def first_and_last_element(self):
+    def first_and_last_element(self) -> Tuple[Optional[EmbroideryElement], Optional[EmbroideryElement]]:
         with self.clone_elements() as elements:
             if len(elements):
                 return elements[0], elements[-1]
@@ -153,7 +153,7 @@ class Clone(EmbroideryElement):
             for cloned_node in cloned_nodes:
                 cloned_node.delete()
 
-    def resolve_clone(self, recursive=True) -> List[BaseElement]:
+    def resolve_clone(self, recursive: bool = True) -> List[BaseElement]:
         """
         "Resolve" this clone element by copying the node it hrefs as if unlinking the clone in Inkscape.
         The node will be added as a sibling of this element's node, with its transform and style applied.
@@ -162,9 +162,12 @@ class Clone(EmbroideryElement):
         :param recursive: Recursively "resolve" all child clones in the same manner
         :returns: A list where the first element is the "resolved" node, and zero or more commands attached to that node
         """
-        parent: BaseElement = self.node.getparent()
-        source_node: BaseElement = self.node.href
-        source_parent: BaseElement = source_node.getparent()
+        parent: Optional[BaseElement] = self.node.getparent()
+        assert parent is not None, f"Element {self.node.get_id()} should have a parent"
+        source_node: Optional[BaseElement] = self.node.href
+        assert source_node is not None, f"Target of {self.node.get_id()} was None!"
+        source_parent: Optional[BaseElement] = source_node.getparent()
+        assert source_parent is not None, f"Target {source_node.get_id()} of {self.node.get_id()} should have a parent"
         cloned_node = clone_with_fixup(parent, source_node)
 
         if recursive:
@@ -201,7 +204,7 @@ class Clone(EmbroideryElement):
         if cloned_node.tag == SVG_SYMBOL_TAG:
             source_transform: Transform = parent.composed_transform()
         else:
-            source_transform: Transform = source_parent.composed_transform()
+            source_transform = source_parent.composed_transform()
         clone_transform: Transform = self.node.composed_transform()
         angle_transform = clone_transform @ -source_transform
         self.apply_angles(cloned_node, angle_transform)
@@ -242,7 +245,7 @@ class Clone(EmbroideryElement):
                 # We have to negate the angle because SVG/Inkscape's definition of rotation is clockwise, while Inkstitch uses counter-clockwise
                 fill_vector = (angle_transform @ Transform(f"rotate(${-element_angle})")).apply_to_point((1, 0))
                 # Same reason for negation here.
-                element_angle = -degrees(fill_vector.angle)
+                element_angle = -degrees(fill_vector.angle or 0)  # Fallback to 0 if an insane transform is used.
             else:  # If clone_fill_angle is specified, override the angle instead.
                 element_angle = self.clone_fill_angle
 
@@ -252,26 +255,28 @@ class Clone(EmbroideryElement):
             node.set(INKSTITCH_ATTRIBS['angle'], round(element_angle, 6))
 
     @property
-    def shape(self):
+    def shape(self) -> Geometry:
         path = self.node.get_path()
         transform = Transform(self.node.composed_transform())
         path = path.transform(transform)
         path = path.to_superpath()
         return MultiLineString(path[0])
 
-    def center(self, source_node):
+    def center(self, source_node: BaseElement) -> Vector2d:
         translate = Transform(f"translate({float(self.node.get('x', '0'))}, {float(self.node.get('y', '0'))})")
-        transform = get_node_transform(self.node.getparent()) @ translate
+        parent = self.node.getparent()
+        assert parent is not None, "This should be part of a tree and therefore have a parent"
+        transform = get_node_transform(parent) @ translate
         center = self.node.bounding_box(transform).center
         return center
 
-    def validation_warnings(self):
+    def validation_warnings(self) -> Generator[CloneWarning, Any, None]:
         source_node = self.node.href
         point = self.center(source_node)
         yield CloneWarning(point)
 
 
-def is_clone(node):
+def is_clone(node: BaseElement) -> bool:
     if node.tag == SVG_USE_TAG and node.href is not None and not is_command_symbol(node):
         return True
     return False
@@ -299,7 +304,7 @@ def clone_with_fixup(parent: BaseElement, node: BaseElement) -> BaseElement:
 
     ret = clone_children(parent, node)
 
-    def fixup_id_attr(node: BaseElement, attr: str):
+    def fixup_id_attr(node: BaseElement, attr: str) -> None:
         # Replace the id value for this attrib with the corresponding one in the clone subtree, if applicable.
         val = node.get(attr)
         if val is not None:
diff --git a/lib/elements/element.py b/lib/elements/element.py
index ea2d5d6b..6f2c52e9 100644
--- a/lib/elements/element.py
+++ b/lib/elements/element.py
@@ -2,6 +2,7 @@
 #
 # Copyright (c) 2010 Authors
 # Licensed under the GNU GPL version 3.0 or later.  See the file LICENSE for details.
+from __future__ import annotations
 import json
 import sys
 from contextlib import contextmanager
@@ -79,8 +80,9 @@ class EmbroideryElement(object):
             prop = getattr(cls, attr)
             if isinstance(prop, property):
                 # The 'param' attribute is set by the 'param' decorator defined above.
-                if hasattr(prop.fget, 'param'):
-                    params.append(prop.fget.param)
+                fget = prop.fget
+                if fget is not None and hasattr(fget, 'param'):
+                    params.append(fget.param)
         return params
 
     @cache
@@ -215,18 +217,18 @@ class EmbroideryElement(object):
 
         # First, figure out the translation component of the transform.  Using a zero
         # vector completely cancels out the rotation, scale, and skew components.
-        zero = [0, 0]
+        zero = (0, 0)
         zero = inkex.Transform.apply_to_point(node_transform, zero)
         translate = Point(*zero)
 
         # Next, see how the transform affects unit vectors in the X and Y axes.  We
         # need to subtract off the translation or it will affect the magnitude of
         # the resulting vector, which we don't want.
-        unit_x = [1, 0]
+        unit_x = (1, 0)
         unit_x = inkex.Transform.apply_to_point(node_transform, unit_x)
         sx = (Point(*unit_x) - translate).length()
 
-        unit_y = [0, 1]
+        unit_y = (0, 1)
         unit_y = inkex.Transform.apply_to_point(node_transform, unit_y)
         sy = (Point(*unit_y) - translate).length()
 
@@ -455,7 +457,7 @@ class EmbroideryElement(object):
         raise NotImplementedError("INTERNAL ERROR: %s must implement shape()", self.__class__)
 
     @property
-    def first_stitch(self):
+    def first_stitch(self) -> Optional[ShapelyPoint]:
         # first stitch is an approximation to where the first stitch may possibly be
         # if not defined through commands or repositioned by the previous element
         raise NotImplementedError("INTERNAL ERROR: %s must implement first_stitch()", self.__class__)
@@ -521,7 +523,7 @@ class EmbroideryElement(object):
 
         return lock_start, lock_end
 
-    def to_stitch_groups(self, last_stitch_group: Optional[StitchGroup], next_element: Optional[ShapelyPoint] = None) -> List[StitchGroup]:
+    def to_stitch_groups(self, last_stitch_group: Optional[StitchGroup], next_element: Optional[EmbroideryElement] = None) -> List[StitchGroup]:
         raise NotImplementedError("%s must implement to_stitch_groups()" % self.__class__.__name__)
 
     @debug.time
diff --git a/lib/elements/fill_stitch.py b/lib/elements/fill_stitch.py
index f8db39d5..ee77cd60 100644
--- a/lib/elements/fill_stitch.py
+++ b/lib/elements/fill_stitch.py
@@ -808,13 +808,17 @@ class FillStitch(EmbroideryElement):
     def validation_errors(self):
         if not self.shape.is_valid:
             why = explain_validity(self.shape)
-            message, x, y = re.match(r"(?P<message>.+)\[(?P<x>.+)\s(?P<y>.+)\]", why).groups()
+            match = re.match(r"(?P<message>.+)\[(?P<x>.+)\s(?P<y>.+)\]", why)
+            assert match is not None, f"Could not parse validity message '{why}'"
+            message, x, y = match.groups()
             yield InvalidShapeError((x, y))
 
     def validation_warnings(self):  # noqa: C901
         if not self.original_shape.is_valid:
             why = explain_validity(self.original_shape)
-            message, x, y = re.match(r"(?P<message>.+)\[(?P<x>.+)\s(?P<y>.+)\]", why).groups()
+            match = re.match(r"(?P<message>.+)\[(?P<x>.+)\s(?P<y>.+)\]", why)
+            assert match is not None, f"Could not parse validity message '{why}'"
+            message, x, y = match.groups()
             if "Hole lies outside shell" in message:
                 yield UnconnectedWarning((x, y))
             else:
diff --git a/lib/elements/image.py b/lib/elements/image.py
index 695515dc..ad012975 100644
--- a/lib/elements/image.py
+++ b/lib/elements/image.py
@@ -23,14 +23,16 @@ class ImageObject(EmbroideryElement):
     name = "Image"
 
     def center(self):
-        transform = get_node_transform(self.node.getparent())
+        parent = self.node.getparent()
+        assert parent is not None, "This should be part of a tree and therefore have a parent"
+        transform = get_node_transform(parent)
         center = self.node.bounding_box(transform).center
         return center
 
     def validation_warnings(self):
         yield ImageTypeWarning(self.center())
 
-    def to_stitch_groups(self, last_stitch_group):
+    def to_stitch_groups(self, last_stitch_group, next_element):
         return []
 
     def first_stitch(self):
diff --git a/lib/elements/text.py b/lib/elements/text.py
index dd886dbc..34ce14e2 100644
--- a/lib/elements/text.py
+++ b/lib/elements/text.py
@@ -21,7 +21,9 @@ class TextTypeWarning(ObjectTypeWarning):
 class TextObject(EmbroideryElement):
 
     def pointer(self):
-        transform = get_node_transform(self.node.getparent())
+        parent = self.node.getparent()
+        assert parent is not None, "This should be part of a tree and therefore have a parent"
+        transform = get_node_transform(parent)
         point = self.node.bounding_box(transform).center
 
         return point
diff --git a/lib/elements/utils.py b/lib/elements/utils.py
index dfe1eb3a..cf770af4 100644
--- a/lib/elements/utils.py
+++ b/lib/elements/utils.py
@@ -3,7 +3,7 @@
 # Copyright (c) 2010 Authors
 # Licensed under the GNU GPL version 3.0 or later.  See the file LICENSE for details.
 
-from typing import List, Optional
+from typing import List, Optional, Iterable
 
 from inkex import BaseElement
 from lxml.etree import Comment
@@ -41,7 +41,7 @@ def node_to_elements(node, clone_to_element=False) -> List[EmbroideryElement]:
         return [MarkerObject(node)]
 
     elif node.tag in EMBROIDERABLE_TAGS or is_clone(node):
-        elements = []
+        elements: List[EmbroideryElement] = []
 
         from ..sew_stack import SewStack
         sew_stack = SewStack(node)
@@ -73,7 +73,7 @@ def node_to_elements(node, clone_to_element=False) -> List[EmbroideryElement]:
         return []
 
 
-def nodes_to_elements(nodes):
+def nodes_to_elements(nodes: Iterable[BaseElement]) -> List[EmbroideryElement]:
     elements = []
     for node in nodes:
         elements.extend(node_to_elements(node))
@@ -89,7 +89,8 @@ def iterate_nodes(node: BaseElement,  # noqa: C901
     def walk(node: BaseElement, selected: bool) -> List[BaseElement]:
         nodes = []
 
-        if node.tag == Comment:
+        # lxml-stubs types are wrong, node.tag can be Comment.
+        if node.tag is Comment:  # type:ignore[comparison-overlap]
             return []
 
         element = EmbroideryElement(node)
diff --git a/lib/elements/validation.py b/lib/elements/validation.py
index 9ac8e745..8edcc7c5 100644
--- a/lib/elements/validation.py
+++ b/lib/elements/validation.py
@@ -3,6 +3,7 @@
 # Copyright (c) 2010 Authors
 # Licensed under the GNU GPL version 3.0 or later.  See the file LICENSE for details.
 
+from typing import Optional, List
 from shapely.geometry import Point as ShapelyPoint
 
 from ..utils import Point as InkstitchPoint
@@ -21,9 +22,9 @@ class ValidationMessage(object):
     '''
 
     # Subclasses will fill these in.
-    name = None
-    description = None
-    steps_to_solve = []
+    name: Optional[str] = None
+    description: Optional[str] = None
+    steps_to_solve: List[str] = []
 
     def __init__(self, position=None, label=""):
         if isinstance(position, ShapelyPoint):