👌 Improve warnings for invalid filters (add source location and subtype) (#1128)

Author: chrisjsewell

sphinx_needs/api/need.py

@@ -153,8 +153,10 @@ def run():
     if need_type not in configured_need_types:
         logger.warning(
             f"Couldn't create need {id}. Reason: The need-type (i.e. `{need_type}`) is not set "
-            "in the project's 'need_types' configuration in conf.py. [needs]",
+            "in the project's 'need_types' configuration in conf.py. [needs.add]",
             type="needs",
+            subtype="add",
+            location=(docname, lineno) if docname else None,
         )
 
     for ntype in types:
@@ -219,9 +221,11 @@ def run():
         for i in range(len(tags)):
             if len(tags[i]) == 0 or tags[i].isspace():
                 logger.warning(
-                    f"Scruffy tag definition found in need {need_id}. "
-                    "Defined tag contains spaces only. [needs]",
+                    f"Scruffy tag definition found in need {need_id!r}. "
+                    "Defined tag contains spaces only. [needs.add]",
                     type="needs",
+                    subtype="add",
+                    location=(docname, lineno) if docname else None,
                 )
             else:
                 new_tags.append(tags[i])
@@ -256,9 +260,11 @@ def run():
         for i in range(len(constraints)):
             if len(constraints[i]) == 0 or constraints[i].isspace():
                 logger.warning(
-                    f"Scruffy tag definition found in need {need_id}. "
-                    "Defined constraint contains spaces only. [needs]",
+                    f"Scruffy constraint definition found in need {need_id!r}. "
+                    "Defined constraint contains spaces only. [needs.add]",
                     type="needs",
+                    subtype="add",
+                    location=(docname, lineno) if docname else None,
                 )
             else:
                 new_constraints.append(constraints[i])

sphinx_needs/builder.py

@@ -85,7 +85,10 @@ def finish(self) -> None:
 
         filter_string = needs_config.builder_filter
         filtered_needs: list[NeedsInfoType] = filter_needs(
-            data.get_or_create_needs().values(), needs_config, filter_string
+            data.get_or_create_needs().values(),
+            needs_config,
+            filter_string,
+            append_warning="(from need_builder_filter)",
         )
 
         for need in filtered_needs:
@@ -182,7 +185,12 @@ def finish(self) -> None:
         filter_string = needs_config.builder_filter
         from sphinx_needs.filter_common import filter_needs
 
-        filtered_needs = filter_needs(needs, needs_config, filter_string)
+        filtered_needs = filter_needs(
+            needs,
+            needs_config,
+            filter_string,
+            append_warning="(from need_builder_filter)",
+        )
         needs_build_json_per_id_path = needs_config.build_json_per_id_path
         needs_dir = os.path.join(self.outdir, needs_build_json_per_id_path)
         if not os.path.exists(needs_dir):

sphinx_needs/directives/needbar.py

@@ -157,7 +157,10 @@ def run(self) -> Sequence[nodes.Node]:
 
         add_doc(env, env.docname)
 
-        return [targetnode, Needbar("")]
+        bar_node = Needbar("")
+        self.set_source_info(bar_node)
+
+        return [targetnode, bar_node]
 
 
 # Algorithm:
@@ -301,7 +304,9 @@ def process_needbar(
                 if element.isdigit():
                     line_number.append(float(element))
                 else:
-                    result = len(filter_needs(need_list, needs_config, element))
+                    result = len(
+                        filter_needs(need_list, needs_config, element, location=node)
+                    )
                     line_number.append(float(result))
             local_data_number.append(line_number)
 

sphinx_needs/directives/needextend.py

@@ -98,15 +98,12 @@ def extend_needs_data(
                 logger.info(error)
                 continue
         else:
-            # a filter string
-            try:
-                found_needs = filter_needs(
-                    all_needs.values(), needs_config, need_filter
-                )
-            except NeedsInvalidFilter as e:
-                raise NeedsInvalidFilter(
-                    f"Filter not valid for needextend on page {current_needextend['docname']}:\n{e}"
-                )
+            found_needs = filter_needs(
+                all_needs.values(),
+                needs_config,
+                need_filter,
+                location=(current_needextend["docname"], current_needextend["lineno"]),
+            )
 
         for found_need in found_needs:
             # Work in the stored needs, not on the search result

sphinx_needs/directives/needgantt.py

@@ -124,7 +124,10 @@ def run(self) -> Sequence[nodes.Node]:
 
         add_doc(env, env.docname)
 
-        return [targetnode] + [Needgantt("")]
+        gantt_node = Needgantt("")
+        self.set_source_info(gantt_node)
+
+        return [targetnode, gantt_node]
 
     def get_link_type_option(self, name: str, default: str = "") -> list[str]:
         link_types = [
@@ -244,10 +247,17 @@ def process_needgantt(
                 complete_option = current_needgantt["completion_option"]
                 complete = need[complete_option]  # type: ignore[literal-required]
                 if not (duration and duration.isdigit()):
+                    need_location = (
+                        f" (located: {need['docname']}:{need['lineno']})"
+                        if need["docname"]
+                        else ""
+                    )
                     logger.warning(
                         "Duration not set or invalid for needgantt chart. "
-                        "Need: {}. Duration: {} [needs]".format(need["id"], duration),
+                        f"Need: {need['id']!r}{need_location}. Duration: {duration!r} [needs.gantt]",
                         type="needs",
+                        subtype="gantt",
+                        location=node,
                     )
                     duration = 1
                 gantt_element = "[{}] as [{}] lasts {} days\n".format(

sphinx_needs/directives/needpie.py

@@ -102,7 +102,10 @@ def run(self) -> Sequence[nodes.Node]:
         }
         add_doc(env, env.docname)
 
-        return [targetnode, Needpie("")]
+        pie_node = Needpie("")
+        self.set_source_info(pie_node)
+
+        return [targetnode, pie_node]
 
 
 @measure_time("needpie")
@@ -162,7 +165,9 @@ def process_needpie(
                 if line.isdigit():
                     sizes.append(abs(float(line)))
                 else:
-                    result = len(filter_needs(need_list, needs_config, line))
+                    result = len(
+                        filter_needs(need_list, needs_config, line, location=node)
+                    )
                     sizes.append(result)
         elif current_needpie["filter_func"] and not content:
             try:

sphinx_needs/filter_common.py

@@ -9,6 +9,7 @@
 from types import CodeType
 from typing import Any, Iterable, TypedDict, TypeVar
 
+from docutils import nodes
 from docutils.parsers.rst import directives
 from sphinx.application import Sphinx
 from sphinx.util.docutils import SphinxDirective
@@ -180,7 +181,10 @@ def process_filters(
                     found_needs_by_options.append(need_info)
             # Get need by filter string
             found_needs_by_string = filter_needs(
-                all_needs_incl_parts, needs_config, filter_data["filter"]
+                all_needs_incl_parts,
+                needs_config,
+                filter_data["filter"],
+                location=(filter_data["docname"], filter_data["lineno"]),
             )
             # Make an intersection of both lists
             found_needs = intersection_of_need_results(
@@ -190,7 +194,10 @@ def process_filters(
             # There is no other config as the one for filter string.
             # So we only need this result.
             found_needs = filter_needs(
-                all_needs_incl_parts, needs_config, filter_data["filter"]
+                all_needs_incl_parts,
+                needs_config,
+                filter_data["filter"],
+                location=(filter_data["docname"], filter_data["lineno"]),
             )
     else:
         # Provides only a copy of needs to avoid data manipulations.
@@ -296,6 +303,9 @@ def filter_needs(
     config: NeedsSphinxConfig,
     filter_string: None | str = "",
     current_need: NeedsInfoType | None = None,
+    *,
+    location: tuple[str, int | None] | nodes.Node | None = None,
+    append_warning: str = "",
 ) -> list[V]:
     """
     Filters given needs based on a given filter string.
@@ -305,6 +315,8 @@ def filter_needs(
     :param config: NeedsSphinxConfig object
     :param filter_string: strings, which gets evaluated against each need
     :param current_need: current need, which uses the filter.
+    :param location: source location for error reporting (docname, line number)
+    :param append_warning: additional text to append to any failed filter warning
 
     :return: list of found needs
     """
@@ -328,13 +340,15 @@ def filter_needs(
             ):
                 found_needs.append(filter_need)
         except Exception as e:
-            if not error_reported:  # Let's report a filter-problem only onces
-                location = (
-                    (current_need["docname"], current_need["lineno"])
-                    if current_need
-                    else None
+            if not error_reported:  # Let's report a filter-problem only once
+                if append_warning:
+                    append_warning = f" {append_warning}"
+                log.warning(
+                    f"{e}{append_warning} [needs.filter]",
+                    type="needs",
+                    subtype="filter",
+                    location=location,
                 )
-                log.warning(str(e) + " [needs]", type="needs", location=location)
                 error_reported = True
 
     return found_needs
@@ -381,5 +395,5 @@ def filter_single_need(
         else:
             result = bool(eval(filter_string, filter_context))
     except Exception as e:
-        raise NeedsInvalidFilter(f"Filter {filter_string} not valid. Error: {e}.")
+        raise NeedsInvalidFilter(f"Filter {filter_string!r} not valid. Error: {e}.")
     return result

sphinx_needs/functions/common.py

@@ -166,7 +166,11 @@ def copy(
 
     if filter:
         result = filter_needs(
-            needs.values(), NeedsSphinxConfig(app.config), filter, need
+            needs.values(),
+            NeedsSphinxConfig(app.config),
+            filter,
+            need,
+            location=(need["docname"], need["lineno"]),
         )
         if result:
             need = result[0]

sphinx_needs/roles/need_count.py

@@ -37,11 +37,28 @@ def process_need_count(
             filters = filter.split(" ? ")
             if len(filters) == 1:
                 need_list = prepare_need_list(all_needs)  # adds parts to need_list
-                amount = str(len(filter_needs(need_list, needs_config, filters[0])))
+                amount = str(
+                    len(
+                        filter_needs(
+                            need_list,
+                            needs_config,
+                            filters[0],
+                            location=node_need_count,
+                        )
+                    )
+                )
             elif len(filters) == 2:
                 need_list = prepare_need_list(all_needs)  # adds parts to need_list
-                amount_1 = len(filter_needs(need_list, needs_config, filters[0]))
-                amount_2 = len(filter_needs(need_list, needs_config, filters[1]))
+                amount_1 = len(
+                    filter_needs(
+                        need_list, needs_config, filters[0], location=node_need_count
+                    )
+                )
+                amount_2 = len(
+                    filter_needs(
+                        need_list, needs_config, filters[1], location=node_need_count
+                    )
+                )
                 amount = f"{amount_1 / amount_2 * 100:2.1f}"
             elif len(filters) > 2:
                 raise NeedsInvalidFilter(

sphinx_needs/warnings.py

@@ -62,7 +62,10 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
             if isinstance(warning_filter, str):
                 # filter string used
                 result = filter_needs(
-                    checked_needs.values(), needs_config, warning_filter
+                    checked_needs.values(),
+                    needs_config,
+                    warning_filter,
+                    append_warning=f"(from warning filter {warning_name!r})",
                 )
             elif callable(warning_filter):
                 # custom defined filter code used from conf.py