🔀🔧 MERGE: Add strict typing for most sphinx_needs modules (#1002)

This PR adds strict typing to most of the package, building on #987 and #998 to try to confine use of dynamic types to a small surface area.
This hopefully makes it easier for developers to navigate the code base, and lessens the potential for bugs arising from type mismatches.

---

There are still some open questions, particularly around `NeedsInfoType`, which

1. has some dynamic fields (such as user defined links/options) and,
2. has some fields which are only set after post-transforms (such `process_need_nodes`)

.pre-commit-config.yaml

@@ -35,6 +35,7 @@ repos:
       additional_dependencies:
       - sphinx==6
       - types-docutils
+      - types-jsonschema
       - types-requests
 
   - repo: https://github.com/python-poetry/poetry

pyproject.toml

@@ -86,51 +86,45 @@ strict = true
 show_error_codes = true
 implicit_reexport = true
 namespace_packages = true
+disallow_any_generics = true
+disallow_subclassing_any = true
+# disallow_any_unimported = true
+# disallow_any_explicit = true
+# disallow_any_expr = true
+# disallow_any_decorated = true
 
 [[tool.mypy.overrides]]
 module = [
+  "matplotlib.*",
+  "numpy.*",
   "requests_file",
-  "sphinx_data_viewer.*"
+  "sphinx_data_viewer.*",
+  "sphinxcontrib.plantuml.*",
 ]
 ignore_missing_imports = true
 
 [[tool.mypy.overrides]]
 module = [
   'sphinx_needs.api.need',
-  'sphinx_needs.data',
-  'sphinx_needs.diagrams_common',
-  'sphinx_needs.directives.need',
-  'sphinx_needs.directives.needbar',
-  'sphinx_needs.directives.needextend',
-  'sphinx_needs.directives.needextract',
-  'sphinx_needs.directives.needfilter',
-  'sphinx_needs.directives.needflow',
-  'sphinx_needs.directives.needgantt',
-  'sphinx_needs.directives.needlist',
-  'sphinx_needs.directives.needpie',
-  'sphinx_needs.directives.needsequence',
-  'sphinx_needs.directives.needtable',
   'sphinx_needs.directives.needuml',
-  'sphinx_needs.directives.utils',
-  'sphinx_needs.environment',
-  'sphinx_needs.external_needs',
-  'sphinx_needs.filter_common',
-  'sphinx_needs.functions.common',
   'sphinx_needs.functions.functions',
-  'sphinx_needs.layout',
-  'sphinx_needs.needs',
-  'sphinx_needs.needsfile',
-  'sphinx_needs.roles.need_incoming',
-  'sphinx_needs.roles.need_outgoing',
-  'sphinx_needs.roles.need_part',
-  'sphinx_needs.roles.need_ref',
-  'sphinx_needs.services.github',
-  'sphinx_needs.services.manager',
-  'sphinx_needs.utils',
-  'sphinx_needs.warnings',
 ]
 ignore_errors = true
 
+[[tool.mypy.overrides]]
+module = [
+  "sphinx_needs.directives.needextend"
+]
+# TODO dynamically overriding TypeDict keys is a bit tricky
+disable_error_code = "literal-required"
+
+[[tool.mypy.overrides]]
+module = [
+  "sphinx_needs.data"
+]
+disable_error_code = ["attr-defined", "no-any-return"]
+
+
 [build-system]
 requires = ["setuptools", "poetry_core>=1.0.8"]  # setuptools for deps like plantuml
 build-backend = "poetry.core.masonry.api"

sphinx_needs/api/need.py

@@ -309,7 +309,6 @@ def run():
         "docname": docname,
         "doctype": doctype,
         "lineno": lineno,
-        # "target_node": target_node,
         "target_id": need_id,
         "external_url": external_url,
         "content_node": None,  # gets set after rst parsing

sphinx_needs/builder.py

@@ -1,13 +1,13 @@
 import os
-from typing import Iterable, Optional, Set
+from typing import Iterable, List, Optional, Set
 
 from docutils import nodes
 from sphinx import version_info
 from sphinx.application import Sphinx
 from sphinx.builders import Builder
 
 from sphinx_needs.config import NeedsSphinxConfig
-from sphinx_needs.data import SphinxNeedsData
+from sphinx_needs.data import NeedsInfoType, SphinxNeedsData
 from sphinx_needs.logging import get_logger
 from sphinx_needs.needsfile import NeedsList
 from sphinx_needs.utils import unwrap
@@ -27,7 +27,6 @@ def write_doc(self, docname: str, doctree: nodes.document) -> None:
     def finish(self) -> None:
         env = unwrap(self.env)
         data = SphinxNeedsData(env)
-        needs = data.get_or_create_needs().values()  # We need a list of needs for later filter checks
         filters = data.get_or_create_filters()
         version = getattr(env.config, "version", "unset")
         needs_list = NeedsList(env.config, self.outdir, self.srcdir)
@@ -50,7 +49,7 @@ def finish(self) -> None:
         from sphinx_needs.filter_common import filter_needs
 
         filter_string = needs_config.builder_filter
-        filtered_needs = filter_needs(self.app, needs, filter_string)
+        filtered_needs: List[NeedsInfoType] = filter_needs(self.app, data.get_or_create_needs().values(), filter_string)
 
         for need in filtered_needs:
             needs_list.add_need(version, need)

sphinx_needs/config.py

@@ -145,7 +145,7 @@ def __setattr__(self, name: str, value: Any) -> None:
         default=DEFAULT_DIAGRAM_TEMPLATE,
         metadata={"rebuild": "html", "types": (str,)},
     )
