🐛 FIX: NeedImport logic

Author: chrisjsewell

sphinx_needs/api/need.py

@@ -193,13 +193,6 @@ def run():
     if needs_config.id_regex and not re.match(needs_config.id_regex, need_id):
         raise NeedsInvalidException(f"Given ID '{need_id}' does not match configured regex '{needs_config.id_regex}'")
 
-    # Calculate target id, to be able to set a link back
-    if is_external:
-        target_node = None
-    else:
-        target_node = nodes.target("", "", ids=[need_id], refid=need_id, anonymous="")
-        external_url = None
-
     # Handle status
     # Check if status is in needs_statuses. If not raise an error.
     if needs_config.statuses and status not in [stat["name"] for stat in needs_config.statuses]:
@@ -310,7 +303,7 @@ def run():
         "doctype": doctype,
         "lineno": lineno,
         "target_id": need_id,
-        "external_url": external_url,
+        "external_url": external_url if is_external else None,
         "content_node": None,  # gets set after rst parsing
         "content_id": None,  # gets set after rst parsing
         "type": need_type,
@@ -443,8 +436,10 @@ def run():
     node_need.line = needs_info["lineno"]
 
     if needs_info["hide"]:
+        # add node to doctree, so we can later compute the containing section(s)
+        # (for use with section filters)
         node_need["hidden"] = True
-        return [target_node, node_need]
+        return [node_need]
 
     node_need_content = _render_template(content, docname, lineno, state)
 
@@ -485,7 +480,13 @@ def run():
     # Create a copy of the content
     needs_info["content_node"] = node_need.deepcopy()
 
-    return_nodes = [target_node] + [node_need]
+    return_nodes = [node_need]
+    if not is_external:
+        # Calculate target id, to be able to set a link back
+        target_node = nodes.target("", "", ids=[need_id], refid=need_id, anonymous="")
+        # TODO add to document?
+        return_nodes = [target_node, node_need]
+
     if pre_content:
         node_need_pre_content = _render_template(pre_content, docname, lineno, state)
         return_nodes = node_need_pre_content.children + return_nodes

sphinx_needs/directives/need.py

@@ -548,7 +548,7 @@ def remove_hidden_needs(app: Sphinx, doctree: nodes.document, fromdocname: str)
     """Remove hidden needs from the doctree, before it is rendered."""
     if fromdocname not in SphinxNeedsData(app.env).get_or_create_docs().get("all", []):
         return
-    for node_need in doctree.findall(Need):
+    for node_need in list(doctree.findall(Need)):
         if node_need.get("hidden"):
             node_need.parent.remove(node_need)  # type: ignore
 

sphinx_needs/directives/needimport.py

@@ -1,7 +1,7 @@
 import json
 import os
 import re
-from typing import Sequence
+from typing import Dict, Sequence
 from urllib.parse import urlparse
 
 import requests
@@ -12,6 +12,7 @@
 
 from sphinx_needs.api import add_need
 from sphinx_needs.config import NEEDS_CONFIG, NeedsSphinxConfig
+from sphinx_needs.data import NeedsInfoType
 from sphinx_needs.debug import measure_time
 from sphinx_needs.filter_common import filter_single_need
 from sphinx_needs.needsfile import check_needs_file
@@ -105,12 +106,11 @@ def run(self) -> Sequence[nodes.Node]:
                 for error in errors.schema:
                     logger.info(f'  {error.message} -> {".".join(error.path)}')
 
-            with open(correct_need_import_path) as needs_file:
-                needs_file_content = needs_file.read()
             try:
-                needs_import_list = json.loads(needs_file_content)
+                with open(correct_need_import_path) as needs_file:
+                    needs_import_list = json.load(needs_file)
             except json.JSONDecodeError as e:
-                # ToDo: Add exception handling
+                # TODO: Add exception handling
                 raise e
 
         if version is None:
@@ -123,8 +123,8 @@ def run(self) -> Sequence[nodes.Node]:
         if version not in needs_import_list["versions"].keys():
             raise VersionNotFound(f"Version {version} not found in needs import file {correct_need_import_path}")
 
-        # TODO type this (it uncovers lots of bugs)
-        needs_list = needs_import_list["versions"][version]["needs"]
+        # TODO this is not exactly NeedsInfoType, because the export removes/adds some keys
+        needs_list: Dict[str, NeedsInfoType] = needs_import_list["versions"][version]["needs"]
 
         # Filter imported needs
         needs_list_filtered = {}
