♻️ Default to warning for missing needextend ID (#1066)

Rather than defaulting to "strict" mode for unknown IDs in the `needextend` directives, whereby the entire build is excepted,
The default is now changed to "non-strict" mode, and this emits a warning rather than an info log.

docs/directives/needextend.rst

@@ -81,22 +81,20 @@ Options
 
 strict
 ~~~~~~
-The purpose of the ``:strict:`` option is to handle whether an exception gets thrown
-or a log-info message gets written if there is no need object to match the ``needextend's``
-required argument (e.g. an ID).
+The purpose of the ``:strict:`` option is to handle whether an exception gets thrown or a warning log message gets written, if there is no need object to match the ``needextend's`` required argument (e.g. an ID).
 
 If you set the ``:strict:`` option value to ``true``,
 ``needextend`` raises an exception because the given ID does not exist, and the build stops.
 
 If you set  the ``:strict:`` option value to ``false``,
-``needextend`` logs an info-level message in the console, and the build continues.
+``needextend`` logs an warning-level message in the console, and the build continues.
 
 Allowed values:
 
 * true or
 * false
 
-Default: true
+Default: false
 
 .. note::
 

sphinx_needs/config.py

@@ -289,7 +289,7 @@ def __setattr__(self, name: str, value: Any) -> None:
     )
     """Added to options on need directives, and key on need data items, for use by ``needgantt``"""
     needextend_strict: bool = field(
-        default=True, metadata={"rebuild": "html", "types": (bool,)}
+        default=False, metadata={"rebuild": "html", "types": (bool,)}
     )
     statuses: list[dict[str, str]] = field(
         default_factory=list, metadata={"rebuild": "html", "types": ()}

sphinx_needs/directives/needextend.py

@@ -48,13 +48,11 @@ def run(self) -> Sequence[nodes.Node]:
                 f"Filter of needextend must be set. See {env.docname}:{self.lineno}"
             )
 
-        strict_option = self.options.get(
-            "strict", str(NeedsSphinxConfig(self.env.app.config).needextend_strict)
-        )
-        strict = True
-        if strict_option.upper() == "TRUE":
+        strict = NeedsSphinxConfig(self.env.app.config).needextend_strict
+        strict_option: str = self.options.get("strict", "").upper()
+        if strict_option == "TRUE":
             strict = True
-        elif strict_option.upper() == "FALSE":
+        elif strict_option == "FALSE":
             strict = False
 
         data = SphinxNeedsData(env).get_or_create_extends()
@@ -91,19 +89,43 @@ def extend_needs_data(
             needs_config.id_regex, need_filter
         ):
             # an unknown ID
-            error = f"Provided id {need_filter} for needextend does not exist."
+            error = f"Provided id {need_filter!r} for needextend does not exist. [needs.extend]"
             if current_needextend["strict"]:
                 raise NeedsInvalidFilter(error)
             else:
-                logger.info(error)
-                continue
+                logger.warning(
+                    error,
+                    type="needs",
+                    subtype="extend",
+                    location=(
+                        current_needextend["docname"],
+                        current_needextend["lineno"],
+                    ),
+                )
+            continue
         else:
-            found_needs = filter_needs(
-                all_needs.values(),
-                needs_config,
-                need_filter,
-                location=(current_needextend["docname"], current_needextend["lineno"]),
-            )
+            # a filter string
+            try:
+                found_needs = filter_needs(
+                    all_needs.values(),
+                    needs_config,
+                    need_filter,
+                    location=(
+                        current_needextend["docname"],
+                        current_needextend["lineno"],
+                    ),
+                )
+            except NeedsInvalidFilter as e:
+                logger.warning(
+                    f"Invalid filter {need_filter!r}: {e} [needs.extend]",
+                    type="needs",
+                    subtype="extend",
+                    location=(
+                        current_needextend["docname"],
+                        current_needextend["lineno"],
+                    ),
+                )
+                continue
 
         for found_need in found_needs:
             # Work in the stored needs, not on the search result

tests/doc_test/doc_needextend_strict/index.rst → tests/doc_test/doc_needextend_unknown_id/index.rst

@@ -1,5 +1,5 @@
-needextend strict-mode disabled
-===============================
+needextend unknown id
+=====================
 
 .. story:: needextend Example 3
    :id: extend_test_003
@@ -16,9 +16,5 @@ needextend strict-mode disabled
 .. needextend:: extend_test_003
    :links: extend_test_004
 
-.. needextend:: strict_disable_extend_test
+.. needextend:: unknown_id
    :status: open
-   :strict: false
-
-.. needextend:: strict_enable_extend_test
-   :status: closed

tests/test_needextend.py

@@ -1,9 +1,9 @@
 import json
-import sys
 from pathlib import Path
 
 import pytest
 from sphinx.application import Sphinx
+from sphinx.util.console import strip_colors
 from syrupy.filters import props
 
 
@@ -45,38 +45,23 @@ def test_doc_needextend_html(test_app: Sphinx, snapshot):
 
 @pytest.mark.parametrize(
     "test_app",
-    [{"buildername": "html", "srcdir": "doc_test/doc_needextend_strict"}],
+    [
+        {
+            "buildername": "html",
+            "srcdir": "doc_test/doc_needextend_unknown_id",
+            "no_plantuml": True,
+        }
+    ],
     indirect=True,
 )
-def test_doc_needextend_strict(test_app):
-    import os
-    import subprocess
-
+def test_doc_needextend_unknown_id(test_app: Sphinx):
     app = test_app
+    app.build()
 
-    srcdir = Path(app.srcdir)
-    out_dir = os.path.join(srcdir, "_build")
-
-    out = subprocess.run(
-        ["sphinx-build", "-b", "html", srcdir, out_dir], capture_output=True
-    )
-
-    # Strict option is set to false on needextend. Log info-level message
-    assert (
-        "Provided id strict_disable_extend_test for needextend does not exist."
-        in out.stdout.decode("utf-8")
-    )
-    # Strict option is set to true on needextend. Raise Exception
-    if sys.platform == "win32":
-        assert (
-            "Sphinx error:\r\nProvided id strict_enable_extend_test for needextend does not exist."
-            in out.stderr.decode("utf-8")
-        )
-    else:
-        assert (
-            "Sphinx error:\nProvided id strict_enable_extend_test for needextend does not exist."
-            in out.stderr.decode("utf-8")
-        )
+    warnings = strip_colors(app._warning.getvalue()).splitlines()
+    assert warnings == [
+        f"{Path(str(app.srcdir)) / 'index.rst'}:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.extend]"
+    ]
 
 
 @pytest.mark.parametrize(

tests/test_needs_builder.py

@@ -79,7 +79,7 @@ def test_needs_html_and_json(test_app):
     build_dir = os.path.join(app.outdir, "../needs")
     print(build_dir)
     output = subprocess.run(
-        ["python", "-m", "sphinx.cmd.build", "-b", "needs", srcdir, build_dir],
+        ["sphinx-build", "-b", "needs", srcdir, build_dir],
         capture_output=True,
     )
     print(output)