Skip to content

[bugfix] fn:json-to-xml: enforce option-parameter type & permitted-value checks (+~10 XQTS HEAD)#6351

Open
joewiz wants to merge 3 commits into
eXist-db:developfrom
joewiz:bugfix/fn-json-to-xml-options-xpty
Open

[bugfix] fn:json-to-xml: enforce option-parameter type & permitted-value checks (+~10 XQTS HEAD)#6351
joewiz wants to merge 3 commits into
eXist-db:developfrom
joewiz:bugfix/fn-json-to-xml-options-xpty

Conversation

@joewiz
Copy link
Copy Markdown
Member

@joewiz joewiz commented May 11, 2026

Summary

Enforce F&O 3.1 §2.4 (option-parameter conventions) and §17.5.3 on the options
map of fn:json-to-xml (and, where shared, fn:parse-json / fn:json-doc):

  • Wrong-typed option values now raise XPTY0004 (was: FOJS0005 for
    escape, silently accepted for validate, liberal with non-boolean, etc.).
  • duplicates is checked against the permitted set {reject, use-first, retain}; non-permitted values raise FOJS0005.
  • validate: true() combined with duplicates: 'retain' raises FOJS0005.
  • Unknown option keys on fn:json-to-xml raise FOJS0005.
  • Empty-sequence option values raise XPTY0004 (cardinality 0).
  • fallback's declared type function(xs:string) as xs:string is validated
    for arity (=1).

Behavior under recognized, valid options is unchanged — implementing the
validate, escape: true(), and fallback semantics themselves is out of
scope (tracked separately).

What Changed

  • exist-core/src/main/java/org/exist/xquery/functions/fn/JSON.java

    • New parseOptions(...) helper and requireBooleanOpt /
      requireStringOpt / requireFunctionOpt helpers implement strict
      type-checking per F&O §2.4 (no XPath promotion; xs:untypedAtomic is
      cast; everything else is rejected with XPTY0004).
    • New rejectUnknownJsonToXmlOptions enforces the closed option set for
      fn:json-to-xml.
    • Pulls the eval-method options block out of line, dropping its
      NPath complexity below the PMD threshold.
  • exist-core/src/test/xquery/xquery3/json-to-xml.xql

    • Updates json-to-xml-error-2 (escape non-boolean) from FOJS0005 to
      XPTY0004.
    • Adds 9 new error assertions covering liberal/validate/escape/fallback
      type mismatches, duplicates permitted values, validate+retain incompat,
      and unknown option keys.

Spec References

XQTS HEAD targets

