PolicyDifference: Add type annotations.

Includes some minor code changes to fix errors from static type checking.

Disable unsubscriptable-object pylint check on Wrapper subclass
declarations, as this hits the bug described in pylint-dev/pylint#2822.

Signed-off-by: Chris PeBenito <[email protected]>

Author: pebenito

setools/diff/bool.py

@@ -17,18 +17,28 @@
 # License along with SETools.  If not, see
 # <http://www.gnu.org/licenses/>.
 #
-from collections import defaultdict, namedtuple
+from collections import defaultdict
+from typing import NamedTuple
+
+from ..policyrep import SELinuxPolicy, Boolean
 
 from .descriptors import DiffResultDescriptor
 from .difference import Difference, SymbolWrapper
+from .typing import SymbolCache
+
+
+_bool_cache: SymbolCache[Boolean] = defaultdict(dict)
+
 
+class ModifiedBoolean(NamedTuple):
 
-modified_bool_record = namedtuple("modified_boolean", ["added_state", "removed_state"])
+    """Difference details for a modified Boolean."""
 
-_bool_cache = defaultdict(dict)
+    added_state: bool
+    removed_state: bool
 
 
-def boolean_wrapper(policy, boolean):
+def boolean_wrapper(policy: SELinuxPolicy, boolean: Boolean) -> SymbolWrapper[Boolean]:
     """
     Wrap booleans from the specified policy.
 
@@ -51,7 +61,7 @@ class BooleansDifference(Difference):
     removed_booleans = DiffResultDescriptor("diff_booleans")
     modified_booleans = DiffResultDescriptor("diff_booleans")
 
-    def diff_booleans(self):
+    def diff_booleans(self) -> None:
         """Generate the difference in type attributes between the policies."""
 
         self.log.info("Generating Boolean differences from {0.left_policy} to {0.right_policy}".
@@ -68,13 +78,13 @@ def diff_booleans(self):
             # Criteria for modified booleans
             # 1. change to default state
             if left_boolean.state != right_boolean.state:
-                self.modified_booleans[left_boolean] = modified_bool_record(right_boolean.state,
-                                                                            left_boolean.state)
+                self.modified_booleans[left_boolean] = ModifiedBoolean(right_boolean.state,
+                                                                       left_boolean.state)
 
     #
     # Internal functions
     #
-    def _reset_diff(self):
+    def _reset_diff(self) -> None:
         """Reset diff results on policy changes."""
         self.log.debug("Resetting Boolean differences")
         self.added_booleans = None

setools/diff/bounds.py

@@ -17,15 +17,21 @@
 # License along with SETools.  If not, see
 # <http://www.gnu.org/licenses/>.
 #
-from collections import namedtuple
+from typing import cast, List, NamedTuple, Optional
 
-from ..policyrep import BoundsRuletype
+from ..policyrep import Bounds, BoundsRuletype, Type
 from .descriptors import DiffResultDescriptor
 from .difference import Difference, Wrapper
 from .types import type_wrapper_factory
 
 
-modified_bounds_record = namedtuple("modified_bound", ["rule", "added_bound", "removed_bound"])
+class ModifiedBounds(NamedTuple):
+
+    """Difference details for a modified bounds rule."""
+
+    rule: Bounds
+    added_bound: Type
+    removed_bound: Type
 
 
 class BoundsDifference(Difference):
@@ -37,10 +43,10 @@ class BoundsDifference(Difference):
     modified_typebounds = DiffResultDescriptor("diff_typebounds")
 
     # Lists of rules for each policy
-    _left_typebounds = None
-    _right_typebounds = None
+    _left_typebounds: Optional[List[Bounds]] = None
+    _right_typebounds: Optional[List[Bounds]] = None
 
-    def diff_typebounds(self):
+    def diff_typebounds(self) -> None:
         """Generate the difference in typebound rules between the policies."""
 
         self.log.info("Generating typebounds differences from {0.left_policy} to {0.right_policy}".
@@ -50,21 +56,21 @@ def diff_typebounds(self):
             self._create_typebound_lists()
 
         self.added_typebounds, self.removed_typebounds, matched_typebounds = self._set_diff(
-            (BoundsWrapper(c) for c in self._left_typebounds),
-            (BoundsWrapper(c) for c in self._right_typebounds),
+            (BoundsWrapper(c) for c in cast(List[Bounds], self._left_typebounds)),
+            (BoundsWrapper(c) for c in cast(List[Bounds], self._right_typebounds)),
             key=lambda b: str(b.child))
 
         self.modified_typebounds = []
 
         for left_bound, right_bound in matched_typebounds:
             if type_wrapper_factory(left_bound.parent) != type_wrapper_factory(right_bound.parent):
-                self.modified_typebounds.append(modified_bounds_record(
+                self.modified_typebounds.append(ModifiedBounds(
                     left_bound, right_bound.parent, left_bound.parent))
 
     #
     # Internal functions
     #
-    def _create_typebound_lists(self):
+    def _create_typebound_lists(self) -> None:
         """Create rule lists for both policies."""
         self._left_typebounds = []
         for rule in self.left_policy.bounds():
@@ -82,7 +88,7 @@ def _create_typebound_lists(self):
                 self.log.error("Unknown rule type: {0} (This is an SETools bug)".
                                format(rule.ruletype))
 
-    def _reset_diff(self):
+    def _reset_diff(self) -> None:
         """Reset diff results on policy changes."""
         self.log.debug("Resetting all *bounds differences")
         self.added_typebounds = None
@@ -93,13 +99,14 @@ def _reset_diff(self):
         self._right_typebounds = None
 
 
-class BoundsWrapper(Wrapper):
+# Pylint bug: https://github.com/PyCQA/pylint/issues/2822
+class BoundsWrapper(Wrapper[Bounds]):  # pylint: disable=unsubscriptable-object
 
     """Wrap *bounds for diff purposes."""
 
     __slots__ = ("ruletype", "parent", "child")
 
-    def __init__(self, rule):
+    def __init__(self, rule: Bounds) -> None:
         self.origin = rule
         self.ruletype = rule.ruletype
         self.parent = type_wrapper_factory(rule.parent)

setools/diff/commons.py

@@ -16,15 +16,19 @@
 # License along with SETools.  If not, see
 # <http://www.gnu.org/licenses/>.
 #
-from collections import namedtuple
+from typing import NamedTuple, Set
 
 from .descriptors import DiffResultDescriptor
 from .difference import Difference, SymbolWrapper
 
 
-modified_commons_record = namedtuple("modified_common", ["added_perms",
-                                                         "removed_perms",
-                                                         "matched_perms"])
+class ModifiedCommon(NamedTuple):
+
+    """Difference details for a modified common permission set."""
+
+    added_perms: Set[str]
+    removed_perms: Set[str]
+    matched_perms: Set[str]
 
 
 class CommonDifference(Difference):
@@ -38,7 +42,7 @@ class CommonDifference(Difference):
     removed_commons = DiffResultDescriptor("diff_commons")
     modified_commons = DiffResultDescriptor("diff_commons")
 
-    def diff_commons(self):
+    def diff_commons(self) -> None:
         """Generate the difference in commons between the policies."""
 
         self.log.info(
@@ -58,14 +62,14 @@ def diff_commons(self):
                                                                        unwrap=False)
 
             if added_perms or removed_perms:
-                self.modified_commons[left_common] = modified_commons_record(added_perms,
-                                                                             removed_perms,
-                                                                             matched_perms)
+                self.modified_commons[left_common] = ModifiedCommon(added_perms,
+                                                                    removed_perms,
+                                                                    matched_perms)
 
     #
     # Internal functions
     #
-    def _reset_diff(self):
+    def _reset_diff(self) -> None:
         """Reset diff results on policy changes."""
         self.log.debug("Resetting common differences")
         self.added_commons = None

setools/diff/conditional.py

@@ -19,13 +19,16 @@
 #
 from collections import defaultdict
 
+from ..policyrep import Conditional
+
 from .difference import Wrapper
+from .typing import Cache
 
 
-_cond_cache = defaultdict(dict)
+_cond_cache: Cache[Conditional, "ConditionalWrapper"] = defaultdict(dict)
 
 
-def conditional_wrapper_factory(cond):
+def conditional_wrapper_factory(cond: Conditional) -> "ConditionalWrapper":
     """
     Wrap type attributes from the specified policy.
 
