✨ Add ndf role, deprecate need_func & [[...]] in need content (#1269)

There were previously two methods to use dynamic functions within need directive's content:

1. ``[[copy("id")]]``; is problematic, because it does not adhere to the rst / myst syntax specification, and has already shown to cause parsing issues and be surprising to users

2. `` :need_func:`[[copy("id")]]` ``; is better but overly verbose

In this commit, these are replaced by the role: `` :ndf:`copy("id")` ``
Here we take the entire content to be the function, as so do not require the `[[]]`, reducing verbosity and processing

The other two methods now emit a deprecation warning, showing how to convert them to the new role.

Author: chrisjsewell

docs/directives/needextend.rst

@@ -35,8 +35,8 @@ Also, you can add links or delete tags.
 
       This requirement got modified.
 
-      | Status was **open**, now it is **[[copy('status')]]**.
-      | Also author got changed from **Foo** to **[[copy('author')]]**.
+      | Status was **open**, now it is :ndf:`copy('status')`.
+      | Also author got changed from **Foo** to :ndf:`copy('author')`.
       | And a tag was added.
       | Finally all links got removed.
 

docs/dynamic_functions.rst

@@ -3,32 +3,35 @@
 Dynamic functions
 =================
 
-**Sphinx-Needs** provides a mechanism to set dynamic data for need-options during generation.
-We do this by giving an author the possibility to set a function call to a predefined function, which calculates
-the final result/value for the option.
+Dynamic functions provide a mechanism to specify need fields or content that are calculated at build time, based on other fields or needs.
+
+We do this by giving an author the possibility to set a function call to a predefined function, which calculates the final value **after all needs have been collected**.
 
 For instance, you can use the feature if the status of a requirement depends on linked test cases and their status.
 Or if you will request specific data from an external server like JIRA.
 
-**needtable**
-
-The options :ref:`needtable_style_row` of :ref:`needtable` also support
-dynamic function execution. In this case, the function gets executed with the found need for each row.
-
-This allows you to set row and column specific styles such as, set a row background to red, if a need-status is *failed*.
+To refer to a dynamic function, you can use the following syntax:
 
+- In a need directive option, wrap the function call in double square brackets: ``function_name(arg)``
+- In a need content, use the :ref:`ndf` role: ``:ndf:\`function_name(arg)\```
 
 .. need-example:: Dynamic function example
 
    .. req:: my test requirement
       :id: df_1
       :status: open
+      :tags: test;[[copy("status")]]
 
-      This need has id **[[copy("id")]]** and status **[[copy("status")]]**.
+      This need has id :ndf:`copy("id")` and status :ndf:`copy("status")`.
+
+.. deprecated:: 3.1.0
+
+   The :ref:`ndf` role replaces the use of the ``[[...]]`` syntax in need content.
 
 Built-in functions
 -------------------
-The following functions are available in all **Sphinx-Needs** installations.
+
+The following functions are available by default.
 
 .. note::
 

docs/needs_templates/spec_template.need

@@ -19,5 +19,5 @@ Tags:
 {# by using dynamic_functions #}
 Links:
 {% for link in links %}
-| **{{link}}**: [[copy('title', '{{link}}')]] ([[copy('type_name', '{{link}}')]])
+| **{{link}}**: :ndf:`copy('title', '{{link}}')` (:ndf:`copy('type_name', '{{link}}')`)
 {%- endfor %}

docs/roles.rst

@@ -188,10 +188,18 @@ To calculate the ratio of one filter to another filter, you can define two filte
 
 need_func
 ---------
-.. versionadded:: 0.6.3
+.. deprecated:: 3.1.0
 
-Executes :ref:`dynamic_functions` and uses the return values as content.
+   Use :ref:`ndf` instead.
+
+.. _ndf:
+
+ndf
+---
+.. versionadded:: 3.1.0
+
+Executes a :ref:`need dynamic function <dynamic_functions>` and uses the return values as content.
 
 .. need-example::
 
-    A nice :need_func:`[[echo("first")]] test` for need_func.
+    A nice :ndf:`echo("first test")` for dynamic functions.

sphinx_needs/directives/needextend.py

@@ -71,7 +71,10 @@ def run(self) -> Sequence[nodes.Node]:
 
         add_doc(env, env.docname)
 
-        return [targetnode, Needextend("")]
+        node = Needextend("")
+        self.set_source_info(node)
+
+        return [targetnode, node]
 
 
 RE_ID_FUNC = re.compile(r"\s*((?P<function>\[\[[^\]]*\]\])|(?P<id>[^;,]+))\s*([;,]|$)")

sphinx_needs/directives/needextract.py

@@ -196,14 +196,15 @@ def _build_needextract(
 
     dummy_need.extend(need_node.children)
 
+    find_and_replace_node_content(dummy_need, env, need_data)
+
     # resolve_references() ignores the given docname and takes the docname from the pending_xref node.
     # Therefore, we need to manipulate this first, before we can ask Sphinx to perform the normal
     # reference handling for us.
     _replace_pending_xref_refdoc(dummy_need, extract_data["docname"])
     env.resolve_references(dummy_need, extract_data["docname"], app.builder)  # type: ignore[arg-type]
 
     dummy_need.attributes["ids"].append(need_data["id"])
-    find_and_replace_node_content(dummy_need, env, need_data)
     rendered_node = build_need_repr(
         dummy_need,  # type: ignore[arg-type]
         need_data,

sphinx_needs/functions/common.py

@@ -39,16 +39,16 @@ def test(
 
         .. req:: test requirement
 
-            [[test('arg_1', [1,2,3], my_keyword='awesome')]]
+            :ndf:`test('arg_1', [1,2,3], my_keyword='awesome')`
 
     .. req:: test requirement
 
-        [[test('arg_1', [1,2,3], my_keyword='awesome')]]
+        :ndf:`test('arg_1', [1,2,3], my_keyword='awesome')`
 
     :return: single test string
     """
     need_id = "none" if need is None else need["id"]
-    return f"Test output of need_func; need: {need_id}; args: {args}; kwargs: {kwargs}"
+    return f"Test output of dynamic function; need: {need_id}; args: {args}; kwargs: {kwargs}"
 
 
 def echo(
@@ -67,9 +67,9 @@ def echo(
 
     .. code-block:: jinja
 
-       A nice :need_func:`[[echo("first")]] test` for need_func.
+       A nice :ndf:`echo("first test")` for a dynamic function.
 
-    **Result**: A nice :need_func:`[[echo("first")]] test` for need_func.
+    **Result**: A nice :ndf:`echo("first test")` for a dynamic function.
 
     """
     return text
@@ -146,15 +146,15 @@ 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]')]]
+           :ndf:`copy('title', filter='current_need["sections"][-1]==sections[-1]')`
 
     .. test:: test of current_need value
        :id: copy_4
 
        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]')]]
+       :ndf:`copy('title', filter='current_need["sections"][-1]==sections[-1]')`
 
     This filter possibilities get really powerful in combination with :ref:`needs_global_options`.
 

sphinx_needs/functions/functions.py

@@ -152,7 +152,7 @@ def execute_func(
     return func_return
 
 
-func_pattern = re.compile(r"\[\[(.*?)\]\]")  # RegEx to detect function strings
+FUNC_RE = re.compile(r"\[\[(.*?)\]\]")  # RegEx to detect function strings
 
 
 def find_and_replace_node_content(
@@ -163,6 +163,9 @@ def find_and_replace_node_content(
     if found, check if it contains a function string and run/replace it.
 
     :param node: Node to analyse
+    :param env: Sphinx environment
+    :param need: Need data
+    :param extract: If True, the function has been called from a needextract node
     """
     new_children = []
     if isinstance(node, NeedFunc):
@@ -181,7 +184,7 @@ def find_and_replace_node_content(
                 return node
         else:
             new_text = node
-        func_match = func_pattern.findall(new_text)
+        func_match = FUNC_RE.findall(new_text)
         for func_string in func_match:
             # sphinx is replacing ' and " with language specific quotation marks (up and down), which makes
             # it impossible for the later used AST render engine to detect a python function call in the given
@@ -194,6 +197,10 @@ def find_and_replace_node_content(
 
             func_string = func_string.replace("‘", "'")  # noqa: RUF001
             func_string = func_string.replace("’", "'")  # noqa: RUF001
+
+            msg = f"The [[{func_string}]] syntax in need content is deprecated. Replace with :ndf:`{func_string}` instead."
+            log_warning(logger, msg, "deprecation", location=node)
+
             func_return = execute_func(env.app, need, func_string, node)
 
             if isinstance(func_return, list):
@@ -388,7 +395,7 @@ def check_and_get_content(
     :param location: source location of the function call
     :return: string
     """
-    func_match = func_pattern.search(content)
+    func_match = FUNC_RE.search(content)
     if func_match is None:
         return content
 
@@ -416,7 +423,7 @@ def _detect_and_execute_field(
     except UnicodeEncodeError:
         content = content.encode("utf-8")
 
-    func_match = func_pattern.search(content)
+    func_match = FUNC_RE.search(content)
     if func_match is None:
         return None, None
 

sphinx_needs/needs.py

@@ -254,7 +254,8 @@ def setup(app: Sphinx) -> dict[str, Any]:
         ),
     )
 
-    app.add_role("need_func", NeedFuncRole())
+    app.add_role("need_func", NeedFuncRole(with_brackets=True))  # deprecrated
+    app.add_role("ndf", NeedFuncRole(with_brackets=False))
 
     ########################################################################
     # EVENTS

sphinx_needs/roles/need_func.py

@@ -1,5 +1,5 @@
 """
-Provide the role ``need_func``, which executes a dynamic function.
+Provide a role which executes a dynamic function.
 """
 
 from __future__ import annotations
@@ -10,30 +10,64 @@
 from sphinx.util.docutils import SphinxRole
 
 from sphinx_needs.data import NeedsInfoType
-from sphinx_needs.logging import get_logger
+from sphinx_needs.logging import get_logger, log_warning
 from sphinx_needs.utils import add_doc
 
-log = get_logger(__name__)
+LOGGER = get_logger(__name__)
 
 
 class NeedFuncRole(SphinxRole):
     """Role for creating ``NeedFunc`` node."""
 
+    def __init__(self, *, with_brackets: bool = False) -> None:
+        """Initialize the role.
+
+        :param with_brackets: If True, the function is expected to be wrapped in brackets ``[[]]``.
+        """
+        self.with_brackets = with_brackets
+        super().__init__()
+
     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.rawtext,
+            nodes.literal(self.rawtext, self.text),
+            with_brackets=self.with_brackets,
+            **self.options,
         )
         self.set_source_info(node)
+        if self.with_brackets:
+            from sphinx_needs.functions.functions import FUNC_RE
+
+            msg = "The `need_func` role is deprecated. "
+            if func_match := FUNC_RE.search(node.astext()):
+                func_call = func_match.group(1)
+                msg += f"Replace with :ndf:`{func_call}` instead."
+            else:
+                msg += "Replace with ndf role instead."
+            log_warning(LOGGER, msg, "deprecation", location=node)
         return [node], []
 
 
 class NeedFunc(nodes.Inline, nodes.Element):
+    @property
+    def with_brackets(self) -> bool:
+        """Return the function with brackets."""
+        return self.get("with_brackets", False)  # type: ignore[no-any-return]
+
     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
+        from sphinx_needs.functions.functions import check_and_get_content, execute_func
+
+        if not self.with_brackets:
+            func_return = execute_func(env.app, need, self.astext(), self)
+            if isinstance(func_return, list):
+                func_return = ", ".join(str(el) for el in func_return)
+
+            return nodes.Text("" if func_return is None else str(func_return))
 
         result = check_and_get_content(self.astext(), need, env, self)
+
         return nodes.Text(str(result))
 
 

tests/doc_test/doc_dynamic_functions/index.rst

@@ -10,6 +10,8 @@ DYNAMIC FUNCTIONS
 
     This is also id :need_func:`[[copy("id")]]`
 
+    This is the best id :ndf:`copy("id")`
+
 .. spec:: TEST_2
    :id: TEST_2
    :tags: my_tag; [[copy("tags", "SP_TOO_001")]]
@@ -37,4 +39,6 @@ DYNAMIC FUNCTIONS
 
         nested id also :need_func:`[[copy("id")]]`
 
-This should warn since it has no associated need: :need_func:`[[copy("id")]]`
+        nested id best :ndf:`copy("id")`
+
+These should warn since they have no associated need: :need_func:`[[copy("id")]]`, :ndf:`copy("id")`

tests/doc_test/needextract_with_nested_needs/index.rst

@@ -10,7 +10,7 @@ Test
 
    Another, child spec
 
-   This is id [[copy("id")]]
+   This is id [[copy("id")]] :ndf:`copy("id")`
 
    .. spec:: Child spec
      :id: SPEC_1_1
@@ -30,6 +30,6 @@ Test
 
          awesome grandchild spec number 2.
 
-         This is grandchild id [[copy("id")]]
+         This is grandchild id [[copy("id")]] :ndf:`copy("id")`
 
    Some parent text

tests/test_dynamic_functions.py

@@ -26,12 +26,21 @@ def test_doc_dynamic_functions(test_app):
         app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/")
     ).splitlines()
     assert warnings == [
-        "srcdir/index.rst:40: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]"
+        'srcdir/index.rst:11: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]',
+        'srcdir/index.rst:40: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]',
+        'srcdir/index.rst:44: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]',
+        'srcdir/index.rst:9: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]',
+        'srcdir/index.rst:27: WARNING: The [[copy("tags")]] syntax in need content is deprecated. Replace with :ndf:`copy("tags")` instead. [needs.deprecation]',
+        "srcdir/index.rst:33: WARNING: The [[copy('id')]] syntax in need content is deprecated. Replace with :ndf:`copy('id')` instead. [needs.deprecation]",
+        "srcdir/index.rst:38: WARNING: The [[copy('id')]] syntax in need content is deprecated. Replace with :ndf:`copy('id')` instead. [needs.deprecation]",
+        "srcdir/index.rst:44: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]",
+        "srcdir/index.rst:44: 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 "This is the best id SP_TOO_001" in html
 
     assert (
         sum(1 for _ in re.finditer('<span class="needs_data">test2</span>', html)) == 2
@@ -59,14 +68,15 @@ 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_func; need: TEST_3" in html
+    assert "Test output of dynamic function; 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
+    assert "nested id best TEST_6" in html
 
 
 @pytest.mark.parametrize(
@@ -118,8 +128,12 @@ def test_doc_df_user_functions(test_app):
     # print(warnings)
     expected = [
         "srcdir/index.rst:10: WARNING: Return value of function 'bad_function' is of type <class 'object'>. Allowed are str, int, float, list [needs.dynamic_function]",
+        "srcdir/index.rst:8: WARNING: The [[my_own_function()]] syntax in need content is deprecated. Replace with :ndf:`my_own_function()` instead. [needs.deprecation]",
+        "srcdir/index.rst:14: WARNING: The [[bad_function()]] syntax in need content is deprecated. Replace with :ndf:`bad_function()` instead. [needs.deprecation]",
         "srcdir/index.rst:14: WARNING: Return value of function 'bad_function' is of type <class 'object'>. Allowed are str, int, float, list [needs.dynamic_function]",
+        "srcdir/index.rst:16: WARNING: The [[invalid]] syntax in need content is deprecated. Replace with :ndf:`invalid` instead. [needs.deprecation]",
         "srcdir/index.rst:16: WARNING: Function string 'invalid' could not be parsed: Given dynamic function string is not a valid python call. Got: invalid [needs.dynamic_function]",
+        "srcdir/index.rst:18: WARNING: The [[unknown()]] syntax in need content is deprecated. Replace with :ndf:`unknown()` instead. [needs.deprecation]",
         "srcdir/index.rst:18: WARNING: Unknown function 'unknown' [needs.dynamic_function]",
     ]
     if version_info >= (7, 3):