diff --git a/packages/sphinx-needs/docs/changelog.rst b/packages/sphinx-needs/docs/changelog.rst index 0aa36c0d6..c76a986af 100644 --- a/packages/sphinx-needs/docs/changelog.rst +++ b/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 ` calls, a :ref:`calc_sum ` over computed summands, @@ -95,7 +95,7 @@ Improvements (:ref:`argument `); 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. diff --git a/packages/sphinx-needs/docs/dynamic_functions.rst b/packages/sphinx-needs/docs/dynamic_functions.rst index 056a8f29b..234dd04d7 100644 --- a/packages/sphinx-needs/docs/dynamic_functions.rst +++ b/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 diff --git a/packages/sphinx-needs/src/sphinx_needs/functions/order.py b/packages/sphinx-needs/src/sphinx_needs/functions/order.py index 42c8f4c67..66d14bb09 100644 --- a/packages/sphinx-needs/src/sphinx_needs/functions/order.py +++ b/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.`` 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: diff --git a/packages/sphinx-needs/tests/test_dynamic_functions_strata.py b/packages/sphinx-needs/tests/test_dynamic_functions_strata.py index 87ed3f189..9ffa56659 100644 --- a/packages/sphinx-needs/tests/test_dynamic_functions_strata.py +++ b/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 ======================== diff --git a/packages/sphinx-needs/tests/test_functions_order.py b/packages/sphinx-needs/tests/test_functions_order.py index 69cb4dc6f..f9f7cce10 100644 --- a/packages/sphinx-needs/tests/test_functions_order.py +++ b/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)]]",