@@ -40,13 +43,14 @@ def conditional_wrapper_factory(cond):
         return a
 
 
-class ConditionalWrapper(Wrapper):
+# Pylint bug: https://github.com/PyCQA/pylint/issues/2822
+class ConditionalWrapper(Wrapper[Conditional]):  # pylint: disable=unsubscriptable-object
 
     """Wrap conditional policy expressions to allow comparisons by truth table."""
 
     __slots__ = ("truth_table")
 
-    def __init__(self, cond):
+    def __init__(self, cond: Conditional) -> None:
         self.origin = cond
         self.truth_table = cond.truth_table()
 

setools/diff/constraints.py

@@ -18,11 +18,14 @@
 # <http://www.gnu.org/licenses/>.
 #
 from collections import defaultdict
+from typing import FrozenSet, List, Optional, Union
+
+from ..policyrep import AnyConstraint, ConstraintRuletype, Role, Type, User
 
-from ..policyrep import ConstraintRuletype
 from .descriptors import DiffResultDescriptor
 from .difference import Difference, SymbolWrapper, Wrapper
 from .objclass import class_wrapper_factory
+from .typing import RuleList
 
 
 class ConstraintsDifference(Difference):
@@ -55,10 +58,10 @@ class ConstraintsDifference(Difference):
     removed_mlsvalidatetrans = DiffResultDescriptor("diff_mlsvalidatetrans")
 
     # Lists of rules for each policy
-    _left_constraints = None
-    _right_constraints = None
+    _left_constraints: RuleList[ConstraintRuletype, AnyConstraint] = None
+    _right_constraints: RuleList[ConstraintRuletype, AnyConstraint] = None
 
