Lint: Fix features module (#218)

* Lint: Fix type issues in features module

* Fix redundant Optional type hints in Parser init

* Fix undefined feature_dir call in ValueError raise in graph function

* Fix Callable type annotation in filter functions

* Explicitly cast exclude_graph_view to DiGraph to fix unknown member

* Raise RuntimeError in graph function if flavor is None

* Tests: Cover bad path for mermaid function

Author: ByteOtter

src/gardenlinux/features/__main__.py

@@ -8,8 +8,6 @@
 import argparse
 import logging
 import os
-import re
-import sys
 from functools import reduce
 from os import path
 from typing import Any, List, Set
@@ -100,9 +98,9 @@ def main() -> None:
         commit_id = cname.commit_id
         version = cname.version
 
-        input_features = Parser.get_cname_as_feature_set(flavor)
+        _ = Parser.get_cname_as_feature_set(flavor)
     else:
-        input_features = args.features
+        _ = args.features
 
     if arch is None or arch == "" and (args.type in ("cname", "arch")):
         raise RuntimeError(
@@ -140,10 +138,10 @@ def main() -> None:
             cname = flavor
 
             if arch is not None:
-                cname += f"-{arch}"
+                cname += f"-{arch}"  # type: ignore - None check is carried out.
 
             if commit_id is not None:
-                cname += f"-{version}-{commit_id}"
+                cname += f"-{version}-{commit_id}"  # type: ignore - None check is carried out.
 
             print(cname)
         elif args.type == "graph":
@@ -173,7 +171,7 @@ def main() -> None:
         print(f"{version}-{commit_id}")
 
 
-def get_cname_base(sorted_features: Set[str]):
+def get_cname_base(sorted_features: List[str]):
     """
     Get the base cname for the feature set given.
 
@@ -228,7 +226,7 @@ def get_minimal_feature_set(graph: Any) -> Set[str]:
     return set([node for (node, degree) in graph.in_degree() if degree == 0])
 
 
-def graph_as_mermaid_markup(flavor: str, graph: Any) -> str:
+def graph_as_mermaid_markup(flavor: str | None, graph: Any) -> str:
     """
     Generates a mermaid.js representation of the graph.
     This is helpful to identify dependencies between features.
@@ -243,6 +241,9 @@ def graph_as_mermaid_markup(flavor: str, graph: Any) -> str:
     :since:  0.7.0
     """
 
+    if flavor is None:
+        raise RuntimeError("Error while generating graph: Flavor is None!")
+
     markup = f"---\ntitle: Dependency Graph for Feature {flavor}\n---\ngraph TD;\n"
 
     for u, v in graph.edges:

src/gardenlinux/features/cname.py

@@ -92,13 +92,13 @@ def cname(self) -> str:
 
         :return: (str) CName
         """
-
+        assert self._flavor is not None, "CName flavor is not set!"
         cname = self._flavor
 
         if self._arch is not None:
             cname += f"-{self._arch}"
 
-        if self._commit_id is not None:
+        if self._commit_id is not None and self._version is not None:
             cname += f"-{self.version_and_commit_id}"
 
         return cname
@@ -114,7 +114,7 @@ def commit_id(self) -> Optional[str]:
         return self._commit_id
 
     @property
-    def flavor(self) -> str:
+    def flavor(self) -> str | None:
         """
         Returns the flavor for the cname parsed.
 
@@ -140,6 +140,7 @@ def platform(self) -> str:
 
         :return: (str) Flavor
         """
+        assert self._flavor is not None, "Flavor not set!"
 
         return re.split("[_-]", self._flavor, maxsplit=1)[0]
 

src/gardenlinux/features/cname_main.py

@@ -8,7 +8,6 @@
 import argparse
 import logging
 import re
-from functools import reduce
 from os.path import basename, dirname
 
 from .__main__ import (
@@ -81,8 +80,7 @@ def main():
 
     generated_cname = get_cname_base(sorted_minimal_features)
 
-    if cname.arch is not None:
-        generated_cname += f"-{cname.arch}"
+    generated_cname += f"-{cname.arch}"
 
     if cname.version_and_commit_id is not None:
         generated_cname += f"-{cname.version_and_commit_id}"

src/gardenlinux/features/parser.py

@@ -6,19 +6,14 @@
 
 import logging
 import os
-import re
-import subprocess
 from glob import glob
-from typing import Callable, Optional
+from pathlib import Path
+from typing import Callable, Optional, cast
 
 import networkx
 import yaml
 
-from ..constants import (
-    ARCHS,
-    BARE_FLAVOR_FEATURE_CONTENT,
-    BARE_FLAVOR_LIBC_FEATURE_CONTENT,
-)
+from ..constants import BARE_FLAVOR_FEATURE_CONTENT, BARE_FLAVOR_LIBC_FEATURE_CONTENT
 from ..logger import LoggerSetup
 
 
@@ -42,8 +37,8 @@ class Parser(object):
 
     def __init__(
         self,
-        gardenlinux_root: Optional[str] = None,
-        feature_dir_name: Optional[str] = "features",
+        gardenlinux_root: str | None = None,
+        feature_dir_name: str = "features",
         logger: Optional[logging.Logger] = None,
     ):
         """
@@ -59,7 +54,7 @@ def __init__(
         if gardenlinux_root is None:
             gardenlinux_root = Parser._GARDENLINUX_ROOT
 
-        feature_base_dir = os.path.join(gardenlinux_root, feature_dir_name)
+        feature_base_dir = Path(gardenlinux_root).resolve() / feature_dir_name
 
         if not os.access(feature_base_dir, os.R_OK):
             raise ValueError(
@@ -70,7 +65,6 @@ def __init__(
             logger = LoggerSetup.get_logger("gardenlinux.features")
 
         self._feature_base_dir = feature_base_dir
-
         self._graph = None
         self._logger = logger
 
@@ -108,7 +102,7 @@ def graph(self) -> networkx.Graph:
                             "{0}/{1}/info.yaml".format(self._feature_base_dir, ref)
                         ):
                             raise ValueError(
-                                f"feature {node} references feature {ref}, but {feature_dir}/{ref}/info.yaml does not exist"
+                                f"feature {node} references feature {ref}, but {self._feature_base_dir}/{ref}/info.yaml does not exist"
                             )
 
                         feature_graph.add_edge(node, ref, attr=attr)
@@ -122,9 +116,9 @@ def graph(self) -> networkx.Graph:
 
     def filter(
         self,
-        cname: str,
+        cname: str | None,
         ignore_excludes: bool = False,
-        additional_filter_func: Optional[Callable[(str,), bool]] = None,
+        additional_filter_func: Optional[Callable[[str], bool]] = None,
     ) -> networkx.Graph:
         """
         Filters the features graph.
@@ -162,15 +156,15 @@ def filter(
         )
 
         if not ignore_excludes:
-            Parser._exclude_from_filter_set(graph, input_features, filter_set)
+            Parser._exclude_from_filter_set(self, graph, input_features, filter_set)
 
         return graph
 
     def filter_as_dict(
         self,
-        cname: str,
+        cname: str | None,
         ignore_excludes: bool = False,
-        additional_filter_func: Optional[Callable[(str,), bool]] = None,
+        additional_filter_func: Optional[Callable[[str], bool]] = None,
     ) -> dict:
         """
         Filters the features graph and returns it as a dict.
@@ -200,9 +194,9 @@ def filter_as_dict(
 
     def filter_as_list(
         self,
-        cname: str,
+        cname: str | None,
         ignore_excludes: bool = False,
-        additional_filter_func: Optional[Callable[(str,), bool]] = None,
+        additional_filter_func: Optional[Callable[[str], bool]] = None,
     ) -> list:
         """
         Filters the features graph and returns it as a list.
@@ -220,9 +214,9 @@ def filter_as_list(
 
     def filter_as_string(
         self,
-        cname: str,
+        cname: str | None,
         ignore_excludes: bool = False,
-        additional_filter_func: Optional[Callable[(str,), bool]] = None,
+        additional_filter_func: Optional[Callable[[str], bool]] = None,
     ) -> str:
         """
         Filters the features graph and returns it as a string.
@@ -240,7 +234,7 @@ def filter_as_string(
 
         return ",".join(features)
 
-    def _exclude_from_filter_set(graph, input_features, filter_set):
+    def _exclude_from_filter_set(self, graph, input_features, filter_set):
         """
         Removes the given `filter_set` out of `input_features`.
 
@@ -250,7 +244,9 @@ def _exclude_from_filter_set(graph, input_features, filter_set):
         :since: 0.7.0
         """
 
-        exclude_graph_view = Parser._get_graph_view_for_attr(graph, "exclude")
+        exclude_graph_view = cast(
+            networkx.DiGraph, Parser._get_graph_view_for_attr(graph, "exclude")
+        )
         exclude_list = []
 
         for node in networkx.lexicographical_topological_sort(graph):

tests/features/test_main.py

@@ -77,6 +77,18 @@ class FakeGraph:
     assert "b-->c" in markup
 
 
+def test_graph_mermaid_raises_no_flavor():
+    # Arrange
+    class MockGraph:
+        edges = [("x", "y"), ("y", "z")]
+
+    # Act / Assert
+    with pytest.raises(
+        RuntimeError, match="Error while generating graph: Flavor is None!"
+    ):
+        fema.graph_as_mermaid_markup(None, MockGraph())
+
+
 def test_get_minimal_feature_set_filters():
     # Arrange
     class FakeGraph: