Skip to content

Commit 6506ecf

Browse files
authored
👌 Include JSON location in $ref resolution errors (#1736)
## Summary `resolve_refs` errors (circular / invalid / missing `$ref`, `$ref`-with-siblings) gave **no** indication of *where* the faulty reference lived, so on a non-trivial `needs_schema_definitions` users had to hunt through the whole config to find it. Other config errors already carry a schema name via `get_schema_name` (`id[idx]`); the global `$ref`-resolution pass was the gap. This threads the JSON path of the current item through the recursive `resolve_refs` walk and appends it to each error message. ## Before → after ```text Circular reference detected for 'type-impl'. → Circular reference detected for 'type-impl' (at $defs.type-impl.allOf[0]). Reference 'not-exist' not found in definitions. → Reference 'not-exist' not found in definitions (at schemas[0].validate.local). Invalid $ref value: 123, expected a string. → Invalid $ref value: 123, expected a string (at schemas[0].validate.local). Invalid $ref value: type-impl, expected to start with '#/$defs/'. → Invalid $ref value: type-impl, expected to start with '#/$defs/' (at schemas[0].validate.local). ``` The path is the JSON location from the root of `needs_schema_definitions` (`schemas[i]...` or `$defs.<name>...`), inherently encoding which schema (by index) the `$ref` sits in. When there is no path (root), the suffix is omitted, so behaviour is unchanged for that degenerate case. ## Notes - Snapshot-only test impact: the five `ref_*` config fixtures in `tests/schema/__snapshots__/test_schema.ambr` now include the location.
1 parent 59b8b4e commit 6506ecf

2 files changed

Lines changed: 34 additions & 17 deletions

File tree

‎sphinx_needs/schema/config_utils.py‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -241,51 +241,68 @@ def resolve_refs(
241241
curr_item: Any,
242242
circular_refs_guard: set[str],
243243
is_defs: bool = False,
244+
path: str = "",
244245
) -> None:
245-
"""Recursively resolve and replace $ref entries in schemas."""
246+
"""Recursively resolve and replace $ref entries in schemas.
247+
248+
``path`` accumulates the JSON location of ``curr_item`` from the root of
249+
``needs_schema_definitions`` (e.g. ``schemas[0].validate.network.links.contains.local``
250+
or ``$defs.safe-impl.allOf[1]``) and is appended to ``$ref`` error messages so a
251+
faulty reference can be located without hunting through the whole config.
252+
"""
253+
loc = f" (at {path})" if path else ""
246254
if isinstance(curr_item, dict):
247255
if "$ref" in curr_item:
248256
if len(curr_item) == 1:
249257
ref_raw = curr_item["$ref"]
250258
if not isinstance(ref_raw, str):
251259
raise NeedsConfigException(
252-
f"Invalid $ref value: {ref_raw}, expected a string."
260+
f"Invalid $ref value: {ref_raw}, expected a string{loc}."
253261
)
254262
if not ref_raw.startswith("#/$defs/"):
255263
raise NeedsConfigException(
256-
f"Invalid $ref value: {ref_raw}, expected to start with '#/$defs/'."
264+
f"Invalid $ref value: {ref_raw}, expected to start with '#/$defs/'{loc}."
257265
)
258266
ref = ref_raw[8:] # Remove '#/$defs/' prefix
259267
if ref not in defs:
260268
raise NeedsConfigException(
261-
f"Reference '{ref}' not found in definitions."
269+
f"Reference '{ref}' not found in definitions{loc}."
262270
)
263271
if ref in circular_refs_guard:
264272
raise NeedsConfigException(
265-
f"Circular reference detected for '{ref}'."
273+
f"Circular reference detected for '{ref}'{loc}."
266274
)
267275
circular_refs_guard.add(ref)
268276
curr_item.clear()
269277
curr_item.update(defs[ref])
270-
resolve_refs(defs, curr_item, circular_refs_guard)
278+
resolve_refs(defs, curr_item, circular_refs_guard, path=path)
271279
circular_refs_guard.remove(ref)
272280
else:
273281
raise NeedsConfigException(
274-
f"Invalid $ref entry, expected a single $ref key: {curr_item}"
282+
f"Invalid $ref entry, expected a single $ref key: {curr_item}{loc}"
275283
)
276284
for key, value in curr_item.items():
285+
child_path = f"{path}.{key}" if path else str(key)
277286
if isinstance(value, dict):
278287
if is_defs:
279288
circular_refs_guard.add(key)
280-
resolve_refs(defs, value, circular_refs_guard, is_defs=(key == "$defs"))
289+
resolve_refs(
290+
defs,
291+
value,
292+
circular_refs_guard,
293+
is_defs=(key == "$defs"),
294+
path=child_path,
295+
)
281296
if is_defs:
282297
circular_refs_guard.remove(key)
283298
if isinstance(value, list):
284-
for item in value:
285-
resolve_refs(defs, item, circular_refs_guard)
299+
for idx, item in enumerate(value):
300+
resolve_refs(
301+
defs, item, circular_refs_guard, path=f"{child_path}[{idx}]"
302+
)
286303
elif isinstance(curr_item, list):
287-
for item in curr_item:
288-
resolve_refs(defs, item, circular_refs_guard)
304+
for idx, item in enumerate(curr_item):
305+
resolve_refs(defs, item, circular_refs_guard, path=f"{path}[{idx}]")
289306

290307

291308
def populate_field_type(

‎tests/schema/__snapshots__/test_schema.ambr‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -579,19 +579,19 @@
579579
"Schema '[0]' defines an unknown network link type 'links2'."
580580
# ---
581581
# name: test_schema_config[ref_error]
582-
"Reference 'not-exist' not found in definitions."
582+
"Reference 'not-exist' not found in definitions (at schemas[0].validate.local)."
583583
# ---
584584
# name: test_schema_config[ref_mixed_error]
585-
"Invalid $ref entry, expected a single $ref key: {'$ref': '#/$defs/not-exist', 'required': ['type']}"
585+
"Invalid $ref entry, expected a single $ref key: {'$ref': '#/$defs/not-exist', 'required': ['type']} (at schemas[0].validate.local)"
586586
# ---
587587
# name: test_schema_config[ref_not_defs_prefix]
588-
"Invalid $ref value: type-impl, expected to start with '#/$defs/'."
588+
"Invalid $ref value: type-impl, expected to start with '#/$defs/' (at schemas[0].validate.local)."
589589
# ---
590590
# name: test_schema_config[ref_recursive_error]
591-
"Circular reference detected for 'type-impl'."
591+
"Circular reference detected for 'type-impl' (at $defs.type-impl.allOf[0])."
592592
# ---
593593
# name: test_schema_config[ref_value_not_string]
594-
'Invalid $ref value: 123, expected a string.'
594+
'Invalid $ref value: 123, expected a string (at schemas[0].validate.local).'
595595
# ---
596596
# name: test_schema_config[schemas_select_pattern_unsafe_error]
597597
'''

0 commit comments

Comments
 (0)