-    def diff_constrains(self):
+    def diff_constrains(self) -> None:
         """Generate the difference in constraint rules between the policies."""
 
         self.log.info("Generating constraint differences from {0.left_policy} to {0.right_policy}".
@@ -67,11 +70,14 @@ def diff_constrains(self):
         if self._left_constraints is None or self._right_constraints is None:
             self._create_constrain_lists()
 
+        assert self._left_constraints is not None, "Left constraints didn't load, this a bug."
+        assert self._right_constraints is not None, "Right constraints didn't load, this a bug."
+
         self.added_constrains, self.removed_constrains, _ = self._set_diff(
             (ConstraintWrapper(c) for c in self._left_constraints[ConstraintRuletype.constrain]),
             (ConstraintWrapper(c) for c in self._right_constraints[ConstraintRuletype.constrain]))
 
-    def diff_mlsconstrains(self):
+    def diff_mlsconstrains(self) -> None:
         """Generate the difference in MLS constraint rules between the policies."""
 
         self.log.info(
@@ -81,13 +87,16 @@ def diff_mlsconstrains(self):
         if self._left_constraints is None or self._right_constraints is None:
             self._create_constrain_lists()
 
+        assert self._left_constraints is not None, "Left constraints didn't load, this a bug."
+        assert self._right_constraints is not None, "Right constraints didn't load, this a bug."
+
         self.added_mlsconstrains, self.removed_mlsconstrains, _ = self._set_diff(
             (ConstraintWrapper(c) for c in self._left_constraints[
                 ConstraintRuletype.mlsconstrain]),
             (ConstraintWrapper(c) for c in self._right_constraints[
                 ConstraintRuletype.mlsconstrain]))
 
-    def diff_validatetrans(self):
+    def diff_validatetrans(self) -> None:
         """Generate the difference in validatetrans rules between the policies."""
 
         self.log.info(
@@ -97,13 +106,16 @@ def diff_validatetrans(self):
         if self._left_constraints is None or self._right_constraints is None:
             self._create_constrain_lists()
 
+        assert self._left_constraints is not None, "Left constraints didn't load, this a bug."
+        assert self._right_constraints is not None, "Right constraints didn't load, this a bug."
+
         self.added_validatetrans, self.removed_validatetrans, _ = self._set_diff(
             (ConstraintWrapper(c) for c in self._left_constraints[
                 ConstraintRuletype.validatetrans]),
             (ConstraintWrapper(c) for c in self._right_constraints[
                 ConstraintRuletype.validatetrans]))
 
-    def diff_mlsvalidatetrans(self):
+    def diff_mlsvalidatetrans(self) -> None:
         """Generate the difference in MLS validatetrans rules between the policies."""
 
         self.log.info(
@@ -113,6 +125,9 @@ def diff_mlsvalidatetrans(self):
         if self._left_constraints is None or self._right_constraints is None:
             self._create_constrain_lists()
 
+        assert self._left_constraints is not None, "Left constraints didn't load, this a bug."
+        assert self._right_constraints is not None, "Right constraints didn't load, this a bug."
+
         self.added_mlsvalidatetrans, self.removed_mlsvalidatetrans, _ = self._set_diff(
             (ConstraintWrapper(c) for c in self._left_constraints[
                 ConstraintRuletype.mlsvalidatetrans]),
@@ -122,7 +137,7 @@ def diff_mlsvalidatetrans(self):
     #
     # Internal functions
     #
-    def _create_constrain_lists(self):
+    def _create_constrain_lists(self) -> None:
         """Create rule lists for both policies."""
         self._left_constraints = defaultdict(list)
         self.log.debug("Building constraint lists from {0.left_policy}".format(self))
@@ -142,7 +157,7 @@ def _create_constrain_lists(self):
 
         self.log.debug("Completed building constraint rule lists.")
 
-    def _reset_diff(self):
+    def _reset_diff(self) -> None:
         """Reset diff results on policy changes."""
         self.log.debug("Resetting all constraints differences")
         self.added_constrains = None
@@ -159,26 +174,28 @@ def _reset_diff(self):
         self._right_constraints = None
 
 
-class ConstraintWrapper(Wrapper):
+# Pylint bug: https://github.com/PyCQA/pylint/issues/2822
+class ConstraintWrapper(Wrapper[AnyConstraint]):  # pylint: disable=unsubscriptable-object
 
     """Wrap constraints for diff purposes."""
 
     __slots__ = ("ruletype", "tclass", "perms", "expr")
 
-    def __init__(self, rule):
+    def __init__(self, rule: AnyConstraint) -> None:
         self.origin = rule
         self.ruletype = rule.ruletype
         self.tclass = class_wrapper_factory(rule.tclass)
 
         try:
-            self.perms = rule.perms
+            self.perms: Optional[FrozenSet[str]] = rule.perms
         except AttributeError:
             # (mls)validatetrans
             self.perms = None
 
         self.key = hash(rule)
 
-        self.expr = []
+        self.expr: List[Union[FrozenSet[SymbolWrapper[Union[Role, Type, User]]], str]] = []
+
         for op in rule.expression:
             if isinstance(op, frozenset):
                 # lists of types/users/roles