Repository: useblocks/sphinx-needs
Pull request #1469 (2025-07-10 11:15:00)

👌 Improve need part processing
The `need_part`/`np` role is not a reference and should not use an `XRefRole` subclass (see #1437). 

In addition, the processing now correctly handles and warns on need parts that are not part of a need and `need` roles that reference an unknown part.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1469?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 88.63%. Comparing base [(`4e10030`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/4e10030581c03c59764e517ab750d3ddbb2695d8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) to head [(`931835f`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/931835f9b2ab0e3ce0bcebc45c2be34986e18c2c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).
> Report is 135 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1469      +/-   ##
==========================================
+ Coverage   86.87%   88.63%   +1.75%     
==========================================
  Files          56       61       +5     
  Lines        6532     7277     +745     
==========================================
+ Hits         5675     6450     +775     
+ Misses        857      827      -30     
```

| [Flag](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1469/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | Coverage Δ | |
|---|---|---|
| [pytests](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | `88.63% <100.00%> (+1.75%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1469?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>

Patch:
diff --git a/pyproject.toml b/pyproject.toml
index 32eecab9..0dbd259d 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -144,7 +144,7 @@ disable_error_code = ["no-redef"]
 
 legacy_tox_ini = """
 [tox]
-envlist = py10
+envlist = py310
 
 [testenv]
 usedevelop = true
diff --git a/sphinx_needs/logging.py b/sphinx_needs/logging.py
index 17e1ee42..e5a88f2e 100644
--- a/sphinx_needs/logging.py
+++ b/sphinx_needs/logging.py
@@ -42,6 +42,7 @@ WarningSubTypes = Literal[
     "load_external_need",
     "load_service_need",
     "mpl",
+    "part",
     "title",
     "uml",
     "unknown_external_keys",
@@ -81,6 +82,7 @@ WarningSubTypeDescription: dict[WarningSubTypes, str] = {
     "load_external_need": "Failed to load an external need",
     "load_service_need": "Failed to load a service need",
     "mpl": "Matplotlib required but not installed",
+    "part": "Error processing need part",
     "title": "Error creating need title",
     "uml": "Error in processing of UML diagram",
     "unknown_external_keys": "Unknown keys found in external need data",
diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py
index 7eab592c..67267a7f 100644
--- a/sphinx_needs/needs.py
+++ b/sphinx_needs/needs.py
@@ -114,7 +114,7 @@ from sphinx_needs.roles.need_count import NeedCount, process_need_count
 from sphinx_needs.roles.need_func import NeedFunc, NeedFuncRole, process_need_func
 from sphinx_needs.roles.need_incoming import NeedIncoming, process_need_incoming
 from sphinx_needs.roles.need_outgoing import NeedOutgoing, process_need_outgoing
-from sphinx_needs.roles.need_part import NeedPart, process_need_part
+from sphinx_needs.roles.need_part import NeedPart, NeedPartRole, process_need_part
 from sphinx_needs.roles.need_ref import NeedRef, process_need_ref
 from sphinx_needs.services.github import GithubService
 from sphinx_needs.services.open_needs import OpenNeedsService
@@ -242,19 +242,8 @@ def setup(app: Sphinx) -> dict[str, Any]:
         ),
     )
 
-    app.add_role(
-        "need_part",
-        NeedsXRefRole(
-            nodeclass=NeedPart, innernodeclass=nodes.inline, warn_dangling=True
-        ),
-    )
-    # Shortcut for need_part
-    app.add_role(
-        "np",
-        NeedsXRefRole(
-            nodeclass=NeedPart, innernodeclass=nodes.inline, warn_dangling=True
-        ),
-    )
+    app.add_role("need_part", NeedPartRole())
+    app.add_role("np", NeedPartRole())  # Shortcut for need_part
 
     app.add_role(
         "need_count",
diff --git a/sphinx_needs/roles/need_part.py b/sphinx_needs/roles/need_part.py
index fc1d789c..f14dd82e 100644
--- a/sphinx_needs/roles/need_part.py
+++ b/sphinx_needs/roles/need_part.py
@@ -3,8 +3,6 @@ NeedPart module
 ---------------
 Provides the ability to mark specific parts of a need with an own id.
 
-Most voodoo is done in need.py
-
 """
 
 from __future__ import annotations
@@ -12,22 +10,65 @@ from __future__ import annotations
 import hashlib
 import re
 from collections.abc import Iterable
-from typing import cast
 
 from docutils import nodes
 from sphinx.application import Sphinx
 from sphinx.environment import BuildEnvironment
+from sphinx.util.docutils import SphinxRole
 from sphinx.util.nodes import make_refnode
 
 from sphinx_needs.data import NeedsInfoType, NeedsPartType
 from sphinx_needs.logging import get_logger, log_warning
 from sphinx_needs.nodes import Need
 
-log = get_logger(__name__)
+LOGGER = get_logger(__name__)
 
 
 class NeedPart(nodes.Inline, nodes.Element):
-    pass
+    @property
+    def title(self) -> str:
+        """Return the title of the part."""
+        return self.attributes["title"]  # type: ignore[no-any-return]
+
+    @property
+    def part_id(self) -> str:
+        """Return the ID of the part."""
+        return self.attributes["part_id"]  # type: ignore[no-any-return]
+
+    @property
+    def need_id(self) -> str | None:
+        """Return the ID of the need this part belongs to."""
+        return self.attributes["need_id"]  # type: ignore[no-any-return]
+
+    @need_id.setter
+    def need_id(self, value: str) -> None:
+        """Set the ID of the need this part belongs to."""
+        self.attributes["need_id"] = value
+
+
+_PART_PATTERN = re.compile(r"\(([\w-]+)\)(.*)", re.DOTALL)
+
+
+class NeedPartRole(SphinxRole):
+    """
+    Role for need parts, which are sub-needs of a need.
+    It is used to mark parts of a need with an own id.
+    """
+
+    def run(self) -> tuple[list[nodes.Node], list[nodes.system_message]]:
+        # note self.text is the content of the role, with backslash escapes removed
+        # TODO perhaps in a future change we should allow escaping parentheses in the part id?
+        # and also strip (unescaped) space before/after the title
+        result = _PART_PATTERN.match(self.text)
+        if result:
+            id_ = result.group(1)
+            title = result.group(2)
+        else:
+            id_ = hashlib.sha1(self.text.encode("UTF-8")).hexdigest().upper()[:3]
+            title = self.text
+        part = NeedPart(title=title, part_id=id_, need_id=None)
+        self.set_source_info(part)
+        return [part], []
 
 
 def process_need_part(
@@ -36,10 +77,16 @@ def process_need_part(
     fromdocname: str,
     found_nodes: list[nodes.Element],
 ) -> None:
-    pass
-
-
-part_pattern = re.compile(r"\(([\w-]+)\)(.*)", re.DOTALL)
+    # note this is called after needs have been processed and parts collected.
+    for node in found_nodes:
+        assert isinstance(node, NeedPart), "Expected NeedPart node"
+        if node.need_id is None:
+            log_warning(
+                LOGGER,
+                "Need part not associated with a need.",
+                "part",
+                node,
+            )
 
 
 def create_need_from_part(need: NeedsInfoType, part: NeedsPartType) -> NeedsInfoType:
@@ -67,58 +114,43 @@ def update_need_with_parts(
 ) -> None:
     app = env.app
     for part_node in part_nodes:
-        content = cast(str, part_node.children[0].children[0])  # ->inline->Text
-        result = part_pattern.match(content)
-        if result:
-            inline_id = result.group(1)
-            part_content = result.group(2)
-        else:
-            part_content = content
-            inline_id = (
-                hashlib.sha1(part_content.encode("UTF-8")).hexdigest().upper()[:3]
-            )
+        part_id = part_node.part_id
 
         if "parts" not in need:
             need["parts"] = {}
 
-        if inline_id in need["parts"]:
+        if part_id in need["parts"]:
             log_warning(
-                log,
+                LOGGER,
                 "part_need id {} in need {} is already taken. need_part may get overridden.".format(
-                    inline_id, need["id"]
+                    part_id, need["id"]
                 ),
                 "duplicate_part_id",
                 part_node,
             )
 
-        need["parts"][inline_id] = {
-            "id": inline_id,
-            "content": part_content,
+        need["parts"][part_id] = {
+            "id": part_id,
+            "content": part_node.title,
             "links": [],
             "links_back": [],
         }
 
-        part_id_ref = "{}.{}".format(need["id"], inline_id)
-
+        part_node.need_id = need["id"]
+        part_id_ref = "{}.{}".format(need["id"], part_id)
         part_node["reftarget"] = part_id_ref
 
-        part_text_node = nodes.Text(part_content)
-
-        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(nodes.Text(part_node.title))
 
         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
+                app.builder, docname, docname, part_id_ref, nodes.Text(f" {part_id}")
             )
             part_ref_node["classes"] += ["needs-id"]
             node_need_part_line.append(part_ref_node)
 
+        part_node.children = []
         part_node.append(node_need_part_line)
 
 
diff --git a/sphinx_needs/roles/need_ref.py b/sphinx_needs/roles/need_ref.py
index 3e3c26d7..ba44859a 100644
--- a/sphinx_needs/roles/need_ref.py
+++ b/sphinx_needs/roles/need_ref.py
@@ -85,7 +85,24 @@ def process_need_ref(
         need_id_full = node_need_ref["reftarget"]
         need_id_main, need_id_part = split_need_id(need_id_full)
 
-        if need_id_main in all_needs:
+        if need_id_main not in all_needs:
+            log_warning(
+                log,
+                f"linked need {node_need_ref['reftarget']} not found",
+                "link_ref",
+                location=node_need_ref,
+            )
+        elif (
+            need_id_part is not None
+            and need_id_part not in all_needs[need_id_main]["parts"]
+        ):
+            log_warning(
+                log,
+                f"linked need part {node_need_ref['reftarget']} not found",
+                "link_ref",
+                location=node_need_ref,
+            )
+        else:
             target_need = all_needs[need_id_main]
 
             dict_need = transform_need_to_dict(
@@ -162,12 +179,4 @@ def process_need_ref(
                     )
                     new_node_ref["classes"].append(target_need["external_css"])
 
-        else:
-            log_warning(
-                log,
-                f"linked need {node_need_ref['reftarget']} not found",
-                "link_ref",
-                location=node_need_ref,
-            )
-
         node_need_ref.replace_self(new_node_ref)


Test patch:
diff --git a/tests/doc_test/doc_need_parts/index.rst b/tests/doc_test/doc_need_parts/index.rst
index 1a407365..9e4fcf86 100644
--- a/tests/doc_test/doc_need_parts/index.rst
+++ b/tests/doc_test/doc_need_parts/index.rst
@@ -23,6 +23,7 @@ NEED PARTS
 
         Part in nested need: :need_part:`(nested_id)something`
 
+:np:`hallo`
 
 :need:`SP_TOO_001.1`
 
@@ -31,3 +32,5 @@ NEED PARTS
 :need:`SP_TOO_001.awesome_id`
 
 :need:`My custom link name <SP_TOO_001.awesome_id>`
+
+:need:`SP_TOO_001.unknown_part`
diff --git a/tests/test_need_parts.py b/tests/test_need_parts.py
index ba0b9479..957aaff4 100644
--- a/tests/test_need_parts.py
+++ b/tests/test_need_parts.py
@@ -19,7 +19,10 @@ def test_doc_need_parts(test_app, snapshot):
         app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/")
     ).splitlines()
     # print(warnings)
-    assert warnings == []
+    assert warnings == [
+        "srcdir/index.rst:26: WARNING: Need part not associated with a need. [needs.part]",
+        "srcdir/index.rst:36: WARNING: linked need part SP_TOO_001.unknown_part not found [needs.link_ref]",
+    ]
 
     html = Path(app.outdir, "index.html").read_text()
     assert (