‼️ Disallow add_extra_option overriding an internal field (#1477)

This is an issue, because all need fields (internal and custom user
ones) are stored / serialized in a "flat" dictionary, and so they cannot
overlap.

Previously there was no check in place to disallow the API from doing
this

Author: chrisjsewell

docs/conf.py

@@ -40,7 +40,7 @@
 if DOCS_THEME == "sphinx_immaterial":
     extensions.append("sphinx_immaterial")
 
-suppress_warnings = ["needs.link_outgoing"]
+suppress_warnings = ["needs.link_outgoing", "needs.github"]
 
 nitpicky = True
 nitpick_ignore = [

sphinx_needs/api/need.py

@@ -776,17 +776,10 @@ def _add_extra_fields(
     needs_info: NeedsInfoType, kwargs: dict[str, Any], config: NeedsSphinxConfig
 ) -> None:
     """Add extra option fields to the needs_info dictionary."""
-    extra_keys = set(kwargs).difference(set(needs_info))
-    for key in config.extra_options:
-        if key in extra_keys:
-            # TODO really we should not need to do this,
-            # but the `add_extra_option` function does not guard against the keys clashing with existing internal fields,
-            # this occurs in the core code with the github service adding `type` as an extra option
-
-            # note we already warn if value not string in external/import code
-            needs_info[key] = str(kwargs.get(key, ""))
-        elif key not in needs_info:
-            needs_info[key] = ""
+    # note we already warn if value not string in external/import code,
+    # but still allow it and convert it here, for backward-comptibility
+    extras = {key: str(kwargs.get(key, "")) for key in config.extra_options}
+    needs_info.update(extras)  # type: ignore[typeddict-item]
 
 
 def _add_link_fields(

sphinx_needs/config.py

@@ -121,6 +121,16 @@ def add_extra_option(
         override: bool = False,
     ) -> None:
         """Adds an extra option to the configuration."""
+        if name in NeedsCoreFields:
+            from sphinx_needs.exceptions import (
+                NeedsApiConfigWarning,  # avoid circular import
+            )
+
+            raise NeedsApiConfigWarning(
+                f"Cannot add extra option with name {name!r}"
+                + (f" ({description!r})" if description else "")
+                + ", as it is already used as a core field name."
+            )
         if name in self._extra_options:
             if override:
                 log_warning(

sphinx_needs/services/manager.py

@@ -2,6 +2,7 @@
 
 from typing import Any
 
+from docutils.parsers.rst import directives
 from sphinx.application import Sphinx
 
 from sphinx_needs.config import _NEEDS_CONFIG, NeedsSphinxConfig
@@ -28,7 +29,11 @@ def register(self, name: str, klass: type[BaseService], **kwargs: Any) -> None:
 
         # Register options from service class
         for option in klass.options:
-            if option not in _NEEDS_CONFIG.extra_options:
+            if option == "type":
+                # TODO this should probably be done a bit more systematically;
+                # the github service adds a "type" option, but this is related to the core need field NOT an extra option
+                NeedserviceDirective.option_spec["type"] = directives.unchanged
+            elif option not in _NEEDS_CONFIG.extra_options:
                 self.log.debug(f'Register option "{option}" for service "{name}"')
                 _NEEDS_CONFIG.add_extra_option(option, f"Added by service {name}")
                 # Register new option directly in Service directive, as its class options got already