From 3916cb7678b59f643c093bc7a134bf2a55a9d2c7 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 3 Sep 2024 13:08:34 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20Improve=20typing/docs=20of=20`Dy?= =?UTF-8?q?namicFunction`=20(#1262)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api.rst | 3 + docs/configuration.rst | 2 +- docs/dynamic_functions.rst | 72 +---------- sphinx_needs/directives/need.py | 2 +- sphinx_needs/directives/needtable.py | 2 +- sphinx_needs/functions/__init__.py | 6 +- sphinx_needs/functions/common.py | 14 +-- sphinx_needs/functions/functions.py | 119 ++++++++++++------ sphinx_needs/roles/need_func.py | 6 +- tests/doc_test/doc_df_user_functions/conf.py | 6 +- .../doc_test/doc_df_user_functions/index.rst | 11 ++ tests/test_dynamic_functions.py | 29 ++++- 12 files changed, 149 insertions(+), 123 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 91abae594..e8aa1801f 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -20,6 +20,9 @@ Configuration .. automodule:: sphinx_needs.api.configuration :members: +.. autoclass:: sphinx_needs.functions.functions.DynamicFunction + :members: __name__,__call__ + Need ---- .. automodule:: sphinx_needs.api.need diff --git a/docs/configuration.rst b/docs/configuration.rst index 8458622a2..9783c9f25 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -1261,7 +1261,7 @@ needs_functions Used to register own dynamic functions. -Must be a list of python functions. +Must be a list of :py:class:`.DynamicFunction`. Default value: ``[]`` diff --git a/docs/dynamic_functions.rst b/docs/dynamic_functions.rst index a5d38de0f..2f4e36b24 100644 --- a/docs/dynamic_functions.rst +++ b/docs/dynamic_functions.rst @@ -18,22 +18,13 @@ dynamic function execution. In this case, the function gets executed with the fo This allows you to set row and column specific styles such as, set a row background to red, if a need-status is *failed*. -Example -------- - -.. code-block:: rst +.. need-example:: Dynamic function example .. req:: my test requirement :id: df_1 :status: open - This need has the id **[[copy("id")]]** and status **[[copy("status")]]**. - -.. req:: my test requirement - :id: df_1 - :status: open - - This need has id **[[copy("id")]]** and status **[[copy("status")]]**. + This need has id **[[copy("id")]]** and status **[[copy("status")]]**. Built-in functions ------------------- @@ -87,8 +78,8 @@ Develop own functions Registration ~~~~~~~~~~~~ -You must register every dynamic function by using the :ref:`needs_functions` configuration parameter -inside your **conf.py** file: +You must register every dynamic function by using the :ref:`needs_functions` configuration parameter, +inside your **conf.py** file, to add a :py:class:`.DynamicFunction`: .. code-block:: python @@ -115,61 +106,6 @@ inside your **conf.py** file: def setup(app): add_dynamic_function(app, my_function) -Reference function -~~~~~~~~~~~~~~~~~~ - -.. code-block:: python - - def test(app, need, needs, *args, **kwargs): - """ - :param app: sphinx app - :param need: current need - :param needs: dictionary of all needs. Key is the need id - :return: str,int,float or list of elements of type str,int,float - """ - # where the magic happens - return "awesome" - -You can call the defined function via: - -.. code-block:: rst - - .. req:: test requirement - :status: [[test()]] - -The parameters ``app``, ``need`` and ``needs`` are set automatically. You are free to add other parameters, which -must be of type str, int, float and list. - - -need structure -~~~~~~~~~~~~~~ - -.. code-block:: text - - need = { - 'docname': str: document name, - 'lineno': int: linenumber, - 'links_back': list: list of incoming links (see restrictions), - 'target_node': node: sphinx target node for internal references, - 'type': str: short name of type, - 'type_name': str: long name of type, - 'type_prefix': str: name of type prefix, - 'type_color': str: type color, - 'type_style': str: type style, - 'status': str: status, - 'tags': list: list of tags, - 'id': str: id, - 'links': list: list of outgoing links, - 'title': str: trimmed title of need, - 'full_title': str: full title of string, - 'content': str: unparsed need content, - 'collapse': bool: true if meta data shall be collapsed, - 'hide': bool: true if complete need shall be hidden, - } - -Adding new keywords to need object will be treated as extra_option in need meta area. - - Restrictions ~~~~~~~~~~~~ diff --git a/sphinx_needs/directives/need.py b/sphinx_needs/directives/need.py index 7dc0964e0..1126e6288 100644 --- a/sphinx_needs/directives/need.py +++ b/sphinx_needs/directives/need.py @@ -436,7 +436,7 @@ def format_need_nodes( find_and_replace_node_content(node_need, env, need_data) for index, attribute in enumerate(node_need.attributes["classes"]): node_need.attributes["classes"][index] = check_and_get_content( - attribute, need_data, env + attribute, need_data, env, node_need ) rendered_node = build_need_repr(node_need, need_data, app, docname=fromdocname) diff --git a/sphinx_needs/directives/needtable.py b/sphinx_needs/directives/needtable.py index 6e2af279c..0b97f4ede 100644 --- a/sphinx_needs/directives/needtable.py +++ b/sphinx_needs/directives/needtable.py @@ -244,7 +244,7 @@ def sort(need: NeedsInfoType) -> Any: for need_info in filtered_needs: style_row = check_and_get_content( - current_needtable["style_row"], need_info, env + current_needtable["style_row"], need_info, env, node ) style_row = style_row.replace( " ", "_" diff --git a/sphinx_needs/functions/__init__.py b/sphinx_needs/functions/__init__.py index 92559b4ed..16b57a1c4 100644 --- a/sphinx_needs/functions/__init__.py +++ b/sphinx_needs/functions/__init__.py @@ -1,4 +1,4 @@ -from typing import Any, Callable, List +from typing import List from sphinx_needs.functions.common import ( calc_sum, @@ -9,6 +9,7 @@ test, ) from sphinx_needs.functions.functions import ( # noqa: F401 + DynamicFunction, FunctionParsingException, execute_func, find_and_replace_node_content, @@ -17,8 +18,7 @@ resolve_variants_options, ) -# TODO better type signature (Sphinx, NeedsInfoType, Dict[str, NeedsInfoType], *args, **kwargs) -NEEDS_COMMON_FUNCTIONS: List[Callable[..., Any]] = [ +NEEDS_COMMON_FUNCTIONS: List[DynamicFunction] = [ test, echo, copy, diff --git a/sphinx_needs/functions/common.py b/sphinx_needs/functions/common.py index 97f56b315..6e2b68847 100644 --- a/sphinx_needs/functions/common.py +++ b/sphinx_needs/functions/common.py @@ -14,7 +14,7 @@ from sphinx_needs.api.exceptions import NeedsInvalidFilter from sphinx_needs.config import NeedsSphinxConfig -from sphinx_needs.data import NeedsInfoType +from sphinx_needs.data import NeedsInfoType, NeedsView from sphinx_needs.filter_common import filter_needs, filter_single_need from sphinx_needs.logging import log_warning from sphinx_needs.utils import logger @@ -23,7 +23,7 @@ def test( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, *args: Any, **kwargs: Any, ) -> str: @@ -50,7 +50,7 @@ def test( def echo( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, text: str, *args: Any, **kwargs: Any, @@ -74,7 +74,7 @@ def echo( def copy( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, option: str, need_id: str | None = None, lower: bool = False, @@ -191,7 +191,7 @@ def copy( def check_linked_values( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, result: Any, search_option: str, search_value: Any, @@ -360,7 +360,7 @@ def check_linked_values( def calc_sum( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, option: str, filter: str | None = None, links_only: bool = False, @@ -472,7 +472,7 @@ def calc_sum( def links_from_content( app: Sphinx, need: NeedsInfoType, - needs: dict[str, NeedsInfoType], + needs: NeedsView, need_id: str | None = None, filter: str | None = None, ) -> list[str]: diff --git a/sphinx_needs/functions/functions.py b/sphinx_needs/functions/functions.py index 2825593cf..10f8962c5 100644 --- a/sphinx_needs/functions/functions.py +++ b/sphinx_needs/functions/functions.py @@ -10,7 +10,7 @@ import ast import re -from typing import Any, Callable, List, Union +from typing import Any, Protocol from docutils import nodes from sphinx.application import Sphinx @@ -21,18 +21,27 @@ from sphinx_needs.config import NeedsSphinxConfig from sphinx_needs.data import NeedsInfoType, NeedsMutable, NeedsView, SphinxNeedsData from sphinx_needs.debug import measure_time_func -from sphinx_needs.logging import get_logger +from sphinx_needs.logging import get_logger, log_warning from sphinx_needs.utils import NEEDS_FUNCTIONS, match_variants logger = get_logger(__name__) unicode = str ast_boolean = ast.NameConstant -# TODO these functions also take optional *args and **kwargs -DynamicFunction = Callable[ - [Sphinx, NeedsInfoType, NeedsView], - Union[str, int, float, List[Union[str, int, float]]], -] + +class DynamicFunction(Protocol): + """A protocol for a sphinx-needs dynamic function.""" + + __name__: str + + def __call__( + self, + app: Sphinx, + need: NeedsInfoType, + needs: NeedsView, + *args: Any, + **kwargs: Any, + ) -> str | int | float | list[str] | list[int] | list[float] | None: ... def register_func(need_function: DynamicFunction, name: str | None = None) -> None: @@ -64,23 +73,40 @@ def register_func(need_function: DynamicFunction, name: str | None = None) -> No NEEDS_FUNCTIONS[func_name] = {"name": func_name, "function": need_function} -def execute_func(app: Sphinx, need: NeedsInfoType, func_string: str) -> Any: +def execute_func( + app: Sphinx, + need: NeedsInfoType, + func_string: str, + location: str | tuple[str | None, int | None] | nodes.Node | None, +) -> str | int | float | list[str] | list[int] | list[float] | None: """Executes a given function string. :param env: Sphinx environment :param need: Actual need, which contains the found function string :param func_string: string of the found function. Without ``[[ ]]`` + :param location: source location of the function call :return: return value of executed function """ global NEEDS_FUNCTIONS - func_name, func_args, func_kwargs = _analyze_func_string(func_string, need) + try: + func_name, func_args, func_kwargs = _analyze_func_string(func_string, need) + except FunctionParsingException as err: + log_warning( + logger, + f"Function string {func_string!r} could not be parsed: {err}", + "dynamic_function", + location=location, + ) + return "??" if func_name not in NEEDS_FUNCTIONS: - raise SphinxError( - "Unknown dynamic sphinx-needs function: {}. Found in need: {}".format( - func_name, need["id"] - ) + log_warning( + logger, + f"Unknown function {func_name!r}", + "dynamic_function", + location=location, ) + return "??" func = measure_time_func( NEEDS_FUNCTIONS[func_name]["function"], category="dyn_func", source="user" @@ -93,18 +119,24 @@ def execute_func(app: Sphinx, need: NeedsInfoType, func_string: str) -> Any: **func_kwargs, ) - if not isinstance(func_return, (str, int, float, list, unicode)) and func_return: - raise SphinxError( - f"Return value of function {func_name} is of type {type(func_return)}. Allowed are str, int, float" + if func_return is not None and not isinstance(func_return, (str, int, float, list)): + log_warning( + logger, + f"Return value of function {func_name!r} is of type {type(func_return)}. Allowed are str, int, float, list", + "dynamic_function", + location=location, ) - + return "??" if isinstance(func_return, list): - for element in func_return: - if not isinstance(element, (str, int, float, unicode)): - raise SphinxError( - f"Element of return list of function {func_name} is of type {type(func_return)}. " - "Allowed are str, int, float" + for i, element in enumerate(func_return): + if not isinstance(element, (str, int, float)): + log_warning( + logger, + f"Return value item {i} of function {func_name!r} is of type {type(element)}. Allowed are str, int, float", + "dynamic_function", + location=location, ) + return "??" return func_return @@ -149,13 +181,15 @@ def find_and_replace_node_content( func_string = func_string.replace("‘", "'") # noqa: RUF001 func_string = func_string.replace("’", "'") # noqa: RUF001 - func_return = execute_func(env.app, need, func_string) + func_return = execute_func(env.app, need, func_string, node) - # This should never happen, but we can not be sure. if isinstance(func_return, list): - func_return = ", ".join(func_return) + func_return = ", ".join(str(el) for el in func_return) - new_text = new_text.replace(f"[[{func_string_org}]]", func_return) + new_text = new_text.replace( + f"[[{func_string_org}]]", + "" if func_return is None else str(func_return), + ) if isinstance(node, nodes.reference): node.attributes["refuri"] = new_text @@ -207,7 +241,7 @@ def resolve_dynamic_values(needs: NeedsMutable, app: Sphinx) -> None: func_call: str | None = "init" while func_call: try: - func_call, func_return = _detect_and_execute( + func_call, func_return = _detect_and_execute_field( need[need_option], need, app ) except FunctionParsingException: @@ -239,7 +273,9 @@ def resolve_dynamic_values(needs: NeedsMutable, app: Sphinx) -> None: new_values = [] for element in need[need_option]: try: - func_call, func_return = _detect_and_execute(element, need, app) + func_call, func_return = _detect_and_execute_field( + element, need, app + ) except FunctionParsingException: raise SphinxError( "Function definition of {option} in file {file}:{line} has " @@ -319,7 +355,7 @@ def resolve_variants_options( def check_and_get_content( - content: str, need: NeedsInfoType, env: BuildEnvironment + content: str, need: NeedsInfoType, env: BuildEnvironment, location: nodes.Node ) -> str: """ Checks if the given content is a function call. @@ -329,6 +365,7 @@ def check_and_get_content( :param content: option content string :param need: need :param env: Sphinx environment object + :param location: source location of the function call :return: string """ @@ -343,18 +380,23 @@ def check_and_get_content( func_call = func_match.group(1) # Extract function call func_return = execute_func( - env.app, need, func_call + env.app, need, func_call, location ) # Execute function call and get return value + if isinstance(func_return, list): + func_return = ", ".join(str(el) for el in func_return) + # Replace the function_call with the calculated value - content = content.replace(f"[[{func_call}]]", func_return) + content = content.replace( + f"[[{func_call}]]", "" if func_return is None else str(func_return) + ) return content -def _detect_and_execute( +def _detect_and_execute_field( content: Any, need: NeedsInfoType, app: Sphinx -) -> tuple[str | None, Any]: - """Detects if given content is a function call and executes it.""" +) -> tuple[str | None, str | int | float | list[str] | list[int] | list[float] | None]: + """Detects if given need field value is a function call and executes it.""" try: content = str(content) except UnicodeEncodeError: @@ -366,7 +408,10 @@ def _detect_and_execute( func_call = func_match.group(1) # Extract function call func_return = execute_func( - app, need, func_call + app, + need, + func_call, + (need["docname"], need["lineno"]) if need["docname"] else None, ) # Execute function call and get return value return func_call, func_return @@ -391,14 +436,14 @@ def _analyze_func_string( func = ast.parse(func_string) except SyntaxError as e: need_id = need["id"] if need else "UNKNOWN" - raise SphinxError( + raise FunctionParsingException( f"Parsing function string failed for need {need_id}: {func_string}. {e}" ) try: func_call = func.body[0].value # type: ignore func_name = func_call.func.id except AttributeError: - raise SphinxError( + raise FunctionParsingException( f"Given dynamic function string is not a valid python call. Got: {func_string}" ) diff --git a/sphinx_needs/roles/need_func.py b/sphinx_needs/roles/need_func.py index 3497f395b..4e9218238 100644 --- a/sphinx_needs/roles/need_func.py +++ b/sphinx_needs/roles/need_func.py @@ -7,6 +7,7 @@ from docutils import nodes from sphinx.application import Sphinx +from sphinx_needs.data import NeedsInfoType from sphinx_needs.functions.functions import check_and_get_content from sphinx_needs.logging import get_logger @@ -25,11 +26,10 @@ def process_need_func( ) -> 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"], - {"id": "need_func_dummy"}, # type: ignore - env, + node_need_func.attributes["reftarget"], dummy_need, env, node_need_func ) new_node_func = nodes.Text(str(result)) node_need_func.replace_self(new_node_func) diff --git a/tests/doc_test/doc_df_user_functions/conf.py b/tests/doc_test/doc_df_user_functions/conf.py index 20f8cea84..44ba9de64 100644 --- a/tests/doc_test/doc_df_user_functions/conf.py +++ b/tests/doc_test/doc_df_user_functions/conf.py @@ -38,4 +38,8 @@ def my_own_function(app, need, needs): return "Awesome" -needs_functions = [my_own_function] +def bad_function(app, need, needs): + return object() + + +needs_functions = [my_own_function, bad_function] diff --git a/tests/doc_test/doc_df_user_functions/index.rst b/tests/doc_test/doc_df_user_functions/index.rst index efd1e29a5..e9b4087cd 100644 --- a/tests/doc_test/doc_df_user_functions/index.rst +++ b/tests/doc_test/doc_df_user_functions/index.rst @@ -5,3 +5,14 @@ DYNAMIC FUNCTIONS :id: TEST_1 :status: [[my_own_function()]] + [[my_own_function()]] + +.. spec:: TEST_2 + :id: TEST_2 + :status: [[bad_function()]] + + [[bad_function()]] + + [[invalid]] + + [[unknown()]] diff --git a/tests/test_dynamic_functions.py b/tests/test_dynamic_functions.py index 59d0a7978..fcc94c698 100644 --- a/tests/test_dynamic_functions.py +++ b/tests/test_dynamic_functions.py @@ -1,7 +1,10 @@ +import os import re from pathlib import Path import pytest +from sphinx import version_info +from sphinx.util.console import strip_colors @pytest.mark.parametrize( @@ -76,11 +79,35 @@ def test_doc_df_linked_values(test_app): @pytest.mark.parametrize( "test_app", - [{"buildername": "html", "srcdir": "doc_test/doc_df_user_functions"}], + [ + { + "buildername": "html", + "srcdir": "doc_test/doc_df_user_functions", + "no_plantuml": True, + } + ], indirect=True, ) def test_doc_df_user_functions(test_app): app = test_app app.build() + + warnings = strip_colors( + app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/") + ).splitlines() + # print(warnings) + expected = [ + "srcdir/index.rst:10: WARNING: Return value of function 'bad_function' is of type . Allowed are str, int, float, list [needs.dynamic_function]", + "srcdir/index.rst:14: WARNING: Return value of function 'bad_function' is of type . Allowed are str, int, float, list [needs.dynamic_function]", + "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: Unknown function 'unknown' [needs.dynamic_function]", + ] + if version_info >= (7, 3): + warn = "WARNING: cannot cache unpickable configuration value: 'needs_functions' (because it contains a function, class, or module object)" + if version_info >= (8, 0): + warn += " [config.cache]" + expected.insert(0, warn) + assert warnings == expected + html = Path(app.outdir, "index.html").read_text() assert "Awesome" in html