✨ Add error messages for constraint failures (#1036)

This commit adds the ability to set an optional `error_message` key for each constraint in the `needs_constraint` configuration.
This can contain Jinja placeholders, for needs data fields, and will be added to the need data, under the `constraints_error` field, if the constraint fails.

docs/configuration.rst

@@ -1939,6 +1939,22 @@ constraints_passed is a bool showing if ALL constraints of a corresponding need
 
 constraints_results is a dictionary similar in structure to needs_constraints above. Instead of executable python statements, inner values contain a bool describing if check_0, check_1 ... passed successfully.
 
+.. versionadded:: 1.4.0
+
+    The ``"error_message"`` key can contain a string, with Jinja templating, which will be displayed if the constraint fails, and saved on the need as ``constraints_error``:
+
+    .. code-block:: python
+
+        needs_constraints = {
+
+            "critical": {
+                "check_0": "'critical' in tags",
+                "severity": "CRITICAL",
+                "error_message": "need {% raw %}{{id}}{% endraw %} does not fulfill CRITICAL constraint, because tags are {% raw %}{{tags}}{% endraw %}"
+            }
+        
+        }
+
 
 .. code-block:: rst
 

sphinx_needs/config.py

@@ -232,7 +232,12 @@ def __setattr__(self, name: str, value: Any) -> None:
     """path to needs_report_template file which is based on the conf.py directory."""
 
     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."""
+    """Mapping of constraint name, to check name, to filter string.
+    There are also some special keys for a constraint:
+
+    - severity: The severity of the constraint. This is used to determine what to do if the constraint is not fulfilled.
+    - error_message: A help text for the constraint, can include Jinja2 variables.
+    """
     constraint_failed_options: dict[str, ConstraintFailedType] = field(
         default_factory=dict, metadata={"rebuild": "html", "types": (dict,)}
     )

sphinx_needs/data.py

@@ -44,6 +44,7 @@ class NeedsWorkflowType(TypedDict):
     add_sections: bool
     variant_option_resolved: bool
     needs_extended: bool
+    needs_constraints: bool
 
 
 class NeedsBaseDataType(TypedDict):
@@ -166,6 +167,8 @@ class NeedsInfoType(NeedsBaseDataType):
     """Mapping of constraint name, to check name, to result."""
     constraints_passed: None | bool
     """True if all constraints passed, False if any failed, None if not yet checked."""
+    constraints_error: str
+    """An error message set if any constraint failed, and `error_message` field is set in config."""
 
     # additional source information
     doctype: str
@@ -460,8 +463,8 @@ def get_or_create_workflow(self) -> NeedsWorkflowType:
                 "add_sections": False,
                 "variant_option_resolved": False,
                 "needs_extended": False,
+                "needs_constraints": False,
             }
-            # TODO use needs_config here
             for link_type in self.env.app.config.needs_extra_links:
                 self.env.needs_workflow["backlink_creation_{}".format(link_type["option"])] = False
 

sphinx_needs/directives/need.py

@@ -394,28 +394,13 @@ def process_need_nodes(app: Sphinx, doctree: nodes.document, fromdocname: str) -
     for links in needs_config.extra_links:
         create_back_links(env, links["option"])
 
-    """
-    The output of this phase is a doctree for each source file; that is a tree of docutils nodes.
-
-    https://www.sphinx-doc.org/en/master/extdev/index.html
-
-    """
-    needs = SphinxNeedsData(env).get_or_create_needs()
-
-    # Used to store needs in the docs, which are needed again later
-    found_needs_nodes = []
-    for node_need in doctree.findall(Need):
-        need_id = node_need.attributes["ids"][0]
-        found_needs_nodes.append(node_need)
-        need_data = needs[need_id]
-
-        process_constraints(app, need_data)
+    process_constraints(app)
 
     # 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)
 
-    print_need_nodes(app, doctree, fromdocname, found_needs_nodes)
+    print_need_nodes(app, doctree, fromdocname, list(doctree.findall(Need)))
 
 
 @profile("NEED_PRINT")

sphinx_needs/need_constraints.py

@@ -1,86 +1,106 @@
+from typing import Dict
+
+import jinja2
 from sphinx.application import Sphinx
 
 from sphinx_needs.api.exceptions import NeedsConstraintFailed, NeedsConstraintNotAllowed
 from sphinx_needs.config import NeedsSphinxConfig
-from sphinx_needs.data import NeedsInfoType
+from sphinx_needs.data import SphinxNeedsData
 from sphinx_needs.filter_common import filter_single_need
 from sphinx_needs.logging import get_logger
 
 logger = get_logger(__name__)
 
 
-def process_constraints(app: Sphinx, need: NeedsInfoType) -> None:
+def process_constraints(app: Sphinx) -> None:
     """Analyse constraints of a single need,
     and set corresponding fields on the need data item.
     """
-    needs_config = NeedsSphinxConfig(app.config)
+    env = app.env
+    needs_config = NeedsSphinxConfig(env.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:
-        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'."
-            )
-
-        # name is check_0, check_1, ...
-        for name, cmd in executable_constraints.items():
-            if name == "severity":
-                # special key, that is not a check
-                continue
-
-            # compile constraint and check if need fulfils it
-            constraint_passed = filter_single_need(app, need, cmd)
-
-            if constraint_passed:
-                need["constraints_results"].setdefault(constraint, {})[name] = True
-            else:
-                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", []))
+    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] = {}
+
+    for need in needs.values():
+        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:
+            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'."
+                )
 
-                if failed_options.get("force_style", False):
-                    need["style"] = new_styles.strip(", ")
+            # name is check_0, check_1, ...
+            for name, cmd in executable_constraints.items():
+                if name in ("severity", "error_message"):
+                    # special keys, that are not a check
+                    continue
+
+                # compile constraint and check if need fulfils it
+                constraint_passed = filter_single_need(app, need, cmd)
+
+                if constraint_passed:
+                    need["constraints_results"].setdefault(constraint, {})[name] = True
                 else:
-                    constraint_failed_style = old_style + new_styles
-                    need["style"] = constraint_failed_style
+                    need["constraints_results"].setdefault(constraint, {})[name] = False
+                    need["constraints_passed"] = False
+
+                    if "error_message" in executable_constraints:
+                        msg = str(executable_constraints["error_message"])
+                        template = error_templates_cache.setdefault(msg, jinja2.Template(msg))
+                        need["constraints_error"] = template.render(**need)
+
+                    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} {need.get('constraints_error', '')} [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

tests/__snapshots__/test_external.ambr

@@ -17,7 +17,7 @@
             'completion': '',
             'constraints': list([
             ]),
-            'constraints_passed': None,
+            'constraints_passed': True,
             'constraints_results': dict({
             }),
             'content_id': None,
@@ -89,7 +89,7 @@
             'completion': '',
             'constraints': list([
             ]),
-            'constraints_passed': None,
+            'constraints_passed': True,
             'constraints_results': dict({
             }),
             'content_id': None,