diff --git a/CHANGELOG.md b/CHANGELOG.md index 55764fa..df333f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A repeating model whose optional trailing particle and leading particle share a name, `(c, b?, c?)*`, is now correctly rejected as a Unique Particle Attribution violation, as libxml2/Xerces reject it: after `b` the next `c` could be the trailing `c?` or the looped leading `c`, two distinct particles. The compositional determinism check formed the internal sequence gap before the enclosing repetition's loop edge was added, so the combined `{trailing, leading}` followpos was never checked (an invalid-schemas-accepted false negative the finite XSTS corpus did not contain). The sequence now carries each trailing end-item's internal suffix in its `lastFollow`, so a parent gap or the loop edge forms the end-item's full followpos. The carried suffix is always a subset of the true followpos, so the check only catches more genuine conflicts and never over-rejects: the non-repeated `(c, b?, c?)` and disjoint shapes like `(a, b?)*` stay valid. Found by the new generated-schema determinism differential against libxml2; verified over the full W3C XSTS corpus in release (`valid-schemas-rejected`/`valid-instances-rejected` stay 0, `invalid-*-accepted` unchanged) with an adversarial over-rejection critic finding no valid schema newly rejected over 47 shapes. The related ambiguity through two references to one group in a repeating sequence stays an under-rejection (the cached group identity cannot distinguish a looped from a trailing occurrence without per-occurrence tracking), named in `DeterminismDifferentialTests`. + - Two references to the same `` that overlap (`(g?, g?)`, `(g | g)`, `(g*, g)`) are now correctly rejected as a Unique Particle Attribution violation, as the inline forms `(x?, x?)` / `(x | x)` already were and as libxml2/Xerces reject them. The compositional determinism check summarises each group once and reused the group's node identity across references, so the distinct-identity overlap test could not see that two references are two expanded occurrences, and PureXML accepted these models (an invalid-schemas-accepted false negative). It also let the per-instance content matcher run on such a non-UPA model, where a multiply-referenced nullable group is exponential: a depth-18 optional doubling chain was accepted and then hung instance validation, a keystroke-time interactive-safety hang on a schema PureXML itself accepted. The fix dedupes each decision set (first set, sequence gap, `all` cross-follow) by particle identity and treats a same-identity collision as the conflict, while the repetition self-loop (`a*`, `(a*)*`) keeps its non-flagging dedupe so a single particle looping is not misread as two occurrences. This also bounds the check: the deduped first/follow sets stay `O(labels)` instead of `2^K`, so the same depth-40 chain is now rejected at compile in 11 ms and no instance is ever matched against it. A redefine self-extension (`` inside the redefinition of `T`) lists a type's own content twice through `effectiveModelNodes`; that assembly artifact is deduped before the walk so it is not read as a conflict (addB094, schZ007). Verified over the full W3C XSTS corpus in release: `valid-schemas-rejected` and `valid-instances-rejected` stay 0 and `invalid-*-accepted` are unchanged (the corpus does not exercise this shape in its pinned buckets), with an adversarial over-rejection critic finding no valid schema newly rejected across the group-overlap shape space. - XSTS invalid-schemas-accepted (#145): a particle restriction whose restricting element has a union-typed base element now applies Type Derivation OK (Simple) (`cos-st-derived-ok`) rather than structural member subsetting. The NameAndTypeOK type clause (ยง3.9.6) requires the restricting element's type to be validly derived from the base element's type; PureXML accepted a *separately declared* union whenever its member types were a subset of the base union's members, which is not the spec's derivation relation. The restricting type is now valid over a union base only if it is the base type, derives from it through a declared restriction chain, or (clause 2.2.4) validly derives from one of the union's member types. Catches particlesIk011 (a fresh `union(integer)` is not a restriction of `union(integer, string)`): invalid-schemas-accepted 9 to 8, with valid-schemas-rejected held at 0 (verified over the full XSTS; the change is scoped to the element-restriction path, so no attribute or simple-type derivation check moved). Disclosed divergence: particlesIk025/Ik027, where the restricting type derives from a union *member*, are accepted per `cos-st-derived-ok` clause 2.2.4, a contested reading their XSTS `invalid` expectation does not share. diff --git a/Sources/Schema/ContentModelDeterminismCompositional.swift b/Sources/Schema/ContentModelDeterminismCompositional.swift index 39fd2aa..b8f8cff 100644 --- a/Sources/Schema/ContentModelDeterminismCompositional.swift +++ b/Sources/Schema/ContentModelDeterminismCompositional.swift @@ -215,15 +215,20 @@ private struct CompWalk { /// sequence). The sequence's own `lastFollow` is its trailing nullable suffix's /// end-items, carried up for the parent to combine with whatever follows. mutating func sequence(_ parts: [CompSummary]) -> CompSummary { + // `suffixFirst[index]` is the first of the nullable run starting after `index`, + // i.e. what can follow an end-item of `parts[index]` WITHIN this sequence. + var suffixFirst: [[CompItem]] = Array(repeating: [], count: parts.count) for index in parts.indices { - var suffixFirst: [CompItem] = [] + var suffix: [CompItem] = [] for next in parts[(index + 1)...] { - suffixFirst.append(contentsOf: next.first) + suffix.append(contentsOf: next.first) if !next.nullable { break } } - guard !suffixFirst.isEmpty else { continue } + suffixFirst[index] = suffix + } + for index in parts.indices where !suffixFirst[index].isEmpty { for entry in parts[index].lastFollow { - emit(merge(entry.follow, suffixFirst)) + emit(merge(entry.follow, suffixFirst[index])) } } var prefixFirsts: [[CompItem]] = [] @@ -231,10 +236,19 @@ private struct CompWalk { prefixFirsts.append(part.first) if !part.nullable { break } } + // Each trailing end-item carries its internal follow PLUS the suffix that follows + // it inside the sequence. Otherwise an end-item that is also followed internally + // (an optional element after it) loses that continuation in `lastFollow`, so a + // parent gap, or an enclosing repetition's loop edge, forms only a partial + // followpos: a repeating `(c, b?, c?)` would not see that after `b` both the + // trailing `c` and the looped leading `c` are reachable. The gap loop already + // flagged the internal followpos, so dedupe without re-flagging here. var trailing: [(item: CompItem, follow: [CompItem])] = [] - for part in parts.reversed() { - trailing.append(contentsOf: part.lastFollow) - if !part.nullable { break } + for index in parts.indices.reversed() { + for entry in parts[index].lastFollow { + trailing.append((entry.item, dedupOnly([entry.follow, suffixFirst[index]]))) + } + if !parts[index].nullable { break } } return CompSummary(nullable: parts.allSatisfy(\.nullable), first: mergeList(prefixFirsts), lastFollow: mergeLastFollow(trailing)) } diff --git a/Tests/DeterminismDifferentialTests.swift b/Tests/DeterminismDifferentialTests.swift index b64909d..2d6c1f9 100644 --- a/Tests/DeterminismDifferentialTests.swift +++ b/Tests/DeterminismDifferentialTests.swift @@ -19,12 +19,16 @@ import Testing /// this generator, the same shape space that earlier hid the multiply-referenced-group /// overlap (now fixed). The false negatives it surfaces are NAMED here rather than left /// silent, and triaged genuine vs contested: -/// - GENUINE, tracked under-rejection debt (its own FP-critical fix): a repeating model -/// whose optional trailing particle and leading particle share a name, `(c, b?, c?)*` -/// (and the same through two references to one group). They are distinct particles that -/// both match at the loop point, but the compositional check computes the internal -/// sequence gap before the enclosing repetition's loop edge is added, so the combined -/// `{trailing, leading}` decision set is never formed. +/// - FIXED: a repeating model whose optional trailing particle and leading particle share +/// a name, `(c, b?, c?)*`. Distinct particles that both match at the loop point; the +/// sequence now carries each end-item's internal suffix in `lastFollow` so the loop edge +/// forms the combined `{trailing, leading}` followpos (`DeterminismSequenceFollowposTests`). +/// - GENUINE but unfixed (a fundamental compositional limitation): the SAME shape through +/// two references to ONE group in a repeating sequence (`(g+, c, g?)+`). The trailing +/// and looped occurrences share the cached group identity, so the loop fold's +/// non-flagging dedupe cannot tell them from a single particle looping (`(a*)*`) without +/// over-rejecting; distinguishing them needs per-occurrence tracking the summarize-once +/// design omits. /// - CONTESTED, libxml2 the stricter party: an UNUSED group definition with an ambiguous /// model (`all(b, b)`) (XSD's UPA constrains the effective content type of complex /// types, and an unreferenced group is never one), and `(a?)+`, a nullable body diff --git a/Tests/DeterminismSequenceFollowposTests.swift b/Tests/DeterminismSequenceFollowposTests.swift new file mode 100644 index 0000000..f46e084 --- /dev/null +++ b/Tests/DeterminismSequenceFollowposTests.swift @@ -0,0 +1,55 @@ +import Testing +@testable import PureXML + +/// A repeating model whose optional trailing particle and leading particle share a name, +/// `(c, b?, c?)*`, is ambiguous: after `b` the next `c` could be the trailing `c?` or the +/// looped leading `c`, two distinct particles. The compositional determinism check +/// emitted the internal sequence gap before the enclosing repetition's loop edge, so the +/// combined `{trailing, leading}` followpos was never formed. The fix carries each +/// trailing end-item's internal suffix in the sequence's `lastFollow`, so the loop (and a +/// parent gap) form the full followpos. Found by the generated-schema determinism +/// differential against libxml2 (`DeterminismDifferentialTests`). +@Suite("UPA on a repeating sequence's trailing/leading overlap") +struct DeterminismSequenceFollowposTests { + private let opt = " minOccurs=\"0\"" + private let star = " maxOccurs=\"unbounded\"" + private let optStar = " minOccurs=\"0\" maxOccurs=\"unbounded\"" + + private func element(_ name: String, _ occurrence: String = "") -> String { + "" + } + + private func sequence(_ occurrence: String, _ members: String...) -> String { + "\(members.joined())" + } + + private func accepts(_ content: String) -> Bool { + let schema = "" + + "\(content)" + return (try? PureXML.Schema.Document(schema)) != nil + } + + @Test("a repeating sequence whose trailing optional and leading particle overlap is rejected") + func test_repeatingTrailingLeadingOverlapRejected() { + // (c, b?, c?)*: after b, the trailing c? and the looped leading c both match c. + #expect(!accepts(sequence(optStar, element("c"), element("b", opt), element("c", opt)))) + // (a, b?, a?)*: the same shape, leading and trailing a. + #expect(!accepts(sequence(star, element("a"), element("b", opt), element("a", opt)))) + // (a?, b, a?)*: leading optional a and trailing optional a, around a required b. + #expect(!accepts(sequence(star, element("a", opt), element("b"), element("a", opt)))) + } + + @Test("the same sequences NOT repeated, or without the overlap, stay valid") + func test_noOverRejection() { + // (c, b?, c?) NOT repeated: the trailing c is the only c after b, unambiguous. + #expect(accepts(sequence("", element("c"), element("b", opt), element("c", opt)))) + // (a, b?)*: distinct names, the loop's a does not overlap the trailing b. + #expect(accepts(sequence(optStar, element("a"), element("b", opt)))) + // (a, b, c)*: all distinct, the loop's a follows the required c unambiguously. + #expect(accepts(sequence(star, element("a"), element("b"), element("c")))) + // a*: a single particle looping is not two occurrences (the loop-fold must not flag). + #expect(accepts(sequence("", element("a", optStar)))) + // (c, b?, d?)*: the trailing d does not share the leading c's name. + #expect(accepts(sequence(star, element("c"), element("b", opt), element("d", opt)))) + } +}