-    functions: list[Any] = field(default_factory=list, metadata={"rebuild": "html", "types": (list,)})
+    functions: list[Callable[..., Any]] = field(default_factory=list, metadata={"rebuild": "html", "types": (list,)})
     global_options: dict[str, Any] = field(default_factory=dict, metadata={"rebuild": "html", "types": (dict,)})
     duration_option: str = field(default="duration", metadata={"rebuild": "html", "types": (str,)})
     completion_option: str = field(default="completion", metadata={"rebuild": "html", "types": (str,)})
@@ -171,7 +171,7 @@ def __setattr__(self, name: str, value: Any) -> None:
     extra_links: list[dict[str, Any]] = field(default_factory=list, metadata={"rebuild": "html", "types": ()})
     """List of additional links, which can be used by setting related option
     Values needed for each new link:
-    * name (will also be the option name)
+    * option (will also be the option name)
     * incoming
     * copy_link (copy to common links data. Default: True)
     * color (used for needflow. Default: #000000)

sphinx_needs/data.py

@@ -3,13 +3,13 @@
 """
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 try:
     from typing import Literal, TypedDict
 except ImportError:
     # introduced in python 3.8
-    from typing_extensions import Literal, TypedDict
+    from typing_extensions import Literal, TypedDict  # type: ignore
 
 if TYPE_CHECKING:
     from docutils.nodes import Element, Text
@@ -63,18 +63,17 @@ class NeedsPartType(TypedDict):
     id: str
     """ID of the part"""
 
-    # TODO are these necessary? i.e. its always a part, not a need
     is_part: bool
     is_need: bool
 
     content: str
     """Content of the part."""
     document: str
     """docname where the part is defined."""
-    links_back: list[str]
-    """List of need IDs, which are referencing this part."""
     links: list[str]
     """List of need IDs, which are referenced by this part."""
+    links_back: list[str]
+    """List of need IDs, which are referencing this part."""
 
 
 class NeedsInfoType(NeedsBaseDataType):
@@ -129,7 +128,6 @@ class NeedsInfoType(NeedsBaseDataType):
 
     # parts information
     parts: dict[str, NeedsPartType]
-    # TODO are these necessary? i.e. its always a need, not a part
     is_need: bool
     is_part: bool
 
@@ -149,17 +147,21 @@ class NeedsInfoType(NeedsBaseDataType):
     # link information
     links: list[str]
     """List of need IDs, which are referenced by this need."""
-    # TODO there is a lot more dynamically added link information;
-    # for each item in needs_extra_links config,
+    links_back: list[str]
+    """List of need IDs, which are referencing this need."""
+    # TODO there is more dynamically added link information;
+    # for each item in needs_extra_links config
+    # (and in prepare_env 'links' and 'parent_needs' are added if not present),
     # you end up with a key named by the "option" field,
     # and then another key named by the "option" field + "_back"
     # these all have value type `list[str]`
+    # back links are all set in process_need_nodes (-> create_back_links) transform
 
     # constraints information
     # set in process_need_nodes (-> process_constraints) transform
     constraints: list[str]
     constraints_passed: None | bool