Test Before After
json-to-xml-error-018 (escape:'unrecognised') FOJS0001 XPTY0004
json-to-xml-error-019 (liberal:'something') FORG0001 XPTY0004
json-to-xml-error-020 (validate:()) success XPTY0004
json-to-xml-error-021 (validate:(true,true)) success XPTY0004
json-to-xml-error-022 (validate:'EMCA-262') success XPTY0004
json-to-xml-error-023 (escape:()) success XPTY0004
json-to-xml-error-024 (escape:(true,true)) success XPTY0004
json-to-xml-error-025 (escape:'EMCA-262') FOJS0005 XPTY0004
json-to-xml-error-026 (fallback:'dummy') success XPTY0004
json-to-xml-error-040 (extra option keys) success FOJS0005
json-to-xml-error-041 (fallback:concat#2) success XPTY0004

XQTS spot-check rerun pending; will update this table once measured.

Sibling fix: #6350 (fn:xml-to-json validation).

Test Plan

  • mvn test -pl exist-core -Dtest=xquery.xquery3.XQuery3Tests
    1020 / 1020 passing (1 skipped, pre-existing).
  • PMD via codacy-cli on JSON.java — no new findings; pre-existing
    parseResource / readValue NPath warnings unchanged.
  • XQTS HEAD fn-json-to-xml spot-check (rebuild runner per
    measurement SOP, expect Cat B closed, Cat F partial).

…value checks

Per F&O 3.1 section 2.4 (option parameter conventions) plus section 17.5.3:

* Wrong-typed option values now raise XPTY0004 (was: FOJS0005 for 'escape',
  silently accepted for others).
* 'duplicates' values are checked against the permitted set {reject,
  use-first, retain}; non-permitted values raise FOJS0005.
* 'validate' + 'duplicates: retain' is rejected as an incompatible
  combination (FOJS0005).
* Unknown option keys on fn:json-to-xml raise FOJS0005.
* Empty-sequence option values trigger XPTY0004 (cardinality 0).
* 'fallback' arity is validated against the declared function signature.

Behavior under recognized, valid options is unchanged. Implementing the
'validate', 'escape', and 'fallback' semantics themselves is out of scope
and tracked in the parent triage report.

Targets ~11 fn-json-to-xml XQTS HEAD failures
(json-to-xml-error-018/019/020/021/022/023/024/025/026/040/041).

Refs F&O 3.1: https://www.w3.org/TR/xpath-functions-31/#options

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joewiz joewiz requested a review from a team as a code owner May 11, 2026 20:34
@joewiz
Copy link
Copy Markdown
Member Author

joewiz commented May 11, 2026

XQTS HEAD spot-check (fn-json-to-xml)

Ran --xqts-version HEAD --test-set-pattern 'fn-json-to-xml' against this branch (runner JAR rebuilt against the patched exist-core).

Metric Baseline (2026-05-11 fresh-runner) This PR Delta
Failures 30 fail + 9 err = 39 22 fail + 6 err = 28 −11
Passes 41 (of 80 attempted) 52 (of 80 attempted) +11
Total tests 94 94
Skipped 14 14

Matches the prediction in the parent triage report: Cat B (8) + Cat F (3) closed, no regressions in this test set.

All 11 targeted error tests now pass:

json-to-xml-error-018  ✓
json-to-xml-error-019  ✓
json-to-xml-error-020  ✓
json-to-xml-error-021  ✓
json-to-xml-error-022  ✓
json-to-xml-error-023  ✓
json-to-xml-error-024  ✓
json-to-xml-error-025  ✓
json-to-xml-error-026  ✓
json-to-xml-error-040  ✓
json-to-xml-error-041  ✓

Spot-checked fn-parse-json and fn-json-doc separately — out of scope for this commit message but no regressions expected since the option-parsing code path is shared and behavior under recognized, valid options is unchanged. (Will post follow-up if those measurements differ.)

Per F&O 3.1 section 2.4, option values undergo function-call-style coercion:
nodes are atomized to xs:untypedAtomic before the type check, which is
then cast to the declared type. Calling .atomize() on the item picks up
both element() and attribute() values that the previous strict-type
check was wrongly rejecting with XPTY0004.

Closes fn-parse-json-946 (XQTS HEAD) which uses
map { 'duplicates': <foo>reject</foo> }.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joewiz
Copy link
Copy Markdown
Member Author

joewiz commented May 11, 2026

Follow-up: confirmed no regressions on sibling test sets and improved one additional case.

After atomize fix (commit 873f28d), running --xqts-version HEAD --test-set-pattern 'fn-(json-to-xml|parse-json|json-doc)':

Test set Tests Pass Fail Err Skipped
fn-json-to-xml 94 52 22 6 14
fn-parse-json 153 122 31 0 0
fn-json-doc 67 54 13 0 0

Vs. the baseline triage numbers, this PR closes −11 fn-json-to-xml plus −1 fn-parse-json (fn-parse-json-946, which constructs map { 'duplicates': <foo>reject</foo> } and expects the element to atomize to a string). No regressions in fn-json-doc.

The atomize fix is needed because F&O 3.1 §2.4 specifies option values undergo function-call-style coercion — node values atomize to xs:untypedAtomic and only then get checked against the declared type. The first commit was too strict.

Commit e3a2da8 introduced a single PERMITTED_DUPLICATES set
{reject, use-first, retain} that was applied to both fn:parse-json and
fn:json-to-xml. That regressed fn:parse-json calls using "use-last"
(the XQSuite test arr:parse-json-duplicates use-last case) with
FOJS0005 "value not permitted".

Per F&O 3.1 the permitted values differ:

* §17.4.1 fn:parse-json: "reject", "use-first", "use-last"
  https://www.w3.org/TR/xpath-functions-31/#func-parse-json
* §17.5.1 fn:json-to-xml: "reject", "use-first", "retain"
  https://www.w3.org/TR/xpath-functions-31/#func-json-to-xml

Both share "reject" and "use-first"; parse-json adds "use-last"
(preserve last value), json-to-xml adds "retain" (keep duplicates in
the XML output, incompatible with validate=true).

Splits PERMITTED_DUPLICATES into per-function sets selected by the
existing isJsonToXml flag. fn:json-doc shares fn:parse-json's option
semantics (§17.4.4) and so picks up the parse-json permitted set, which
is correct.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joewiz
Copy link
Copy Markdown
Member Author

joewiz commented May 12, 2026

[This response was co-authored with Claude Code. -Joe]

Pushed 7c4245e to address the xqts.org.exist-db.test.arrays.parse-json-duplicates failure CI surfaced.

Root cause: commit e3a2da8 (the original options-XPTY commit, not the atomize one) introduced a single shared PERMITTED_DUPLICATES = {reject, use-first, retain} and applied it to both fn:parse-json and fn:json-to-xml. Per F&O 3.1 the two functions have different permitted duplicates value sets:

  • §17.4.1 fn:parse-json permits reject, use-first, use-last
  • §17.5.1 fn:json-to-xml permits reject, use-first, retain

The XQSuite test arr:parse-json-duplicates exercises all three of parse-json's values, so the "use-last" case regressed to FOJS0005.

Fix: split into per-function permitted sets, selected by the existing isJsonToXml flag. fn:json-doc shares parse-json's option semantics (§17.4.4) and picks up the parse-json set, which is correct.

Verification: local ArrayTests (164/164) and XQuery3Tests (1020/1020) both green. The fix is scoped to the permitted-values check and does not touch the type check, the atomize step, or the unknown-key/validate-retain checks — so the +11 fn-json-to-xml HEAD lift and the fn-parse-json-946 win are unaffected.

@duncdrum duncdrum requested a review from a team May 12, 2026 09:21
@line-o line-o added the xquery issue is related to xquery implementation label May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

xquery issue is related to xquery implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants