From 7c33886f7dbbbb9f8f69e759b64eadaadcf3d01b Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 6 Sep 2023 15:20:10 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=91=8C=20Improve=20`process=5Fconstra?= =?UTF-8?q?ints`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add deprecation warning for `needs_constraints_failed_color`, which is no longer used - Improve exceptions for missing keys in config - Add source location to failing constraints warnings - Improve docstrings and type annotations for data fields/config options --- sphinx_needs/config.py | 25 +++++- sphinx_needs/data.py | 9 +- sphinx_needs/need_constraints.py | 144 +++++++++++++------------------ sphinx_needs/needs.py | 13 ++- 4 files changed, 99 insertions(+), 92 deletions(-) diff --git a/sphinx_needs/config.py b/sphinx_needs/config.py index bb0c4ddc8..4e7f852f4 100644 --- a/sphinx_needs/config.py +++ b/sphinx_needs/config.py @@ -8,6 +8,12 @@ from sphinx_needs.defaults import DEFAULT_DIAGRAM_TEMPLATE, NEEDS_TABLES_CLASSES +try: + from typing import Literal, TypedDict +except ImportError: + # introduced in python 3.8 + from typing_extensions import Literal, TypedDict # type: ignore + if TYPE_CHECKING: from sphinx.util.logging import SphinxLoggerAdapter @@ -54,6 +60,17 @@ def warnings(self) -> dict[str, str | Callable[[NeedsInfoType, SphinxLoggerAdapt NEEDS_CONFIG = Config() +class ConstraintFailedType(TypedDict): + """Defines what to do if a constraint is not fulfilled""" + + on_fail: list[Literal["warn", "break"]] + """warn: log a warning, break: raise a ``NeedsConstraintFailed`` exception""" + style: list[str] + """How to style the rendered need.""" + force_style: bool + """If True, append styles to existing styles, else replace existing styles.""" + + @dataclass class NeedsSphinxConfig: """A wrapper around the Sphinx configuration, @@ -214,12 +231,14 @@ def __setattr__(self, name: str, value: Any) -> None: report_template: str = field(default="", metadata={"rebuild": "html", "types": (str,)}) """path to needs_report_template file which is based on the conf.py directory.""" - # add constraints option - constraints: dict[str, dict[str, Any]] = field(default_factory=dict, metadata={"rebuild": "html", "types": (dict,)}) - constraint_failed_options: dict[str, dict[str, Any]] = field( + constraints: dict[str, dict[str, str]] = field(default_factory=dict, metadata={"rebuild": "html", "types": (dict,)}) + """Mapping of constraint name, to check name, to filter string.""" + constraint_failed_options: dict[str, ConstraintFailedType] = field( default_factory=dict, metadata={"rebuild": "html", "types": (dict,)} ) + """Mapping of constraint severity to what to do if a constraint is not fulfilled.""" constraints_failed_color: str = field(default="", metadata={"rebuild": "html", "types": (str,)}) + """DEPRECATED: Use constraint_failed_options instead.""" # add variants option variants: dict[str, str] = field(default_factory=dict, metadata={"rebuild": "html", "types": (dict,)}) diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py index 88706c59b..3f3c9a52c 100644 --- a/sphinx_needs/data.py +++ b/sphinx_needs/data.py @@ -3,7 +3,7 @@ """ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING try: from typing import Literal, TypedDict @@ -158,10 +158,13 @@ class NeedsInfoType(NeedsBaseDataType): # back links are all set in process_need_nodes (-> create_back_links) transform # constraints information - # set in process_need_nodes (-> process_constraints) transform constraints: list[str] + """List of constraint names, which are defined for this need.""" + # set in process_need_nodes (-> process_constraints) transform + constraints_results: dict[str, dict[str, bool]] + """Mapping of constraint name, to check name, to result.""" constraints_passed: None | bool - constraints_results: dict[str, dict[str, Any]] + """True if all constraints passed, False if any failed, None if not yet checked.""" # additional source information doctype: str diff --git a/sphinx_needs/need_constraints.py b/sphinx_needs/need_constraints.py index 19f3b618a..0cee5bc2d 100644 --- a/sphinx_needs/need_constraints.py +++ b/sphinx_needs/need_constraints.py @@ -10,101 +10,77 @@ def process_constraints(app: Sphinx, need: NeedsInfoType) -> None: - """ - Finally creates the need-node in the docurils node-tree. - - :param app: sphinx app for access to config files - :param need: need object to process + """Analyse constraints of a single need, + and set corresponding fields on the need data item. """ needs_config = NeedsSphinxConfig(app.config) config_constraints = needs_config.constraints - need_id = need["id"] - constraints = need["constraints"] + # flag that is set to False if any check fails + need["constraints_passed"] = True + for constraint in constraints: - # check if constraint is defined in config - if constraint not in config_constraints.keys(): + try: + executable_constraints = config_constraints[constraint] + except KeyError: + # Note, this is already checked for in add_need raise NeedsConstraintNotAllowed( f"Constraint {constraint} of need id {need_id} is not allowed by config value 'needs_constraints'." ) - else: - # access constraints defined in conf.py - executable_constraints = config_constraints[constraint] - - # lazily gather all results to determine results_passed later - results_list = [] - - # name is check_0, check_1, ... - for name, cmd in executable_constraints.items(): - # compile constraint and check single need if it fulfills constraint - if name != "severity": - # check current need if it meets constraint given in check_0, check_1 in conf.py ... - constraint_passed = filter_single_need(app, need, cmd) - results_list.append(constraint_passed) - - if not constraint_passed: - # prepare structure per name - if constraint not in need["constraints_results"]: - need["constraints_results"][constraint] = {} - - # defines what to do if a constraint is not fulfilled. from conf.py - constraint_failed_options = needs_config.constraint_failed_options - - # prepare structure for check_0, check_1 ... - if name not in need["constraints_results"][constraint]: - need["constraints_results"][constraint][name] = {} - need["constraints_results"][constraint][name] = False + # name is check_0, check_1, ... + for name, cmd in executable_constraints.items(): + if name == "severity": + # special key, that is not a check + continue - # severity of failed constraint - severity = executable_constraints["severity"] + # compile constraint and check if need fulfils it + constraint_passed = filter_single_need(app, need, cmd) - # configurable force of constraint failed style - force_style = constraint_failed_options[severity]["force_style"] - - actions_on_fail = constraint_failed_options[severity]["on_fail"] - style_on_fail = constraint_failed_options[severity]["style"] - - if "warn" in actions_on_fail: - logger.warning( - f"Constraint {cmd} for need {need_id} FAILED! severity: {severity} [needs]", - type="needs", - color="red", - ) - - if "break" in actions_on_fail: - raise NeedsConstraintFailed( - f"FAILED a breaking constraint: >> {cmd} << for need " - f"{need_id} FAILED! breaking build process" - ) - - old_style = need["style"] - - # append to style if present - if old_style and len(old_style) > 0: - new_styles = "".join(", " + x for x in style_on_fail) - else: - old_style = "" - new_styles = "".join(x + "," for x in style_on_fail) - - if force_style: - need["style"] = new_styles.strip(", ") - else: - constraint_failed_style = old_style + new_styles - need["style"] = constraint_failed_style - - else: - # constraint is met, fill corresponding need attributes - - # prepare structure - if constraint not in need["constraints_results"].keys(): - need["constraints_results"][constraint] = {} - need["constraints_results"][constraint][name] = constraint_passed - - # access all previous results, if one check failed, set constraints_passed to False for easy filtering - if False in results_list: - need["constraints_passed"] = False + if constraint_passed: + need["constraints_results"].setdefault(constraint, {})[name] = True else: - need["constraints_passed"] = True + need["constraints_results"].setdefault(constraint, {})[name] = False + need["constraints_passed"] = False + + if "severity" not in executable_constraints: + raise NeedsConstraintFailed( + f"'severity' key not set for constraint {constraint!r} in config 'needs_constraints'" + ) + severity = executable_constraints["severity"] + if severity not in needs_config.constraint_failed_options: + raise NeedsConstraintFailed( + f"Severity {severity!r} not set in config 'needs_constraint_failed_options'" + ) + failed_options = needs_config.constraint_failed_options[severity] + + # log/except if needed + if "warn" in failed_options.get("on_fail", []): + logger.warning( + f"Constraint {cmd} for need {need_id} FAILED! severity: {severity} [needs.constraint]", + type="needs", + subtype="constraint", + color="red", + location=(need["docname"], need["lineno"]), + ) + if "break" in failed_options.get("on_fail", []): + raise NeedsConstraintFailed( + f"FAILED a breaking constraint: >> {cmd} << for need " + f"{need_id} FAILED! breaking build process" + ) + + # set styles + old_style = need["style"] + if old_style and len(old_style) > 0: + new_styles = "".join(", " + x for x in failed_options.get("style", [])) + else: + old_style = "" + new_styles = "".join(x + "," for x in failed_options.get("style", [])) + + if failed_options.get("force_style", False): + need["style"] = new_styles.strip(", ") + else: + constraint_failed_style = old_style + new_styles + need["style"] = constraint_failed_style diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py index fbe8340d8..860e4e1f4 100644 --- a/sphinx_needs/needs.py +++ b/sphinx_needs/needs.py @@ -311,7 +311,7 @@ def load_config(app: Sphinx, *_args: Any) -> None: extra_options = NEEDS_CONFIG.extra_options for option in needs_config.extra_options: if option in extra_options: - log.warning(f'extra_option "{option}" already registered. [needs]', type="needs") + log.warning(f'extra_option "{option}" already registered. [needs.config]', type="needs", subtype="config") NEEDS_CONFIG.extra_options[option] = directives.unchanged # Get extra links and create a dictionary of needed options. @@ -387,7 +387,16 @@ def load_config(app: Sphinx, *_args: Any) -> None: if name not in NEEDS_CONFIG.warnings: NEEDS_CONFIG.warnings[name] = check else: - log.warning(f'{name} for "warnings" is already registered. [needs]', type="needs") + log.warning( + f'{name} for "needs_warnings" is already registered. [needs.config]', type="needs", subtype="config" + ) + + if needs_config.constraints_failed_color: + log.warning( + 'Config option "needs_constraints_failed_color" is deprecated. Please use "needs_constraint_failed_options" styles instead. [needs.config]', + type="needs", + subtype="config", + ) def visitor_dummy(*_args: Any, **_kwargs: Any) -> None: From 3346810672ea220433ccbf3e9e99da54273da091 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 6 Sep 2023 16:09:47 +0200 Subject: [PATCH 2/2] fix test --- sphinx_needs/needs.py | 2 +- tests/test_needs_warning.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py index 3b6edfefb..0ca0d3b23 100644 --- a/sphinx_needs/needs.py +++ b/sphinx_needs/needs.py @@ -393,7 +393,7 @@ def load_config(app: Sphinx, *_args: Any) -> None: NEEDS_CONFIG.warnings[name] = check else: log.warning( - f'{name} for "needs_warnings" is already registered. [needs.config]', type="needs", subtype="config" + f"{name!r} in 'needs_warnings' is already registered. [needs.config]", type="needs", subtype="config" ) if needs_config.constraints_failed_color: diff --git a/tests/test_needs_warning.py b/tests/test_needs_warning.py index 07823ae4f..3a4f03411 100644 --- a/tests/test_needs_warning.py +++ b/tests/test_needs_warning.py @@ -13,7 +13,7 @@ def test_needs_warnings(test_app): warnings = warning.getvalue() # check multiple warning registration - assert 'invalid_status for "warnings" is already registered.' in warnings + assert "'invalid_status' in 'needs_warnings' is already registered." in warnings # check warnings contents assert "WARNING: invalid_status: failed" in warnings