Repository: useblocks/sphinx-needs
Pull request #1266 (2024-09-02 11:35:52)

👌 pass parent need to `need_func`
Currently, the `need_func` role passes a "dummy need" to dynamic functions, which would except for most dynamic functions. So:

1. we replace the dummy need with `None` and ensure dynamic functions account for this input.
2. when a `need_func` is used within a need directive, it uses that need as input for the dynamic function, rather than `None`

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) Report
Attention: Patch coverage is `79.66102%` with `12 lines` in your changes missing coverage. Please review.
> Project coverage is 86.84%. 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 [(`fb31623`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/fb31623ede5d0c1d92c5bee55087d2b85d17891c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).
> Report is 28 commits behind head on master.

| [Files with missing lines](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | Patch % | Lines |
|---|---|---|
| [sphinx\_needs/functions/common.py](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?src=pr&el=tree&filepath=sphinx_needs%2Ffunctions%2Fcommon.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#diff-c3BoaW54X25lZWRzL2Z1bmN0aW9ucy9jb21tb24ucHk=) | 56.25% | [7 Missing :warning: ](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) |
| [sphinx\_needs/functions/functions.py](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?src=pr&el=tree&filepath=sphinx_needs%2Ffunctions%2Ffunctions.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#diff-c3BoaW54X25lZWRzL2Z1bmN0aW9ucy9mdW5jdGlvbnMucHk=) | 87.50% | [5 Missing :warning: ](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) |

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


```diff
@@            Coverage Diff             @@
##           master    #1266      +/-   ##
==========================================
- Coverage   86.87%   86.84%   -0.04%     
==========================================
  Files          56       60       +4     
  Lines        6532     6938     +406     
==========================================
+ Hits         5675     6025     +350     
- Misses        857      913      +56     
```

| [Flag](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1266/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/1266/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | `86.84% <79.66%> (-0.04%)` | :arrow_down: |

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/1266?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).


Patch:
diff --git a/sphinx_needs/functions/common.py b/sphinx_needs/functions/common.py
index 6e2b6884..a047adef 100644
--- a/sphinx_needs/functions/common.py
+++ b/sphinx_needs/functions/common.py
@@ -22,7 +22,7 @@ from sphinx_needs.utils import logger
 
 def test(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     *args: Any,
     **kwargs: Any,
@@ -44,12 +44,13 @@ def test(
 
     :return: single test string
     """
-    return f"Test output of need {need['id']}. args: {args}. kwargs: {kwargs}"
+    need_id = "none" if need is None else need["id"]
+    return f"Test output of need_func; need: {need_id}; args: {args}; kwargs: {kwargs}"
 
 
 def echo(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     text: str,
     *args: Any,
@@ -73,7 +74,7 @@ def echo(
 
 def copy(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     option: str,
     need_id: str | None = None,
@@ -139,7 +140,7 @@ def copy(
         .. test:: test of current_need value
            :id: copy_4
 
-           The es the title of the first need found under the same highest
+           The following copy command copies the title of the first need found under the same  highest
            section (headline):
 
            [[copy('title', filter='current_need["sections"][-1]==sections[-1]')]]
@@ -150,7 +151,7 @@ def copy(
        The following copy command copies the title of the first need found under the same  highest
        section (headline):
 
-            [[copy('title', filter='current_need["sections"][-1]==sections[-1]')]]
+       [[copy('title', filter='current_need["sections"][-1]==sections[-1]')]]
 
     This filter possibilities get really powerful in combination with :ref:`needs_global_options`.
 
@@ -171,26 +172,32 @@ def copy(
             NeedsSphinxConfig(app.config),
             filter,
             need,
-            location=(need["docname"], need["lineno"]) if need["docname"] else None,
+            location=(need["docname"], need["lineno"])
+            if need and need["docname"]
+            else None,
         )
         if result:
             need = result[0]
 
-    value = need[option]  # type: ignore[literal-required]
+    if need is None:
+        raise ValueError("Need not found")
+
+    if option not in need:
+        raise ValueError(f"Option {option} not found in need {need['id']}")
 
-    # TODO check if str?
+    value = need[option]  # type: ignore[literal-required]
 
     if lower:
-        return value.lower()
+        return str(value).lower()
     if upper:
-        return value.upper()
+        return str(value).upper()
 
     return value
 
 
 def check_linked_values(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     result: Any,
     search_option: str,
@@ -329,6 +336,9 @@ def check_linked_values(
     :param one_hit: If True, only one linked need must have a positive check
     :return: result, if all checks are positive
     """
+    if need is None:
+        raise ValueError("No need given for check_linked_values")
+
     needs_config = NeedsSphinxConfig(app.config)
     links = need["links"]
     if not isinstance(search_value, list):
@@ -359,7 +369,7 @@ def check_linked_values(
 
 def calc_sum(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     option: str,
     filter: str | None = None,
@@ -444,6 +454,9 @@ def calc_sum(
 
     :return: A float number
     """
+    if need is None:
+        raise ValueError("No need given for check_linked_values")
+
     needs_config = NeedsSphinxConfig(app.config)
     check_needs = (
         [needs[link] for link in need["links"]] if links_only else needs.values()
@@ -471,7 +484,7 @@ def calc_sum(
 
 def links_from_content(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     needs: NeedsView,
     need_id: str | None = None,
     filter: str | None = None,
@@ -529,6 +542,9 @@ def links_from_content(
     """
     source_need = needs[need_id] if need_id else need
 
+    if source_need is None:
+        raise ValueError("No need found for links_from_content")
+
     links = re.findall(r":need:`(\w+)`|:need:`.+\<(.+)\>`", source_need["content"])
     raw_links = []
     for link in links:
diff --git a/sphinx_needs/functions/functions.py b/sphinx_needs/functions/functions.py
index 34c1cd3e..38926a40 100644
--- a/sphinx_needs/functions/functions.py
+++ b/sphinx_needs/functions/functions.py
@@ -23,6 +23,7 @@ from sphinx_needs.data import NeedsInfoType, NeedsMutable, NeedsView, SphinxNeed
 from sphinx_needs.debug import measure_time_func
 from sphinx_needs.logging import get_logger, log_warning
 from sphinx_needs.nodes import Need
+from sphinx_needs.roles.need_func import NeedFunc
 from sphinx_needs.utils import NEEDS_FUNCTIONS, match_variants
 
 logger = get_logger(__name__)
@@ -38,7 +39,7 @@ class DynamicFunction(Protocol):
     def __call__(
         self,
         app: Sphinx,
-        need: NeedsInfoType,
+        need: NeedsInfoType | None,
         needs: NeedsView,
         *args: Any,
         **kwargs: Any,
@@ -76,7 +77,7 @@ def register_func(need_function: DynamicFunction, name: str | None = None) -> No
 
 def execute_func(
     app: Sphinx,
-    need: NeedsInfoType,
+    need: NeedsInfoType | None,
     func_string: str,
     location: str | tuple[str | None, int | None] | nodes.Node | None,
 ) -> str | int | float | list[str] | list[int] | list[float] | None:
@@ -112,13 +113,23 @@ def execute_func(
     func = measure_time_func(
         NEEDS_FUNCTIONS[func_name]["function"], category="dyn_func", source="user"
     )
-    func_return = func(
-        app,
-        need,
-        SphinxNeedsData(app.env).get_needs_view(),
-        *func_args,
-        **func_kwargs,
-    )
+
+    try:
+        func_return = func(
+            app,
+            need,
+            SphinxNeedsData(app.env).get_needs_view(),
+            *func_args,
+            **func_kwargs,
+        )
+    except Exception as e:
+        log_warning(
+            logger,
+            f"Error while executing function {func_name!r}: {e}",
+            "dynamic_function",
+            location=location,
+        )
+        return "??"
 
     if func_return is not None and not isinstance(func_return, (str, int, float, list)):
         log_warning(
@@ -152,10 +163,11 @@ def find_and_replace_node_content(
     if found, check if it contains a function string and run/replace it.
 
     :param node: Node to analyse
-    :return: None
     """
     new_children = []
-    if (
+    if isinstance(node, NeedFunc):
+        return node.get_text(env, need)
+    elif (
         not node.children
         and isinstance(node, nodes.Text)
         or isinstance(node, nodes.reference)
@@ -360,7 +372,10 @@ def resolve_variants_options(
 
 
 def check_and_get_content(
-    content: str, need: NeedsInfoType, env: BuildEnvironment, location: nodes.Node
+    content: str,
+    need: NeedsInfoType | None,
+    env: BuildEnvironment,
+    location: nodes.Node,
 ) -> str:
     """
     Checks if the given content is a function call.
@@ -373,12 +388,6 @@ def check_and_get_content(
     :param location: source location of the function call
     :return: string
     """
-
-    try:
-        content = str(content)
-    except UnicodeEncodeError:
-        content = content.encode("utf-8")  # type: ignore
-
     func_match = func_pattern.search(content)
     if func_match is None:
         return content
diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py
index 116a5297..930c8732 100644
--- a/sphinx_needs/needs.py
+++ b/sphinx_needs/needs.py
@@ -100,7 +100,7 @@ from sphinx_needs.functions import NEEDS_COMMON_FUNCTIONS, register_func
 from sphinx_needs.logging import get_logger, log_warning
 from sphinx_needs.roles import NeedsXRefRole
 from sphinx_needs.roles.need_count import NeedCount, process_need_count
-from sphinx_needs.roles.need_func import NeedFunc, process_need_func
+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
@@ -253,12 +253,7 @@ def setup(app: Sphinx) -> dict[str, Any]:
         ),
     )
 
-    app.add_role(
-        "need_func",
-        NeedsXRefRole(
-            nodeclass=NeedFunc, innernodeclass=nodes.inline, warn_dangling=True
-        ),
-    )
+    app.add_role("need_func", NeedFuncRole())
 
     ########################################################################
     # EVENTS
diff --git a/sphinx_needs/roles/need_func.py b/sphinx_needs/roles/need_func.py
index 4e921823..16c7b364 100644
--- a/sphinx_needs/roles/need_func.py
+++ b/sphinx_needs/roles/need_func.py
@@ -6,16 +6,35 @@ from __future__ import annotations
 
 from docutils import nodes
 from sphinx.application import Sphinx
+from sphinx.environment import BuildEnvironment
+from sphinx.util.docutils import SphinxRole
 
 from sphinx_needs.data import NeedsInfoType
-from sphinx_needs.functions.functions import check_and_get_content
 from sphinx_needs.logging import get_logger
+from sphinx_needs.utils import add_doc
 
 log = get_logger(__name__)
 
 
+class NeedFuncRole(SphinxRole):
+    """Role for creating ``NeedFunc`` node."""
+
+    def run(self) -> tuple[list[nodes.Node], list[nodes.system_message]]:
+        add_doc(self.env, self.env.docname)
+        node = NeedFunc(
+            self.rawtext, nodes.literal(self.rawtext, self.text), **self.options
+        )
+        self.set_source_info(node)
+        return [node], []
+
+
 class NeedFunc(nodes.Inline, nodes.Element):
-    pass
+    def get_text(self, env: BuildEnvironment, need: NeedsInfoType | None) -> nodes.Text:
+        """Execute function and return result."""
+        from sphinx_needs.functions.functions import check_and_get_content
+
+        result = check_and_get_content(self.astext(), need, env, self)
+        return nodes.Text(str(result))
 
 
 def process_need_func(
@@ -24,12 +43,7 @@ def process_need_func(
     _fromdocname: str,
     found_nodes: list[nodes.Element],
 ) -> None:
-    env = app.env
-    # for node_need_func in doctree.findall(NeedFunc):
-    dummy_need: NeedsInfoType = {"id": "need_func_dummy"}  # type: ignore[typeddict-item]
-    for node_need_func in found_nodes:
-        result = check_and_get_content(
-            node_need_func.attributes["reftarget"], dummy_need, env, node_need_func
-        )
-        new_node_func = nodes.Text(str(result))
+    node_need_func: NeedFunc
+    for node_need_func in found_nodes:  # type: ignore[assignment]
+        new_node_func = node_need_func.get_text(app.env, None)
         node_need_func.replace_self(new_node_func)


Test patch:
diff --git a/tests/doc_test/doc_dynamic_functions/index.rst b/tests/doc_test/doc_dynamic_functions/index.rst
index 71f3705e..dd03f881 100644
--- a/tests/doc_test/doc_dynamic_functions/index.rst
+++ b/tests/doc_test/doc_dynamic_functions/index.rst
@@ -8,6 +8,8 @@ DYNAMIC FUNCTIONS
 
     This is id [[copy("id")]]
 
+    This is also id :need_func:`[[copy("id")]]`
+
 .. spec:: TEST_2
    :id: TEST_2
    :tags: my_tag; [[copy("tags", "SP_TOO_001")]]
@@ -20,6 +22,8 @@ DYNAMIC FUNCTIONS
     :id: TEST_4
     :tags: test_4a;test_4b;[[copy('title')]]
 
+    Test dynamic func in tags: [[copy("tags")]]
+
 .. spec:: TEST_5
     :id: TEST_5
     :tags: [[copy('id')]]
@@ -30,3 +34,7 @@ DYNAMIC FUNCTIONS
         :id: TEST_6
 
         nested id [[copy('id')]]
+
+        nested id also :need_func:`[[copy("id")]]`
+
+This should warn since it has no associated need: :need_func:`[[copy("id")]]`
diff --git a/tests/test_dynamic_functions.py b/tests/test_dynamic_functions.py
index e4652c73..6de4265c 100644
--- a/tests/test_dynamic_functions.py
+++ b/tests/test_dynamic_functions.py
@@ -25,10 +25,13 @@ def test_doc_dynamic_functions(test_app):
     warnings = strip_colors(
         app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/")
     ).splitlines()
-    assert warnings == []
+    assert warnings == [
+        "srcdir/index.rst:40: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]"
+    ]
 
     html = Path(app.outdir, "index.html").read_text()
     assert "This is id SP_TOO_001" in html
+    assert "This is also id SP_TOO_001" in html
 
     assert (
         sum(1 for _ in re.finditer('<span class="needs_data">test2</span>', html)) == 2
@@ -56,11 +59,14 @@ def test_doc_dynamic_functions(test_app):
         sum(1 for _ in re.finditer('<span class="needs_data">TEST_5</span>', html)) == 2
     )
 
-    assert "Test output of need TEST_3. args:" in html
+    assert "Test output of need_func; need: TEST_3" in html
+
+    assert "Test dynamic func in tags: test_4a, test_4b, TEST_4" in html
 
     assert '<a class="reference external" href="http://www.TEST_5">link</a>' in html
 
     assert "nested id TEST_6" in html
+    assert "nested id also TEST_6" in html
 
 
 @pytest.mark.parametrize(
diff --git a/tests/test_global_options.py b/tests/test_global_options.py
index 9829f1a7..d5d1e19b 100644
--- a/tests/test_global_options.py
+++ b/tests/test_global_options.py
@@ -21,7 +21,7 @@ def test_doc_global_option(test_app):
 
     assert "test_global" in html
     assert "1.27" in html
-    assert "Test output of need GLOBAL_ID" in html
+    assert "Test output of need_func; need: GLOBAL_ID" in html
 
     assert "STATUS_IMPL" in html
     assert "STATUS_UNKNOWN" in html