🔧 Fix typing of need docname/lineno (#1134)

Account for these being `None` for external needs, that have no source mapping within the project

Practically, if `is_external is False` then `docname/lineno` are`str`, but if `is_external is True` then `docname/lineno` are `None`.
This internal relationship is difficult to encode in the type system though.
So here we always account for  `docname is None`, even if we have already checked for `is_external is False`

sphinx_needs/api/need.py

@@ -326,8 +326,8 @@ def run():
 
     # Add the need and all needed information
     needs_info: NeedsInfoType = {
-        "docname": docname,  # type: ignore[typeddict-item]
-        "lineno": lineno,  # type: ignore[typeddict-item]
+        "docname": docname,
+        "lineno": lineno,
         "doctype": doctype,
         "target_id": need_id,
         "content_node": None,

sphinx_needs/data.py

@@ -48,12 +48,10 @@ class NeedsInfoType(TypedDict, total=False):
     id: Required[str]
     """ID of the data (same as target_id)"""
 
-    # TODO docname and lineno can be None, if the need is external,
-    # but currently this raises mypy errors for other parts of the code base
-    docname: Required[str]
-    """Name of the document where the need is defined."""
-    lineno: Required[int]
-    """Line number where the need is defined."""
+    docname: Required[str | None]
+    """Name of the document where the need is defined (None if external)"""
+    lineno: Required[int | None]
+    """Line number where the need is defined (None if external)"""
 
     # meta information
     full_title: Required[str]

sphinx_needs/diagrams_common.py

@@ -195,12 +195,9 @@ def calculate_link(
             if not parsed_url.scheme and not os.path.isabs(need_info["external_url"]):
                 # only need to add ../ or ..\ to get out of the image folder
                 link = ".." + os.path.sep + need_info["external_url"]
-        else:
+        elif _docname := need_info["docname"]:
             link = (
-                "../"
-                + builder.get_target_uri(need_info["docname"])
-                + "#"
-                + need_info["target_id"]
+                "../" + builder.get_target_uri(_docname) + "#" + need_info["target_id"]
             )
             if need_info["is_part"]:
                 link = f"{link}.{need_info['id']}"

sphinx_needs/directives/needfilter.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import os
+from contextlib import suppress
 from typing import Sequence
 from urllib.parse import urlparse
 
@@ -179,12 +180,10 @@ def process_needfilters(
                 # Create a reference
                 if need_info["hide"]:
                     line_node += title
-                else:
+                elif _docname := need_info["docname"]:
                     ref = nodes.reference("", "")
-                    ref["refdocname"] = need_info["docname"]
-                    ref["refuri"] = builder.get_relative_uri(
-                        fromdocname, need_info["docname"]
-                    )
+                    ref["refdocname"] = _docname
+                    ref["refuri"] = builder.get_relative_uri(fromdocname, _docname)
                     ref["refuri"] += "#" + target_id
                     ref.append(title)
                     line_node += ref
@@ -211,16 +210,17 @@ def process_needfilters(
                 # But the generated link in the svg will be relative to the svg-file location
                 # (e.g. server.com/docs/_images/sqwxo499cnq329439dfjne.svg)
                 # and not to current documentation. Therefore we need to add ../ to get out of the image folder.
-                try:
-                    link = (
-                        "../"
-                        + builder.get_target_uri(need_info["docname"])
-                        + "?highlight={}".format(urlparse(need_info["title"]))
-                        + "#"
-                        + target_id
-                    )  # Gets mostly called during latex generation
-                except NoUri:
-                    link = ""
+                link = ""
+                with suppress(NoUri):
+                    # Gets mostly called during latex generation
+                    if _docname := need_info["docname"]:
+                        link = (
+                            "../"
+                            + builder.get_target_uri(_docname)
+                            + "?highlight={}".format(urlparse(need_info["title"]))
+                            + "#"
+                            + target_id
+                        )
 
                 diagram_template = Template(needs_config.diagram_template)
                 node_text = diagram_template.render(

sphinx_needs/directives/needlist.py

@@ -121,13 +121,11 @@ def process_needlist(
                     ref["classes"].append(need_info["external_css"])
                     ref.append(title)
                     para += ref
-                else:
+                elif _docname := need_info["docname"]:
                     target_id = need_info["target_id"]
                     ref = nodes.reference("", "")
-                    ref["refdocname"] = need_info["docname"]
-                    ref["refuri"] = builder.get_relative_uri(
-                        fromdocname, need_info["docname"]
-                    )
+                    ref["refdocname"] = _docname
+                    ref["refuri"] = builder.get_relative_uri(fromdocname, _docname)
                     ref["refuri"] += "#" + target_id
                     ref.append(title)
                     para += ref

sphinx_needs/functions/common.py

@@ -170,7 +170,7 @@ def copy(
             NeedsSphinxConfig(app.config),
             filter,
             need,
-            location=(need["docname"], need["lineno"]),
+            location=(need["docname"], need["lineno"]) if need["docname"] else None,
         )
         if result:
             need = result[0]

sphinx_needs/layout.py

@@ -12,7 +12,7 @@
 from contextlib import suppress
 from functools import lru_cache
 from optparse import Values
-from typing import Callable
+from typing import Callable, cast
 from urllib.parse import urlparse
 
 import requests
@@ -63,7 +63,10 @@ def create_need(
     # This is done for original need content automatically.
     # But as we are working on  a copy, we have to trigger this on our own.
     if docname is None:
-        docname = needs[need_id]["docname"]  # needed to calculate relative references
+        # needed to calculate relative references
+        # TODO ideally we should not cast here:
+        # the docname can still be None, if the need is external, although practically these are not rendered
+        docname = cast(str, needs[need_id]["docname"])
 
     node_container = nodes.container()
     # node_container += needs[need_id]["need_node"].children
@@ -679,16 +682,16 @@ def meta_id(self) -> nodes.inline:
         id_container = nodes.inline(classes=["needs-id"])
 
         nodes_id_text = nodes.Text(self.need["id"])
-        id_ref = make_refnode(
-            self.app.builder,
-            # fromdocname=self.need['docname'],
-            fromdocname=self.fromdocname,
-            todocname=self.need["docname"],
-            targetid=self.need["id"],
-            child=nodes_id_text.deepcopy(),
-            title=self.need["id"],
-        )
-        id_container += id_ref
+        if self.fromdocname and (_docname := self.need["docname"]):
+            id_ref = make_refnode(
+                self.app.builder,
+                fromdocname=self.fromdocname,
+                todocname=_docname,
+                targetid=self.need["id"],
+                child=nodes_id_text.deepcopy(),
+                title=self.need["id"],
+            )
+            id_container += id_ref
         return id_container
 
     def meta_all(
@@ -924,8 +927,12 @@ def image(
 
             url = value
 
-            if not is_external and not os.path.isabs(url):
-                subfolder_amount = self.need["docname"].count("/")
+            if (
+                not is_external
+                and not os.path.isabs(url)
+                and (docname := self.need["docname"])
+            ):
+                subfolder_amount = docname.count("/")
                 url = "../" * subfolder_amount + url
 
         if is_external:
@@ -963,7 +970,8 @@ def image(
         # Sphinx voodoo needed here.
         # It is not enough to just add a doctuils nodes.image, we also have to register the imag location for sphinx
         # Otherwise the images gets not copied to the later build-output location
-        env.images.add_file(self.need["docname"], url)
+        if _docname := self.need["docname"]:
+            env.images.add_file(_docname, url)
 
         data_container.append(image_node)
         return data_container
@@ -1133,10 +1141,10 @@ def permalink(
 
         permalink = self.needs_config.permalink_file
         id = self.need["id"]
-        docname = self.need["docname"]
         permalink_url = ""
-        for _ in range(0, len(docname.split("/")) - 1):
-            permalink_url += "../"
+        if docname := self.need["docname"]:
+            for _ in range(0, len(docname.split("/")) - 1):
+                permalink_url += "../"
         permalink_url += permalink + "?id=" + id
 
         return self.link(

sphinx_needs/roles/need_incoming.py

@@ -57,11 +57,13 @@ def process_need_incoming(
                     #     link_text += ", "
                     node_need_backref[0] = nodes.Text(link_text)
 
-                    if not target_need["is_external"]:
+                    if not target_need["is_external"] and (
+                        _docname := target_need["docname"]
+                    ):
                         new_node_ref = make_refnode(
                             builder,
                             fromdocname,
-                            target_need["docname"],
+                            _docname,
                             target_need["target_id"],
                             node_need_backref[0].deepcopy(),
                             node_need_backref["reftarget"],

sphinx_needs/roles/need_outgoing.py

@@ -81,11 +81,13 @@ def process_need_outgoing(
 
                     node_need_ref[0] = nodes.Text(link_text)
 
-                    if not target_need["is_external"]:
+                    if not target_need["is_external"] and (
+                        _docname := target_need["docname"]
+                    ):
                         new_node_ref = make_refnode(
                             builder,
                             fromdocname,
-                            target_need["docname"],
+                            _docname,
                             target_id,
                             node_need_ref[0].deepcopy(),
                             node_need_ref["reftarget"],

sphinx_needs/roles/need_part.py

@@ -16,6 +16,7 @@
 from docutils import nodes
 from sphinx.application import Sphinx
 from sphinx.environment import BuildEnvironment
+from sphinx.util.nodes import make_refnode
 
 from sphinx_needs.data import NeedsInfoType
 from sphinx_needs.logging import get_logger
@@ -59,7 +60,6 @@ def update_need_with_parts(
     env: BuildEnvironment, need: NeedsInfoType, part_nodes: list[NeedPart]
 ) -> None:
     app = env.app
-    builder = app.builder
     for part_node in part_nodes:
         content = cast(str, part_node.children[0].children[0])  # ->inline->Text
         result = part_pattern.match(content)
@@ -91,24 +91,26 @@ def update_need_with_parts(
         }
 
         part_id_ref = "{}.{}".format(need["id"], inline_id)
-        part_id_show = inline_id
+
         part_node["reftarget"] = part_id_ref
 
-        part_link_text = f" {part_id_show}"
-        part_link_node = nodes.Text(part_link_text)
         part_text_node = nodes.Text(part_content)
 
-        from sphinx.util.nodes import make_refnode
-
-        part_ref_node = make_refnode(
-            builder, need["docname"], need["docname"], part_id_ref, part_link_node
-        )
-        part_ref_node["classes"] += ["needs-id"]
-
         part_node.children = []
         node_need_part_line = nodes.inline(ids=[part_id_ref], classes=["need-part"])
         node_need_part_line.append(part_text_node)
-        node_need_part_line.append(part_ref_node)
+
+        if docname := need["docname"]:
+            part_id_show = inline_id
+            part_link_text = f" {part_id_show}"
+            part_link_node = nodes.Text(part_link_text)
+
+            part_ref_node = make_refnode(
+                app.builder, docname, docname, part_id_ref, part_link_node
+            )
+            part_ref_node["classes"] += ["needs-id"]
+            node_need_part_line.append(part_ref_node)
+
         part_node.append(node_need_part_line)
 
 

sphinx_needs/roles/need_ref.py

@@ -133,11 +133,13 @@ def process_need_ref(
             node_need_ref[0].children[0] = nodes.Text(link_text)  # type: ignore[index]
 
             with contextlib.suppress(NoUri):
-                if not target_need.get("is_external", False):
+                if not target_need.get("is_external", False) and (
+                    _docname := target_need["docname"]
+                ):
                     new_node_ref = make_refnode(
                         builder,
                         fromdocname,
-                        target_need["docname"],
+                        _docname,
                         node_need_ref["reftarget"],
                         node_need_ref[0].deepcopy(),
                         node_need_ref["reftarget"],

sphinx_needs/utils.py

@@ -215,9 +215,9 @@ def row_col_maker(
                             )
                             ref_col["classes"].append(need_info["external_css"])
                             row_col["classes"].append(need_info["external_css"])
-                        else:
+                        elif _docname := need_info["docname"]:
                             ref_col["refuri"] = builder.get_relative_uri(
-                                fromdocname, need_info["docname"]
+                                fromdocname, _docname
                             )
                             ref_col["refuri"] += "#" + datum
                     elif ref_lookup:
@@ -231,9 +231,9 @@ def row_col_maker(
                             )
                             ref_col["classes"].append(temp_need["external_css"])
                             row_col["classes"].append(temp_need["external_css"])
-                        else:
+                        elif _docname := temp_need["docname"]:
                             ref_col["refuri"] = builder.get_relative_uri(
-                                fromdocname, temp_need["docname"]
+                                fromdocname, _docname
                             )
                             ref_col["refuri"] += "#" + temp_need["id"]
                             if link_part: