🐛 sphinx-needs: an empty need.FIELD selector fails the call, as an unset one does

packages/sphinx-needs/docs/changelog.rst

@@ -51,7 +51,7 @@ Improvements
 
 - ✨ Dynamic functions and variants are computed in dependency order,
   and a value that cannot be computed is reported **(changed output)**
-  (:issue:`2064`, :pr:`2080`, :pr:`2081`, :pr:`2134`)
+  (:issue:`2064`, :pr:`2080`, :pr:`2081`, :pr:`2134`, :pr:`2136`)
 
   A ``[[…]]``, ``<<…>>`` or ``<{…}>`` is now computed after every value it reads,
   so a chain of :ref:`copy <copy>` calls, a :ref:`calc_sum <calc_sum>` over computed summands,
@@ -95,7 +95,7 @@ Improvements
   (:ref:`argument <dynamic_functions_need_arguments>`);
   a need that used one in its own field was not created,
   and a ``needextend`` value that used one was dropped with a ``needs.needextend`` warning.
-  One that selects what its call reads fails the call when its field is unset.
+  One that selects what its call reads fails the call when its field is unset or empty.
   A ``None`` result, such as a ``copy`` of an unset field or a failed ``check_linked_values``,
   adds nothing to a field: alone in a nullable string or array field it leaves the field unset,
   where it was stored as the text ``"None"``, or as ``[None]``, which schema validation then refused.

packages/sphinx-needs/docs/dynamic_functions.rst

@@ -64,10 +64,11 @@ its field must be final before the call is computed (see :ref:`needs_processing_
 written in the need or by a ``needextend``, or a link field read from any field that is not one.
 Otherwise the call is not run, its field holds what a field on a cycle holds
 (:ref:`needs_derive_cycle`), and ``needs.derive_scope`` says so (:ref:`needs_derive_scope`).
-If the field is unset, the call fails (``needs.dynamic_function``):
+If the field is unset or empty, the call fails (``needs.dynamic_function``):
 ``copy`` does not read the need it is in instead.
 Any other argument (such as ``check_linked_values``' ``result`` or ``search_value``, or ``copy``'s ``upper``)
-is a value: its field is read like any other, after it is computed, and is ``None`` when unset.
+is a value: its field is read like any other, after it is computed, and is ``None`` when unset
+(``""`` for an untyped extra option).
 
 .. versionchanged:: 9.0.0
 

packages/sphinx-needs/src/sphinx_needs/functions/order.py

@@ -98,6 +98,8 @@ def _parameters(func: Callable[..., Any]) -> tuple[str, ...]:
     name: _parameters(func) for name, func in BUILTINS.items()
 }
 _VARARGS: Final = frozenset({"test", "echo"})
+#: a selector holding one of these is unset: the call fails
+_UNSET: Final = (None, "")
 
 
 def _arguments(call: DynamicFunctionParsed) -> dict[str, Any] | None:
@@ -119,9 +121,9 @@ def unset_selector(
     """The first ``need.<field>`` selector of a built-in call that ``need`` leaves unset.
 
     A selector is an argument that selects what the call reads (an id, a field name, a
-    filter, ``links_only``). A call with an unset one fails: it would otherwise read
-    something else (``copy`` its own need). An unset value in any other argument is
-    ``None``.
+    filter, ``links_only``). A call with an unset one (``None``, or ``""``, which an
+    untyped extra option holds when not set) fails: it would otherwise read something
+    else (``copy`` its own need). Any other argument passes the value on.
 
     :param call: A call of a built-in function.
     :param need: The need the call belongs to.
@@ -133,7 +135,7 @@ def unset_selector(
             key in _SELECTORS[call.name]
             and isinstance(value, NeedAttribute)
             and value.name in need
-            and need[value.name] is None
+            and need[value.name] in _UNSET
         ):
             return value.name
     return None
@@ -610,7 +612,7 @@ def read(self) -> None:
                 continue
             if attr not in self.need:
                 return  # the call fails: need has no attribute
-            if self.need[attr] is None:
+            if self.need[attr] in _UNSET:
                 return  # the call fails: what it reads is not selected
             args[key] = self.need[attr]
         if blocking:

packages/sphinx-needs/tests/test_dynamic_functions_strata.py

@@ -2218,6 +2218,114 @@ def unset(need_id: str, field: str) -> str:
     ]
 
 
+EMPTY_SELECTOR_CONF = (
+    CONF
+    + """\
+# an untyped extra option: a need that does not set it holds ""
+needs_extra_options = ["src"]
+needs_build_json = True
+"""
+)
+
+EMPTY_SELECTOR_INDEX = """\
+Empty selector
+==============
+
+.. req:: Target
+   :id: TGT
+   :hours: 2
+
+.. req:: copy's need
+   :id: E_COPY
+   :summary: [[copy("title", need.src)]]
+
+.. req:: calc_sum's field
+   :id: E_SUM
+   :total: [[calc_sum(need.src)]]
+
+.. req:: check_linked_values' field
+   :id: E_CLV
+   :summary: [[check_linked_values("ok", need.src, "x")]]
+   :links: TGT
+
+.. req:: links_only
+   :id: E_ONLY
+   :total: [[calc_sum("hours", links_only=need.src)]]
+
+.. req:: A value, not a selector
+   :id: E_VALUE
+   :summary: [[copy("title", "TGT", upper=need.src)]]
+
+   ndf says: :ndf:`copy("title", need.src)`
+"""
+
+
+@pytest.mark.parametrize(
+    "test_app",
+    [
+        {
+            "buildername": "html",
+            "files": [
+                (Path("conf.py"), EMPTY_SELECTOR_CONF),
+                (Path("index.rst"), EMPTY_SELECTOR_INDEX),
+            ],
+        }
+    ],
+    indirect=True,
+)
+def test_an_empty_need_attribute_selector_fails_the_call(test_app):
+    """A selector whose field holds ``""`` is unset, as one holding ``None`` is.
+
+    An untyped extra option a need does not set holds ``""``; ``copy("title", "")``
+    would copy the need's own title, ``calc_sum("")`` sum nothing, and a ``links_only``
+    of ``""`` sum every need. Each call fails instead, naming the field, in a field and
+    in an ``ndf`` role alike; in a value argument (``upper``) ``""`` is a value.
+    """
+    app = test_app
+    app.build()
+    data = json.loads(Path(app.outdir, "needs.json").read_text(encoding="utf-8"))
+    needs = data["versions"][data["current_version"]]["needs"]
+    assert {
+        "E_COPY": needs["E_COPY"]["summary"],
+        "E_SUM": needs["E_SUM"]["total"],
+        "E_CLV": needs["E_CLV"]["summary"],
+        "E_ONLY": needs["E_ONLY"]["total"],
+        "E_VALUE": needs["E_VALUE"]["summary"],
+    } == {
+        "E_COPY": None,
+        "E_SUM": None,
+        "E_CLV": None,
+        "E_ONLY": None,
+        "E_VALUE": "Target",
+    }
+    html = Path(app.outdir, "index.html").read_text(encoding="utf-8")
+    assert "ndf says: ??" in html
+
+    def empty(need_id: str, field: str, function: str) -> str:
+        return _warning(
+            "index",
+            EMPTY_SELECTOR_INDEX,
+            need_id,
+            f"Error while resolving dynamic values for field '{field}', of need "
+            f"'{need_id}': Error while applying need to function '{function}': "
+            "need.src selects what the call reads, and is not set",
+            "dynamic_function",
+        )
+
+    warnings = build_warnings(app)
+    assert [warning for warning in warnings if "Error while resolving" in warning] == [
+        empty("E_CLV", "summary", "check_linked_values"),
+        empty("E_COPY", "summary", "copy"),
+        empty("E_ONLY", "total", "calc_sum"),
+        empty("E_SUM", "total", "calc_sum"),
+    ]
+    assert any(
+        "WARNING: Error while applying need to function 'copy': need.src selects what "
+        "the call reads, and is not set [needs.dynamic_function]" in warning
+        for warning in warnings
+    ), warnings
+
+
 UNSET_ROLE_INDEX = """\
 Unset selector in a role
 ========================

packages/sphinx-needs/tests/test_functions_order.py

@@ -161,6 +161,7 @@ def _table_project(field: str, text: str) -> Project:
             grp="g",
             cparent="[[copy('parent')]]",
             unset=None,
+            empty="",
             **{field: text},
         ),
         refs={"RD": ["DYN", "SRC"]},
@@ -221,8 +222,10 @@ def _table_project(field: str, text: str) -> Project:
             "[[copy('summary', 'DYN', upper=need.cparent)]]",
             {"deps": [("RD", "cparent"), ("DYN", "summary")]},
         ),
-        # an unset selector fails the call; an unset value is None
+        # an unset (or empty) selector fails the call; an unset value is None
         ("out", "[[copy('summary', need.unset)]]", {}),
+        ("out", "[[copy('summary', need.empty)]]", {}),
+        ("out", "[[calc_sum('hours', links_only=need.empty)]]", {}),
         (
             "out",
             "[[copy('summary', 'DYN', upper=need.unset)]]",