@@ -136,7 +136,7 @@ def run(self) -> Sequence[nodes.Node]:
 
                 # Support both ways of addressing the description, as "description" is used in json file, but
                 # "content" is the sphinx internal name for this kind of information
-                filter_context["content"] = need["description"]
+                filter_context["content"] = need["description"]  # type: ignore[typeddict-item]
                 try:
                     if filter_single_need(self.env.app, filter_context, filter_string):
                         needs_list_filtered[key] = need
@@ -158,70 +158,67 @@ def run(self) -> Sequence[nodes.Node]:
                 for id in needs_list:
                     # Manipulate links in all link types
                     for extra_link in extra_links:
-                        if extra_link["option"] in need and id in need[extra_link["option"]]:
-                            for n, link in enumerate(need[extra_link["option"]]):
+                        if extra_link["option"] in need and id in need[extra_link["option"]]:  # type: ignore[literal-required]
+                            for n, link in enumerate(need[extra_link["option"]]):  # type: ignore[literal-required]
                                 if id == link:
-                                    need[extra_link["option"]][n] = "".join([id_prefix, id])
+                                    need[extra_link["option"]][n] = "".join([id_prefix, id])  # type: ignore[literal-required]
                     # Manipulate descriptions
                     # ToDo: Use regex for better matches.
-                    need["description"] = need["description"].replace(id, "".join([id_prefix, id]))
+                    need["description"] = need["description"].replace(id, "".join([id_prefix, id]))  # type: ignore[typeddict-item]
 
         # tags update
         for need in needs_list.values():
             need["tags"] = need["tags"] + tags
 
+        known_options = (
+            "title",
+            "status",
+            "content",
+            "id",
+            "tags",
+            "hide",
+            "template",
+            "pre_template",
+            "post_template",
+            "collapse",
+            "style",
+            "layout",
+            "need_type",
+            *[x["option"] for x in extra_links],
+            *NEEDS_CONFIG.extra_options,
+        )
         need_nodes = []
         for need in needs_list.values():
             # Set some values based on given option or value from imported need.
-            need["template"] = self.options.get("template", getattr(need, "template", None))
-            need["pre_template"] = self.options.get("pre_template", getattr(need, "pre_template", None))
-            need["post_template"] = self.options.get("post_template", getattr(need, "post_template", None))
-            need["layout"] = self.options.get("layout", getattr(need, "layout", None))
-            need["style"] = self.options.get("style", getattr(need, "style", None))
-            need["style"] = self.options.get("style", getattr(need, "style", None))
+            need["template"] = self.options.get("template", need.get("template"))
+            need["pre_template"] = self.options.get("pre_template", need.get("pre_template"))
+            need["post_template"] = self.options.get("post_template", need.get("post_template"))
+            need["layout"] = self.options.get("layout", need.get("layout"))
+            need["style"] = self.options.get("style", need.get("style"))
+
             if "hide" in self.options:
                 need["hide"] = True
             else:
-                need["hide"] = getattr(need, "hide", None)
-            need["collapse"] = self.options.get("collapse", getattr(need, "collapse", None))
+                need["hide"] = need.get("hide", False)
+            need["collapse"] = self.options.get("collapse", need.get("collapse"))
 
             # The key needs to be different for add_need() api call.
-            need["need_type"] = need["type"]
+            need["need_type"] = need["type"]  # type: ignore[typeddict-unknown-key]
 
             # Replace id, to get unique ids
             need["id"] = id_prefix + need["id"]
 
-            need["content"] = need["description"]
+            need["content"] = need["description"]  # type: ignore[typeddict-item]
+
             # Remove unknown options, as they may be defined in source system, but not in this sphinx project
-            extra_link_keys = [x["option"] for x in extra_links]
-            extra_option_keys = list(NEEDS_CONFIG.extra_options)
-            default_options = [
-                "title",
-                "status",
-                "content",
-                "id",
-                "tags",
-                "hide",
-                "template",
-                "pre_template",
-                "post_template",
-                "collapse",
-                "style",
-                "layout",
-                "need_type",
-            ]
-            delete_options = []
-            for option in need.keys():
-                if option not in default_options + extra_link_keys + extra_option_keys:
-                    delete_options.append(option)
-
-            for option in delete_options:
-                del need[option]
+            for option in list(need):
+                if option not in known_options:
+                    del need[option]  # type: ignore
 
             need["docname"] = self.docname
             need["lineno"] = self.lineno
 
-            nodes = add_need(self.env.app, self.state, **need)
+            nodes = add_need(self.env.app, self.state, **need)  # type: ignore[call-arg]
             need_nodes.extend(nodes)
 
         add_doc(self.env, self.env.docname)