-    constraints_results: dict[str, dict[str, bool]]
+    constraints_results: dict[str, dict[str, Any]]
 
     # additional source information
     doctype: str
@@ -171,8 +173,15 @@ class NeedsInfoType(NeedsBaseDataType):
     signature: str | Text
     """Derived from a docutils desc_name node"""
     parent_needs: list[str]
+    """List of parents of the this need (by id),
+    i.e. if this need is nested in another
+    """
+    parent_needs_back: list[str]
+    """List of children of this need (by id),
+    i.e. if needs are nested within this one
+    """
     parent_need: str
-    """Simply the first parent"""
+    """Simply the first parent id"""
 
     # default extra options
     # TODO these all default to "" which I don't think is good
@@ -192,7 +201,7 @@ class NeedsInfoType(NeedsBaseDataType):
     max_amount: str
     service: str
     specific: str
-    type: str
+    # _type: str  # type is already set in create_need
     updated_at: str
     user: str
     # OpenNeedsService
@@ -210,6 +219,15 @@ class NeedsInfoType(NeedsBaseDataType):
     # - dynamic global options that can be set by needs_global_options config
 
 
+class NeedsPartsInfoType(NeedsInfoType):
+    """Generated by prepare_need_list"""
+
+    document: str
+    """docname where the part is defined."""
+    id_parent: str
+    id_complete: str
+
+
 class NeedsBarType(NeedsBaseDataType):
     """Data for a single (matplotlib) bar diagram."""
 
@@ -225,9 +243,9 @@ class NeedsBarType(NeedsBaseDataType):
     ylabels_rotation: str
     separator: str
     stacked: bool
-    show_sum: bool
-    show_top_sum: bool
-    sum_rotation: bool
+    show_sum: None | bool
+    show_top_sum: None | bool
+    sum_rotation: None | str
     transpose: bool
     horizontal: bool
     style: str
@@ -246,13 +264,12 @@ class NeedsExtendType(NeedsBaseDataType):
 class NeedsFilteredBaseType(NeedsBaseDataType):
     """A base type for all filtered data."""
 
-    # Filter attributes
     status: list[str]
     tags: list[str]
     types: list[str]
     filter: None | str
     sort_by: None | str
-    filter_code: str
+    filter_code: list[str]
     filter_func: None | str
     export_id: str
 
@@ -276,7 +293,6 @@ class NeedsFilteredDiagramBaseType(NeedsFilteredBaseType):
 class NeedsExtractType(NeedsFilteredBaseType):
     """Data to extract needs from a document."""
 
-    export_id: str
     layout: str
     style: str
     show_filters: bool
@@ -294,24 +310,11 @@ class _NeedsFilterType(NeedsFilteredBaseType):
     show_filters: bool
     show_legend: bool
     layout: Literal["list", "table", "diagram"]
-    export_id: str
 
 
 class NeedsFlowType(NeedsFilteredDiagramBaseType):
     """Data for a single (filtered) flow chart."""
 
-    caption: None | str
-    show_filters: bool
-    show_legend: bool
-    show_link_names: bool
-    debug: bool
-    config_names: str
-    config: str
-    scale: str
-    highlight: str
-    align: str
-    link_types: list[str]
-
 
 class NeedsGanttType(NeedsFilteredDiagramBaseType):
     """Data for a single (filtered) gantt chart."""
@@ -333,7 +336,6 @@ class NeedsListType(NeedsFilteredBaseType):
     show_tags: bool
     show_status: bool
     show_filters: bool
-    export_id: str
 
 
 class NeedsPieType(NeedsBaseDataType):
@@ -363,6 +365,7 @@ class NeedsTableType(NeedsFilteredBaseType):
     caption: None | str
     classes: list[str]
     columns: list[tuple[str, str]]
+    """List of (name, title)"""
     colwidths: list[int]
     style: str
     style_row: str
@@ -449,7 +452,7 @@ def get_or_create_workflow(self) -> NeedsWorkflowType:
             for link_type in self.env.app.config.needs_extra_links:
                 self.env.needs_workflow["backlink_creation_{}".format(link_type["option"])] = False
 
-        return self.env.needs_workflow
+        return self.env.needs_workflow  # type: ignore[return-value]
 
     def get_or_create_services(self) -> ServiceManager:
         """Get information about services.