From 5c32994ba35ca1ee64e6db4e3e835e8db781748e Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Thu, 28 Sep 2023 10:59:28 -0300 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20Consolidate=20needs=20data=20pos?= =?UTF-8?q?t-processing=20(#1039)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is a step towards being able to generate the needs.json without running the Sphinx `BUILD` phase. Here we consolidate all functions that post-process the needs data, after it has been fully extracted from all documents and external sources. We remove the individual logic from within these functions, to check if the post-processing has already been done, and instead check before running all the functions. This refactor does not actually change the sphinx-needs process in any way, this will come later. --- docs/contributing.rst | 2 +- sphinx_needs/data.py | 46 ++---- sphinx_needs/directives/need.py | 114 ++++++------- sphinx_needs/directives/needextend.py | 227 ++++++++++++-------------- sphinx_needs/functions/functions.py | 60 +++---- sphinx_needs/need_constraints.py | 14 +- sphinx_needs/needs.py | 2 - 7 files changed, 197 insertions(+), 268 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index df6506190..f0f71cbbf 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -342,7 +342,7 @@ The following is an outline of the build events which this extension adds to the - Check for dead links (``process_need_nodes -> check_links``) - Generate back links (``process_need_nodes -> create_back_links``) - Process constraints, for each ``Need`` node (``process_need_nodes -> process_constraints``) - - Perform all modifications on need data items, due to ``Needextend`` nodes (``process_need_nodes -> process_needextend``) + - Perform all modifications on need data items, due to ``Needextend`` nodes (``process_need_nodes -> extend_needs_data``) - Format each ``Need`` node to give the desired visual output (``process_need_nodes -> print_need_nodes``) - Process all other need specific nodes, replacing them with the desired visual output (``process_creator``) diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py index 3c8ddaa7d..9fa8d44f2 100644 --- a/sphinx_needs/data.py +++ b/sphinx_needs/data.py @@ -30,23 +30,6 @@ class NeedsFilterType(TypedDict): amount: int -class NeedsWorkflowType(TypedDict): - """ - Used to store workflow status information for already executed tasks. - Some tasks like backlink_creation need be performed only once. - But most sphinx-events get called several times (for each single document file), - which would also execute our code several times... - """ - - backlink_creation_links: bool - dynamic_values_resolved: bool - links_checked: bool - add_sections: bool - variant_option_resolved: bool - needs_extended: bool - needs_constraints: bool - - class NeedsBaseDataType(TypedDict): """A base type for all data.""" @@ -448,27 +431,18 @@ def get_or_create_docs(self) -> dict[str, list[str]]: self.env.needs_all_docs = {"all": []} return self.env.needs_all_docs - def get_or_create_workflow(self) -> NeedsWorkflowType: - """Get workflow information. - - This is lazily created and cached in the environment. - """ + @property + def needs_is_post_processed(self) -> bool: + """Whether needs have been post-processed.""" try: - return self.env.needs_workflow + return self.env.needs_is_post_processed except AttributeError: - self.env.needs_workflow = { - "backlink_creation_links": False, - "dynamic_values_resolved": False, - "links_checked": False, - "add_sections": False, - "variant_option_resolved": False, - "needs_extended": False, - "needs_constraints": False, - } - for link_type in self.env.app.config.needs_extra_links: - self.env.needs_workflow["backlink_creation_{}".format(link_type["option"])] = False - - return self.env.needs_workflow # type: ignore[return-value] + self.env.needs_is_post_processed = False + return self.env.needs_is_post_processed + + @needs_is_post_processed.setter + def needs_is_post_processed(self, value: bool) -> None: + self.env.needs_is_post_processed = value def get_or_create_services(self) -> ServiceManager: """Get information about services. diff --git a/sphinx_needs/directives/need.py b/sphinx_needs/directives/need.py index 7a0987fb6..3895bdf5a 100644 --- a/sphinx_needs/directives/need.py +++ b/sphinx_needs/directives/need.py @@ -17,7 +17,11 @@ from sphinx_needs.data import SphinxNeedsData from sphinx_needs.debug import measure_time from sphinx_needs.defaults import NEED_DEFAULT_OPTIONS -from sphinx_needs.directives.needextend import process_needextend +from sphinx_needs.directives.needextend import ( + Needextend, + extend_needs_data, + remove_needextend_node, +) from sphinx_needs.functions import ( find_and_replace_node_content, resolve_dynamic_values, @@ -376,29 +380,23 @@ def process_need_nodes(app: Sphinx, doctree: nodes.document, fromdocname: str) - return env = app.env + needs_data = SphinxNeedsData(env) # If no needs were defined, we do not need to do anything - if not hasattr(env, "needs_all_needs"): + if not needs_data.get_or_create_needs(): return - # Call dynamic functions and replace related node data with their return values - resolve_dynamic_values(env) - - # Apply variant handling on options and replace its values with their return values - resolve_variants_options(env) - - # check if we have dead links - check_links(env) - - # Create back links of common links and extra links - for links in needs_config.extra_links: - create_back_links(env, links["option"]) - - process_constraints(app) + if not needs_data.needs_is_post_processed: + resolve_dynamic_values(env) + resolve_variants_options(env) + check_links(env) + create_back_links(env) + process_constraints(app) + extend_needs_data(app) + needs_data.needs_is_post_processed = True - # We call process_needextend here by our own, so that we are able - # to give print_need_nodes the already found need_nodes. - process_needextend(app, doctree, fromdocname) + for extend_node in doctree.findall(Needextend): + remove_needextend_node(extend_node) print_need_nodes(app, doctree, fromdocname, list(doctree.findall(Need))) @@ -432,18 +430,16 @@ def print_need_nodes(app: Sphinx, doctree: nodes.document, fromdocname: str, fou def check_links(env: BuildEnvironment) -> None: + """Checks if set links are valid or are dead (referenced need does not exist.) + + For needs with dead links, an extra ``has_dead_links`` field is added and, + if the link is not allowed to be dead, + the ``has_forbidden_dead_links`` field is also added. """ - Checks if set links are valid or are dead (referenced need does not exist.) - :param env: Sphinx environment - :return: - """ + config = NeedsSphinxConfig(env.config) data = SphinxNeedsData(env) - workflow = data.get_or_create_workflow() - if workflow["links_checked"]: - return - needs = data.get_or_create_needs() - extra_links = getattr(env.config, "needs_extra_links", []) + extra_links = config.extra_links for need in needs.values(): for link_type in extra_links: dead_links_allowed = link_type.get("allow_dead_links", False) @@ -464,45 +460,39 @@ def check_links(env: BuildEnvironment) -> None: need["has_forbidden_dead_links"] = True break # One found dead link is enough - # Finally set a flag so that this function gets not executed several times - workflow["links_checked"] = True - -def create_back_links(env: BuildEnvironment, option: str) -> None: - """ - Create back-links in all found needs. - But do this only once, as all needs are already collected and this sorting is for all - needs and not only for the ones of the current document. +def create_back_links(env: BuildEnvironment) -> None: + """Create back-links in all found needs. - :param env: sphinx environment + These are fields for each link type, ``_back``, + which contain a list of all IDs of needs that link to the current need. """ data = SphinxNeedsData(env) - workflow = data.get_or_create_workflow() - option_back = f"{option}_back" - if workflow[f"backlink_creation_{option}"]: # type: ignore[literal-required] - return - + needs_config = NeedsSphinxConfig(env.config) needs = data.get_or_create_needs() - for key, need in needs.items(): - need_link_value = [need[option]] if isinstance(need[option], str) else need[option] # type: ignore[literal-required] - for link in need_link_value: - link_main = link.split(".")[0] - try: - link_part = link.split(".")[1] - except IndexError: - link_part = None - - if link_main in needs: - if key not in needs[link_main][option_back]: # type: ignore[literal-required] - needs[link_main][option_back].append(key) # type: ignore[literal-required] - - # Handling of links to need_parts inside a need - if link_part and link_part in needs[link_main]["parts"]: - if option_back not in needs[link_main]["parts"][link_part].keys(): - needs[link_main]["parts"][link_part][option_back] = [] # type: ignore[literal-required] - needs[link_main]["parts"][link_part][option_back].append(key) # type: ignore[literal-required] - - workflow[f"backlink_creation_{option}"] = True # type: ignore[literal-required] + + for links in needs_config.extra_links: + option = links["option"] + option_back = f"{option}_back" + + for key, need in needs.items(): + need_link_value = [need[option]] if isinstance(need[option], str) else need[option] # type: ignore[literal-required] + for link in need_link_value: + link_main = link.split(".")[0] + try: + link_part = link.split(".")[1] + except IndexError: + link_part = None + + if link_main in needs: + if key not in needs[link_main][option_back]: # type: ignore[literal-required] + needs[link_main][option_back].append(key) # type: ignore[literal-required] + + # Handling of links to need_parts inside a need + if link_part and link_part in needs[link_main]["parts"]: + if option_back not in needs[link_main]["parts"][link_part].keys(): + needs[link_main]["parts"][link_part][option_back] = [] # type: ignore[literal-required] + needs[link_main]["parts"][link_part][option_back].append(key) # type: ignore[literal-required] def _fix_list_dyn_func(list: List[str]) -> List[str]: diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index 2b10fddb3..278593864 100644 --- a/sphinx_needs/directives/needextend.py +++ b/sphinx_needs/directives/needextend.py @@ -71,132 +71,119 @@ def run(self) -> Sequence[nodes.Node]: return [targetnode, Needextend("")] -def process_needextend(app: Sphinx, doctree: nodes.document, fromdocname: str) -> None: - """ - Perform all modifications on needs - """ +def extend_needs_data(app: Sphinx) -> None: + """Use data gathered from needextend directives to modify fields of existing needs.""" env = app.env needs_config = NeedsSphinxConfig(env.config) data = SphinxNeedsData(env) - workflow = data.get_or_create_workflow() - - if not workflow["needs_extended"]: - workflow["needs_extended"] = True - - list_values = ( - ["tags", "links"] - + [x["option"] for x in needs_config.extra_links] - + [f"{x['option']}_back" for x in needs_config.extra_links] - ) # back-links (incoming) - link_names = [x["option"] for x in needs_config.extra_links] - - all_needs = data.get_or_create_needs() - - for current_needextend in data.get_or_create_extends().values(): - need_filter = current_needextend["filter"] - if need_filter in all_needs: - # a single known ID - found_needs = [all_needs[need_filter]] - elif need_filter is not None and re.fullmatch(needs_config.id_regex, need_filter): - # an unknown ID - error = f"Provided id {need_filter} for needextend does not exist." - if current_needextend["strict"]: - raise NeedsInvalidFilter(error) - else: - logger.info(error) - continue + + list_values = ( + ["tags", "links"] + + [x["option"] for x in needs_config.extra_links] + + [f"{x['option']}_back" for x in needs_config.extra_links] + ) # back-links (incoming) + link_names = [x["option"] for x in needs_config.extra_links] + + all_needs = data.get_or_create_needs() + + for current_needextend in data.get_or_create_extends().values(): + need_filter = current_needextend["filter"] + if need_filter in all_needs: + # a single known ID + found_needs = [all_needs[need_filter]] + elif need_filter is not None and re.fullmatch(needs_config.id_regex, need_filter): + # an unknown ID + error = f"Provided id {need_filter} for needextend does not exist." + if current_needextend["strict"]: + raise NeedsInvalidFilter(error) else: - # a filter string - try: - found_needs = filter_needs(app, all_needs.values(), need_filter) - except NeedsInvalidFilter as e: - raise NeedsInvalidFilter( - f"Filter not valid for needextend on page {current_needextend['docname']}:\n{e}" - ) - - for found_need in found_needs: - # Work in the stored needs, not on the search result - need = all_needs[found_need["id"]] - need["is_modified"] = True - need["modifications"] += 1 - - for option, value in current_needextend["modifications"].items(): - if option.startswith("+"): - option_name = option[1:] - if option_name in link_names: - # If we add links, then add all corresponding back links - for ref_need in [i.strip() for i in re.split(";|,", value)]: - if ref_need not in all_needs: - logger.warning( - f"Provided link id {ref_need} for needextend does not exist. [needs]", - type="needs", - location=(current_needextend["docname"], current_needextend["lineno"]), - ) - continue - if ref_need not in need[option_name]: - need[option_name].append(ref_need) - if found_need["id"] not in all_needs[ref_need][f"{option_name}_back"]: - all_needs[ref_need][f"{option_name}_back"] += [found_need["id"]] - elif option_name in list_values: - for item in [i.strip() for i in re.split(";|,", value)]: - if item not in need[option_name]: - need[option_name].append(item) - else: - if need[option_name]: - # If content is already stored, we need to add some whitespace - need[option_name] += " " - need[option_name] += value - - elif option.startswith("-"): - option_name = option[1:] - if option_name in link_names: - # If we remove links, then remove all corresponding back links - for ref_need in (i for i in need[option_name] if i in all_needs): - all_needs[ref_need][f"{option_name}_back"].remove(found_need["id"]) - need[option_name] = [] - if option_name in list_values: - need[option_name] = [] - else: - need[option_name] = "" + logger.info(error) + continue + else: + # a filter string + try: + found_needs = filter_needs(app, all_needs.values(), need_filter) + except NeedsInvalidFilter as e: + raise NeedsInvalidFilter( + f"Filter not valid for needextend on page {current_needextend['docname']}:\n{e}" + ) + + for found_need in found_needs: + # Work in the stored needs, not on the search result + need = all_needs[found_need["id"]] + need["is_modified"] = True + need["modifications"] += 1 + + for option, value in current_needextend["modifications"].items(): + if option.startswith("+"): + option_name = option[1:] + if option_name in link_names: + # If we add links, then add all corresponding back links + for ref_need in [i.strip() for i in re.split(";|,", value)]: + if ref_need not in all_needs: + logger.warning( + f"Provided link id {ref_need} for needextend does not exist. [needs]", + type="needs", + location=(current_needextend["docname"], current_needextend["lineno"]), + ) + continue + if ref_need not in need[option_name]: + need[option_name].append(ref_need) + if found_need["id"] not in all_needs[ref_need][f"{option_name}_back"]: + all_needs[ref_need][f"{option_name}_back"] += [found_need["id"]] + elif option_name in list_values: + for item in [i.strip() for i in re.split(";|,", value)]: + if item not in need[option_name]: + need[option_name].append(item) + else: + if need[option_name]: + # If content is already stored, we need to add some whitespace + need[option_name] += " " + need[option_name] += value + + elif option.startswith("-"): + option_name = option[1:] + if option_name in link_names: + # If we remove links, then remove all corresponding back links + for ref_need in (i for i in need[option_name] if i in all_needs): + all_needs[ref_need][f"{option_name}_back"].remove(found_need["id"]) + need[option_name] = [] + if option_name in list_values: + need[option_name] = [] else: - if option in link_names: - # If we change links, then modify all corresponding back links - for ref_need in (i for i in need[option] if i in all_needs): - all_needs[ref_need][f"{option}_back"].remove(found_need["id"]) - need[option] = [] - for ref_need in [i.strip() for i in re.split(";|,", value)]: - if ref_need not in all_needs: - logger.warning( - f"Provided link id {ref_need} for needextend does not exist. [needs]", - type="needs", - location=(current_needextend["docname"], current_needextend["lineno"]), - ) - continue - need[option].append(ref_need) - for ref_need in need[option]: - if found_need["id"] not in all_needs[ref_need][f"{option}_back"]: - all_needs[ref_need][f"{option}_back"] += [found_need["id"]] - elif option in list_values: - need[option] = [i.strip() for i in re.split(";|,", value)] - else: - need[option] = value - - for node in doctree.findall(Needextend): - # No printouts for needextend - removed_needextend_node(node) - - -def removed_needextend_node(node: Needextend) -> None: + need[option_name] = "" + else: + if option in link_names: + # If we change links, then modify all corresponding back links + for ref_need in (i for i in need[option] if i in all_needs): + all_needs[ref_need][f"{option}_back"].remove(found_need["id"]) + need[option] = [] + for ref_need in [i.strip() for i in re.split(";|,", value)]: + if ref_need not in all_needs: + logger.warning( + f"Provided link id {ref_need} for needextend does not exist. [needs]", + type="needs", + location=(current_needextend["docname"], current_needextend["lineno"]), + ) + continue + need[option].append(ref_need) + for ref_need in need[option]: + if found_need["id"] not in all_needs[ref_need][f"{option}_back"]: + all_needs[ref_need][f"{option}_back"] += [found_need["id"]] + elif option in list_values: + need[option] = [i.strip() for i in re.split(";|,", value)] + else: + need[option] = value + + +def remove_needextend_node(node: Needextend) -> None: """ - # Remove needextend from docutils node-tree, so that no output gets generated for it. - # Ok, this is really dirty. - # If we replace a node, docutils checks, if it will not lose any attributes. - # But this is here the case, because we are using the attribute "ids" of a node. - # However, I do not understand, why losing an attribute is such a big deal, so we delete everything - # before docutils claims about it. - - :param node: - :return: + Remove needextend from docutils node-tree, so that no output gets generated for it. + Ok, this is really dirty. + If we replace a node, docutils checks, if it will not lose any attributes. + But this is here the case, because we are using the attribute "ids" of a node. + However, I do not understand, why losing an attribute is such a big deal, so we delete everything + before docutils claims about it. """ for att in ("ids", "names", "classes", "dupnames"): diff --git a/sphinx_needs/functions/functions.py b/sphinx_needs/functions/functions.py index f184bc2da..ab17613bc 100644 --- a/sphinx_needs/functions/functions.py +++ b/sphinx_needs/functions/functions.py @@ -151,25 +151,18 @@ def find_and_replace_node_content(node, env: BuildEnvironment, need): return node -def resolve_dynamic_values(env: BuildEnvironment): +def resolve_dynamic_values(env: BuildEnvironment) -> None: """ Resolve dynamic values inside need data. Rough workflow: - #. Parse all needs and their data for a string like [[ my_func(a,b,c) ]] + #. Parse all needs and their data for a string like ``[[ my_func(a,b,c) ]]`` #. Extract function name and call parameters #. Execute registered function name with extracted call parameters #. Replace original string with return value - - :param env: Sphinx environment - :return: return value of given function """ data = SphinxNeedsData(env) - workflow = data.get_or_create_workflow() - # Only perform calculation if not already done yet - if workflow["dynamic_values_resolved"]: - return needs = data.get_or_create_needs() for need in needs.values(): @@ -229,11 +222,8 @@ def resolve_dynamic_values(env: BuildEnvironment): need[need_option] = new_values - # Finally set a flag so that this function gets not executed several times - workflow["dynamic_values_resolved"] = True - -def resolve_variants_options(env: BuildEnvironment): +def resolve_variants_options(env: BuildEnvironment) -> None: """ Resolve variants options inside need data. @@ -241,38 +231,30 @@ def resolve_variants_options(env: BuildEnvironment): #. Parse all needs and their data for variant handling #. Replace original string with return value - :param env: Sphinx environment - :return: None """ data = SphinxNeedsData(env) - workflow = data.get_or_create_workflow() - # Only perform calculation if not already done yet - if workflow["variant_option_resolved"]: - return - needs_config = NeedsSphinxConfig(env.config) variants_options = needs_config.variant_options - if variants_options: - needs = data.get_or_create_needs() - for need in needs.values(): - # Data to use as filter context. - need_context: Dict[str, Any] = {**need} - need_context.update(**needs_config.filter_data) # Add needs_filter_data to filter context - _sphinx_tags = env.app.builder.tags.tags # Get sphinx tags - need_context.update(**_sphinx_tags) # Add sphinx tags to filter context - - for var_option in variants_options: - if var_option in need and need[var_option] not in (None, "", []): - if not isinstance(need[var_option], (list, set, tuple)): - option_value: str = need[var_option] - need[var_option] = match_variants(option_value, need_context, needs_config.variants) - else: - option_value = need[var_option] - need[var_option] = match_variants(option_value, need_context, needs_config.variants) + if not variants_options: + return - # Finally set a flag so that this function gets not executed several times - workflow["variant_option_resolved"] = True + needs = data.get_or_create_needs() + for need in needs.values(): + # Data to use as filter context. + need_context: Dict[str, Any] = {**need} + need_context.update(**needs_config.filter_data) # Add needs_filter_data to filter context + _sphinx_tags = env.app.builder.tags.tags # Get sphinx tags + need_context.update(**_sphinx_tags) # Add sphinx tags to filter context + + for var_option in variants_options: + if var_option in need and need[var_option] not in (None, "", []): + if not isinstance(need[var_option], (list, set, tuple)): + option_value: str = need[var_option] + need[var_option] = match_variants(option_value, need_context, needs_config.variants) + else: + option_value = need[var_option] + need[var_option] = match_variants(option_value, need_context, needs_config.variants) def check_and_get_content(content: str, need, env: BuildEnvironment) -> str: diff --git a/sphinx_needs/need_constraints.py b/sphinx_needs/need_constraints.py index 910526f22..52db77436 100644 --- a/sphinx_needs/need_constraints.py +++ b/sphinx_needs/need_constraints.py @@ -13,19 +13,17 @@ def process_constraints(app: Sphinx) -> None: - """Analyse constraints of a single need, - and set corresponding fields on the need data item. + """Analyse constraints of all needs, + and set corresponding fields on the need data item: + ``constraints_passed`` and ``constraints_results``. + + The ``style`` field may also be changed, if a constraint fails + (depending on the config value ``constraint_failed_options``) """ env = app.env needs_config = NeedsSphinxConfig(env.config) config_constraints = needs_config.constraints needs = SphinxNeedsData(env).get_or_create_needs() - workflow = SphinxNeedsData(env).get_or_create_workflow() - - if workflow["needs_constraints"]: - return - - workflow["needs_constraints"] = True error_templates_cache: Dict[str, jinja2.Template] = {} diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py index fd61244ac..13b73d4d7 100644 --- a/sphinx_needs/needs.py +++ b/sphinx_needs/needs.py @@ -489,8 +489,6 @@ def prepare_env(app: Sphinx, env: BuildEnvironment, _docname: str) -> None: needs_config.flow_configs.update(NEEDFLOW_CONFIG_DEFAULTS) - data.get_or_create_workflow() - # Set time measurement flag if needs_config.debug_measurement: debug.START_TIME = timer() # Store the rough start time of Sphinx build