Skip to content

Commit 25b92fa

Browse files
authored
🐛 Register sphinx-needs fields with a typed schema (#80)
## Problem sphinx-codelinks registers its fields (`project`, `file`, `directory`, and the local/remote url fields) via `add_extra_option(app, name)` **without a schema**. Untyped fields default to `""` on *every* need, so when a project enables strict sphinx-needs schema validation (`unevaluatedProperties: false`) those empty fields are reported as unexpected on needs that never set them: Need 'REQ_1' has schema violations: Unevaluated properties are not allowed ('directory', 'file', 'project' were unexpected) This is the codelinks-side counterpart of the same bug found and fixed in sphinx-test-reports. ## Fix Register each field with a typed (string) schema so an unset value defaults to `None`. sphinx-needs strips `None` from the "reduced need" before schema validation, so a strict `unevaluatedProperties: false` schema no longer flags a field a need never set (an untyped field stays `""`, which is not stripped). Registration adapts to the installed sphinx-needs by **capability detection** (no `packaging` dependency, matching sphinx-test-reports): - `add_field(name, description, schema=...)` — the modern API, used wherever it is importable. - otherwise `add_extra_option(app, name, schema=...)` when `add_extra_option` accepts a `schema` keyword (sphinx-needs >= 6). - otherwise `add_extra_option(app, name)` for sphinx-needs 5, which has no schema validation, so the empty-default bug cannot arise there anyway. The `schema` capability is detected with `"schema" in signature(add_extra_option).parameters`. ## Test Adds `tests/doc_test/schema_strictness/` + `tests/test_schema_validation.py`: a strict-schema project with a plain need, asserting no schema violation. Confirmed **red before the fix** (`'directory', 'file', 'project' were unexpected`) and **green after**. The test is skipped when `add_extra_option` has no `schema` keyword (sphinx-needs < 6). Local checks: `ruff` + `mypy` clean; regression test green on sphinx-needs 6/7/8 and correctly skipped on 5; existing src-trace doctree snapshots unchanged.
1 parent 32156a7 commit 25b92fa

6 files changed

Lines changed: 145 additions & 15 deletions

File tree

src/sphinx_codelinks/sphinx_extension/directives/src_trace.py

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66

77
from docutils import nodes
88
from docutils.parsers.rst import directives
9-
from packaging.version import Version
10-
import sphinx
9+
from sphinx.util import logging
1110
from sphinx.util.docutils import SphinxDirective
1211
from sphinx_needs.api import add_need # type: ignore[import-untyped]
1312
from sphinx_needs.utils import add_doc # type: ignore[import-untyped]
@@ -23,14 +22,6 @@
2322
from sphinx_codelinks.source_discover.source_discover import SourceDiscover
2423
from sphinx_codelinks.sphinx_extension.debug import measure_time
2524

26-
sphinx_version = sphinx.__version__
27-
28-
29-
if Version(sphinx_version) >= Version("1.6"):
30-
from sphinx.util import logging
31-
else:
32-
import logging # type: ignore[no-redef]
33-
3425
logger = logging.getLogger(__name__)
3526

3627

src/sphinx_codelinks/sphinx_extension/source_tracing.py

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from collections.abc import Iterator # only in python 3.11 afterwards
22
import contextlib
3+
from inspect import signature
34
from pathlib import Path
45
from timeit import default_timer as timer # Used for timing measurements
56
import tomllib
@@ -35,6 +36,34 @@
3536

3637
logger = logging.getLogger(__name__)
3738

39+
try:
40+
from sphinx_needs.api import add_field as _add_field
41+
except ImportError: # sphinx-needs < 8 has no add_field
42+
_add_field = None
43+
44+
# add_extra_option only accepts a schema on sphinx-needs >= 6 (where schema
45+
# validation exists); older versions take just (app, name). Probe the signature
46+
# instead of comparing versions, so no ``packaging`` dependency is needed.
47+
_USE_FIELD_SCHEMA = "schema" in signature(add_extra_option).parameters
48+
49+
50+
def _register_sn_field(app: Sphinx, name: str, description: str) -> None:
51+
"""Register a string field, preferring the modern ``add_field`` API.
52+
53+
``add_field`` (sphinx-needs >= 8) registers a typed field; older versions
54+
fall back to ``add_extra_option`` (only deprecated on >= 8). A typed field
55+
defaults to ``None`` and is stripped before schema validation, whereas an
56+
untyped field defaults to ``""`` and would trip a strict
57+
``unevaluatedProperties: false`` schema on needs that never set it.
58+
"""
59+
schema = {"type": "string"}
60+
if _add_field is not None:
61+
_add_field(name, description, schema=schema)
62+
elif _USE_FIELD_SCHEMA:
63+
add_extra_option(app, name, schema=schema)
64+
else:
65+
add_extra_option(app, name)
66+
3867

3968
def _check_sphinx_needs_dependency(app: Sphinx) -> bool:
4069
"""Check if sphinx-needs is actually loaded as an extension."""
@@ -185,13 +214,17 @@ def set_config_to_sphinx(
185214

186215
def update_sn_extra_options(app: Sphinx, config: _SphinxConfig) -> None:
187216
src_trace_sphinx_config = CodeLinksConfig.from_sphinx(config)
188-
add_extra_option(app, "project")
189-
add_extra_option(app, "file")
190-
add_extra_option(app, "directory")
217+
_register_sn_field(app, "project", "Source-tracing project")
218+
_register_sn_field(app, "file", "Source file")
219+
_register_sn_field(app, "directory", "Source directory")
191220
if src_trace_sphinx_config.set_local_url:
192-
add_extra_option(app, src_trace_sphinx_config.local_url_field)
221+
_register_sn_field(
222+
app, src_trace_sphinx_config.local_url_field, "Local source URL"
223+
)
193224
if src_trace_sphinx_config.set_remote_url:
194-
add_extra_option(app, src_trace_sphinx_config.remote_url_field)
225+
_register_sn_field(
226+
app, src_trace_sphinx_config.remote_url_field, "Remote source URL"
227+
)
195228

196229

197230
def update_sn_types(app: Sphinx, _config: _SphinxConfig) -> None:
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
project = "schema-strictness-demo"
2+
copyright = "2026, useblocks"
3+
author = "useblocks"
4+
5+
extensions = ["sphinx_needs", "sphinx_codelinks"]
6+
7+
exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"]
8+
9+
# A strict sphinx-needs schema that forbids any field beyond the core
10+
# id/title/type. sphinx-codelinks registers fields (project/file/directory and
11+
# optionally the url fields) globally, attaching them to every need; those must
12+
# not be reported as "additional" fields when left unpopulated.
13+
needs_schema_definitions_from_json = "schemas.json"
14+
15+
html_theme = "alabaster"
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Strict schema with unset sphinx-codelinks fields
2+
================================================
3+
4+
This plain requirement does not use source tracing. It leaves the fields that
5+
sphinx-codelinks registers globally (``project``, ``file``, ``directory`` ...)
6+
unpopulated. A schema that forbids additional fields
7+
(``unevaluatedProperties: false``) must therefore not report them as
8+
unexpected.
9+
10+
.. req:: A plain requirement
11+
:id: REQ_1
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"schemas": [
3+
{
4+
"id": "no-additional-fields",
5+
"severity": "violation",
6+
"validate": {
7+
"local": {
8+
"unevaluatedProperties": false
9+
}
10+
}
11+
}
12+
]
13+
}

tests/test_schema_validation.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""Regression test for sphinx-needs schema validation of sphinx-codelinks fields.
2+
3+
sphinx-codelinks registers extra fields (``project``, ``file``, ``directory``
4+
and, when enabled, the local/remote url fields) with sphinx-needs. These fields
5+
are attached to *every* need in the project.
6+
7+
When a user defines a strict sphinx-needs schema that forbids any field beyond
8+
the core ``id`` / ``title`` / ``type`` (``unevaluatedProperties: false``), those
9+
registered fields must not be treated as "additional" fields for needs that
10+
never populate them. The fields therefore have to be registered with a typed
11+
schema, so unset values default to ``None`` and are stripped before schema
12+
validation (an untyped field defaults to ``""``, which is not stripped and trips
13+
``unevaluatedProperties: false``).
14+
"""
15+
16+
from collections.abc import Callable
17+
from inspect import signature
18+
import json
19+
from pathlib import Path
20+
import shutil
21+
22+
import pytest
23+
from sphinx.testing.util import SphinxTestApp
24+
from sphinx_needs.api import add_extra_option # type: ignore[import-untyped]
25+
26+
# Schema validation (needs_schema_definitions) arrived in sphinx-needs 6.0.0,
27+
# the same release where add_extra_option gained its ``schema`` keyword. Probe
28+
# that keyword to gate the test, instead of pulling in ``packaging`` for a
29+
# version comparison.
30+
SN_SUPPORTS_SCHEMAS = "schema" in signature(add_extra_option).parameters
31+
32+
33+
@pytest.mark.skipif(
34+
not SN_SUPPORTS_SCHEMAS,
35+
reason="needs_schema_definitions requires sphinx-needs>=6.0.0",
36+
)
37+
def test_strict_schema_ignores_unset_codelinks_fields(
38+
tmp_path: Path,
39+
make_app: Callable[..., SphinxTestApp],
40+
) -> None:
41+
this_file_dir = Path(__file__).parent
42+
sphinx_project = Path("doc_test") / "schema_strictness"
43+
sphinx_src_dir = tmp_path / sphinx_project
44+
shutil.copytree(
45+
this_file_dir / sphinx_project,
46+
sphinx_src_dir,
47+
dirs_exist_ok=True,
48+
)
49+
50+
app: SphinxTestApp = make_app(srcdir=sphinx_src_dir, freshenv=True)
51+
app.build()
52+
53+
report = json.loads(
54+
(Path(app.outdir) / "schema_violations.json").read_text(encoding="utf-8")
55+
)
56+
57+
# Schema validation actually ran over our need ...
58+
assert report["validated_needs_count"] >= 1
59+
60+
# ... and produced no violations. If sphinx-codelinks registered its fields
61+
# untyped (default ""), REQ_1 would carry empty project/file/directory values
62+
# and the strict schema would (wrongly) report
63+
# "unevaluated properties are not allowed".
64+
assert report["validation_warnings"] == {}, (
65+
"Strict schema flagged unset sphinx-codelinks fields: "
66+
f"{json.dumps(report['validation_warnings'], indent=2)}"
67+
)

0 commit comments

Comments
 (0)