🔧 Improve process_filters function (#1256)

This PR changes `process_filters`, so that we pass it the `NeedsView` rather than prematurely converting this to a list of needs.
In the future, this will allow us to improve filter performance.

We also parse it the "origin" node, rather than prematurely converting this to a location string.

sphinx_needs/directives/needextract.py

@@ -125,10 +125,10 @@ def process_needextract(
 
         found_needs = process_filters(
             app,
-            all_needs.values(),
+            all_needs,
             current_needextract,
             origin="needextract",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         for need_info in found_needs:

sphinx_needs/directives/needfilter.py

@@ -160,10 +160,10 @@ def process_needfilters(
 
         found_needs = process_filters(
             app,
-            all_needs.values(),
+            all_needs,
             current_needfilter,
             origin="needfilter",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         line_block = nodes.line_block()

sphinx_needs/directives/needflow/_graphviz.py

@@ -116,16 +116,16 @@ def process_needflow_graphviz(
                 allowed_link_types,
                 attributes["root_direction"],
                 attributes["root_depth"],
-            ).values()
+            )
             if (root_id := attributes["root_id"])
-            else all_needs.values()
+            else all_needs
         )
         filtered_needs = process_filters(
             app,
             init_filtered_needs,
             node.attributes,
             origin="needflow",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         if not filtered_needs:

sphinx_needs/directives/needflow/_plantuml.py

@@ -270,17 +270,17 @@ def process_needflow_plantuml(
                 allowed_link_types,
                 current_needflow["root_direction"],
                 current_needflow["root_depth"],
-            ).values()
+            )
             if (root_id := current_needflow.get("root_id"))
-            else all_needs.values()
+            else all_needs
         )
 
         found_needs = process_filters(
             app,
             need_values,
             current_needflow,
             origin="needflow",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         if found_needs:

sphinx_needs/directives/needgantt.py

@@ -194,13 +194,12 @@ def process_needgantt(
         config = current_needgantt["config"]
         puml_node["uml"] += add_config(config)
 
-        all_needs = list(all_needs_dict.values())
         found_needs = process_filters(
             app,
-            all_needs,
+            all_needs_dict,
             current_needgantt,
             origin="needgantt",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         # Scale/timeline handling

sphinx_needs/directives/needlist.py

@@ -85,13 +85,12 @@ def process_needlist(
 
         current_needfilter: NeedsListType = node.attributes
         content: list[nodes.Node] = []
-        all_needs = list(SphinxNeedsData(env).get_needs_view().values())
         found_needs = process_filters(
             app,
-            all_needs,
+            SphinxNeedsData(env).get_needs_view(),
             current_needfilter,
             origin="needlist",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         if len(found_needs) > 0:

sphinx_needs/directives/needtable.py

@@ -212,10 +212,10 @@ def process_needtables(
         # Perform filtering of needs
         filtered_needs = process_filters(
             app,
-            list(all_needs.values()),
+            all_needs,
             current_needtable,
             origin="needtable",
-            location=f"{node.source}:{node.line}",
+            location=node,
         )
 
         def get_sorter(key: str) -> Callable[[NeedsInfoType], Any]:

sphinx_needs/filter_common.py

@@ -20,6 +20,7 @@
 from sphinx_needs.data import (
     NeedsFilteredBaseType,
     NeedsInfoType,
+    NeedsView,
     SphinxNeedsData,
 )
 from sphinx_needs.debug import measure_time, measure_time_func
@@ -100,10 +101,10 @@ def collect_filter_attributes(self) -> FilterAttributesType:
 
 def process_filters(
     app: Sphinx,
-    all_needs: Iterable[NeedsInfoType],
+    needs_view: NeedsView,
     filter_data: NeedsFilteredBaseType,
     origin: str,
-    location: str,
+    location: nodes.Element,
     include_external: bool = True,
 ) -> list[NeedsInfoType]:
     """
@@ -112,39 +113,42 @@ def process_filters(
 
     :param app: Sphinx application object
     :param filter_data: Filter configuration
-    :param all_needs: List of all needs inside document
     :param origin: Origin of the request (e.g. needlist, needtable, needflow)
-    :param location: Location of the request (e.g. "docname:lineno")
+    :param location: Origin node of the request
     :param include_external: Boolean, which decides to include external needs or not
 
     :return: list of needs, which passed the filters
     """
     start = timer()
     needs_config = NeedsSphinxConfig(app.config)
+    all_needs: Iterable[NeedsInfoType]
     found_needs: list[NeedsInfoType]
     sort_key = filter_data["sort_by"]
     if sort_key:
         try:
-            all_needs = sorted(all_needs, key=lambda node: node[sort_key] or "")  # type: ignore[literal-required]
+            all_needs = sorted(
+                needs_view.values(),
+                key=lambda node: node[sort_key] or "",  # type: ignore[literal-required]
+            )
         except KeyError as e:
             log_warning(
-                log, f"Sorting parameter {sort_key} not valid: Error: {e}", None, None
+                log,
+                f"Sorting parameter {sort_key} not valid: Error: {e}",
+                None,
+                location=location,
             )
+            return []
+    else:
+        all_needs = needs_view.values()
 
     # check if include external needs
-    checked_all_needs: Iterable[NeedsInfoType]
     if not include_external:
-        checked_all_needs = []
-        for need in all_needs:
-            if not need["is_external"]:
-                checked_all_needs.append(need)
-    else:
-        checked_all_needs = all_needs
+        all_needs = [need for need in all_needs if not need["is_external"]]
 
     found_needs_by_options: list[NeedsInfoType] = []
 
     # Add all need_parts of given needs to the search list
-    all_needs_incl_parts = prepare_need_list(checked_all_needs)
+    all_needs_incl_parts = prepare_need_list(all_needs)
 
     # Check if external filter code is defined
     try:
@@ -199,7 +203,7 @@ def process_filters(
                 all_needs_incl_parts,
                 needs_config,
                 filter_data["filter"],
-                location=(filter_data["docname"], filter_data["lineno"]),
+                location=location,
             )
             # Make an intersection of both lists
             found_needs = intersection_of_need_results(
@@ -212,7 +216,7 @@ def process_filters(
                 all_needs_incl_parts,
                 needs_config,
                 filter_data["filter"],
-                location=(filter_data["docname"], filter_data["lineno"]),
+                location=location,
             )
     else:
         # Provides only a copy of needs to avoid data manipulations.
@@ -237,7 +241,9 @@ def process_filters(
             )
             filter_func(**context)
         else:
-            log_warning(log, "Something went wrong running filter", None, None)
+            log_warning(
+                log, "Something went wrong running filter", None, location=location
+            )
             return []
 
         # The filter results may be dirty, as it may continue manipulated needs.
@@ -260,7 +266,7 @@ def process_filters(
 
     filter_list[filter_data["target_id"]] = {
         "origin": origin,
-        "location": location,
+        "location": f"{location.source}:{location.line}",
         "filter": filter_data["filter"] or "",
         "status": filter_data["status"],
         "tags": filter_data["tags"],

tests/doc_test/filter_doc/index.rst

@@ -57,4 +57,7 @@ Testing bad filters
 .. needlist::
    :filter: yyy
 
+.. needlist::
+   :sort_by: yyy
+
 :need_count:`zzz`

tests/test_filter.py

@@ -1,3 +1,4 @@
+import os
 from pathlib import Path
 
 import pytest
@@ -12,14 +13,17 @@
 def test_filter_build_html(test_app):
     app = test_app
     app.build()
-    warnings = strip_colors(app._warning.getvalue())
-    print(warnings)
+    warnings = strip_colors(app._warning.getvalue()).replace(
+        str(app.srcdir) + os.path.sep, "<srcdir>/"
+    )
+    # print(warnings.splitlines())
 
     expected_warnings = [
-        f"{Path(str(app.srcdir)) / 'index.rst'}:51: WARNING: Filter 'xxx' not valid. Error: name 'xxx' is not defined. [needs.filter]",
-        f"{Path(str(app.srcdir)) / 'index.rst'}:54: WARNING: Filter '1' not valid. Error: Filter did not evaluate to a boolean, instead <class 'int'>: 1. [needs.filter]",
-        f"{Path(str(app.srcdir)) / 'index.rst'}:57: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
-        f"{Path(str(app.srcdir)) / 'index.rst'}:60: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
+        "<srcdir>/index.rst:51: WARNING: Filter 'xxx' not valid. Error: name 'xxx' is not defined. [needs.filter]",
+        "<srcdir>/index.rst:54: WARNING: Filter '1' not valid. Error: Filter did not evaluate to a boolean, instead <class 'int'>: 1. [needs.filter]",
+        "<srcdir>/index.rst:57: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
+        "<srcdir>/index.rst:60: WARNING: Sorting parameter yyy not valid: Error: 'yyy' [needs]",
+        "<srcdir>/index.rst:63: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
     ]
 
     assert warnings.splitlines() == expected_warnings