diff --git a/CHANGELOG.md b/CHANGELOG.md index d09b009..74d9761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Particle Valid (Restriction) no longer over-rejects valid restrictions (#148). Three corrections, all language-preserving: a particle that can never occur (`maxOccurs="0"`, a "pointless particle") is removed from both the derived and base content before mapping, so it no longer consumes a base particle and starves the ones after it (e.g. `(e1 maxOccurs="0", e2)` restricting `(any)` is now valid); a content-free restriction (an empty ``, or a group whose members all cannot occur) is a valid restriction of an EMPTY base, since both accept only the empty sequence; and a derived group that reduces to empty after that removal is a valid restriction of its base only when the base is itself emptiable (by structure or its own occurrence), closing a vacuous-acceptance gap the removal would otherwise open. XSTS: valid schemas rejected 72 to 45 (27 correct schemas no longer wrongly rejected), with one schema (`addB32`) moving to invalid-accepted because its only invalidity is a malformed imported document PureXML does not yet load (tracked by #161), not the restriction. - A `simpleType` must contain exactly one of `restriction`, `list`, or `union` (#158). Per XSD 1.0 Datatypes its content is `(annotation?, (restriction | list | union))`, but the membership table admitted each name without the cardinality, so an empty `simpleType` or one combining two derivations (two `union`s, a `union` and a `list`, two `restriction`s) was accepted (the stB family). The exactly-one rule is now enforced. XSTS: invalid schemas accepted 537 to 527, no other bucket raised (valid schemas rejected held at 72). - The `final` and `block` derivation-control attributes are now checked against their value space (#158). Each is `#all` or a whitespace list of method tokens, but the admitted tokens depend on the component: an element's `final` is `extension`/`restriction` and its `block` adds `substitution`; a complexType's `final`/`block` are `extension`/`restriction`; a simpleType's `final` is `list`/`union`/`restriction`. The tokens were parsed leniently (unknown tokens ignored, `substitution` admitted everywhere), so `final="substitution"` on an element, a misspelled or miscased token, or `#all` mixed with other tokens was accepted (the elemF/st_final families). The list is now validated against the component's exact set; `finalDefault`/`blockDefault` (the wider schema-level value space) and foreign attributes are left alone. XSTS: invalid schemas accepted 580 to 537, no other bucket raised (valid schemas rejected held at 72). - A `simpleContent` derivation's content is now checked for order and disallowed children (#158). A `restriction` is `(annotation?, (simpleType?, facet*), (attribute | attributeGroup)*, anyAttribute?)` and an `extension` is `(annotation?, (attribute | attributeGroup)*, anyAttribute?)`: simpleContent has no model group, an extension has no facets or `simpleType`, and the attributes precede a single trailing `anyAttribute`. The membership table admitted the names but not the model, so a model group inside simpleContent, a facet in an extension, a facet before its `simpleType`, an attribute after `anyAttribute`, or a second `anyAttribute` (the ctD family) was accepted. The derivation's children are matched against the context's ordered slot model, reusing the slot walk built for complexContent. XSTS: invalid schemas accepted 594 to 580, no other bucket raised (valid schemas rejected held at 72). diff --git a/Sources/Schema/ParticleRestriction.swift b/Sources/Schema/ParticleRestriction.swift index a657cc7..3db3779 100644 --- a/Sources/Schema/ParticleRestriction.swift +++ b/Sources/Schema/ParticleRestriction.swift @@ -20,7 +20,13 @@ extension PureXML.Schema { emptiable(baseParticle) ? nil : "the base content is required, so the restriction cannot be EMPTY" case (.mixed, .elementOnly): "a restriction cannot add mixed content to an element-only base" - case (.elementOnly, .empty), (.mixed, .empty): + case let (.elementOnly(restrictedParticle), .empty): + // A content-free particle (an empty group, or one whose members all + // have maxOccurs=0) accepts only the empty sequence, so it is a valid + // restriction of EMPTY; only a particle that can contribute a child is + // adding content. + contentFree(restrictedParticle) ? nil : "the base content is EMPTY, so the restriction cannot add content" + case (.mixed, .empty): "the base content is EMPTY, so the restriction cannot add content" case let (.elementOnly(restrictedParticle), .elementOnly(baseParticle)), let (.elementOnly(restrictedParticle), .mixed(baseParticle)), @@ -46,6 +52,14 @@ extension PureXML.Schema { // against the base group. return groupValid(Group(compositor: .sequence, particles: [restricted.withUnitRange()]), baseGroup, types, derivation) case let (.group(restrictedGroup), .group(baseGroup)): + // If every member of the derived group is a pointless (maxOccurs=0) + // particle, the group accepts only the empty sequence; that is a valid + // restriction iff the base particle is itself emptiable (by structure + // or its own occurrence). `emptiable(base)` accounts for both, which a + // structural check inside `groupValid` could not. + if restrictedGroup.particles.allSatisfy({ $0.maxOccurs == 0 }) { + return emptiable(base) + } return groupValid(restrictedGroup, baseGroup, types, derivation) case let (.group(restrictedGroup), .wildcard(wildcard)): // NSRecurseCheckCardinality, namespace part: every leaf element @@ -57,21 +71,40 @@ extension PureXML.Schema { } private static func groupValid(_ restricted: Group, _ base: Group, _ types: [String: ElementType], _ derivation: [String: TypeDerivation]) -> Bool { + // A particle that can never occur (maxOccurs=0) contributes nothing to the + // language, so it is removed before mapping (a "pointless particle"). Left + // in, it would wrongly consume a base particle and starve the particles + // after it (e.g. `(e1 maxOccurs=0, e2)` restricting `(any)`). + let restrictedParticles = restricted.particles.filter { $0.maxOccurs != 0 } + let baseParticles = base.particles.filter { $0.maxOccurs != 0 } switch (restricted.compositor, base.compositor) { case (.sequence, .sequence), (.all, .all): - recurse(restricted.particles, base.particles, skippedMustBeEmptiable: true, types, derivation) + return recurse(restrictedParticles, baseParticles, skippedMustBeEmptiable: true, types, derivation) case (.choice, .choice): - recurse(restricted.particles, base.particles, skippedMustBeEmptiable: false, types, derivation) + return recurse(restrictedParticles, baseParticles, skippedMustBeEmptiable: false, types, derivation) case (.sequence, .choice): // MapAndSum (simplified): each restricted particle fits some branch. - restricted.particles.allSatisfy { particle in - base.particles.contains { valid(particle, $0, types, derivation) } + return restrictedParticles.allSatisfy { particle in + baseParticles.contains { valid(particle, $0, types, derivation) } } case (.sequence, .all): // RecurseUnordered: map onto distinct base particles in any order. - recurseUnordered(restricted.particles, base.particles, types, derivation) + return recurseUnordered(restrictedParticles, baseParticles, types, derivation) default: - false + return false + } + } + + /// Whether a particle can never contribute a child element or wildcard: it + /// has `maxOccurs=0`, or is a group all of whose members are content-free + /// (an empty group included). Its only legal content is the empty sequence. + private static func contentFree(_ particle: Particle) -> Bool { + if particle.maxOccurs == 0 { return true } + switch particle.term { + case .element, .wildcard: + return false + case let .group(group): + return group.particles.allSatisfy(contentFree) } } diff --git a/Tests/XSDParticleRestrictionTests.swift b/Tests/XSDParticleRestrictionTests.swift index a927d1d..6454704 100644 --- a/Tests/XSDParticleRestrictionTests.swift +++ b/Tests/XSDParticleRestrictionTests.swift @@ -49,6 +49,34 @@ struct XSDParticleRestrictionTests { base: "", restriction: "", )) + // A maxOccurs=0 particle never occurs (pointless particle); it is removed + // before mapping, so it must not consume the base wildcard the next element + // restricts (particlesJd). + #expect(compiles( + base: "", + restriction: "", + )) + // A content-free restriction (an empty sequence) of an EMPTY base is valid: + // both accept only the empty sequence (addB079). + #expect(compiles( + base: "", + restriction: "", + )) + } + + @Test("An all-maxOccurs=0 derived sequence does not vacuously restrict a required base") + func test_emptyDerivedRequiredBase() { + // After pointless-particle removal the derived sequence is empty (accepts only + // the empty sequence); restricting a required choice {a|b} is invalid. + #expect(restrictionError( + base: "", + restriction: "", + )?.contains("not a valid restriction") == true) + // But restricting an emptiable base choice {a?|b} is valid. + #expect(compiles( + base: "", + restriction: "", + )) } @Test("A restriction widening the occurrence range is rejected") diff --git a/Tests/XSTSSuiteTests.swift b/Tests/XSTSSuiteTests.swift index 55a708a..f2d9783 100644 --- a/Tests/XSTSSuiteTests.swift +++ b/Tests/XSTSSuiteTests.swift @@ -28,8 +28,8 @@ struct XSTSSuiteTests { /// `swift test` are unaffected; run it with `swift test -c release --filter /// XSTS` (debug is far slower and the corpus is large). Per-case deviations /// are written to /tmp/xsts-failures.txt for the burn-down. - private let knownSchemaValidRejected = 72 - private let knownSchemaInvalidAccepted = 527 + private let knownSchemaValidRejected = 45 + private let knownSchemaInvalidAccepted = 528 private let knownInstanceValidRejected = 214 private let knownInstanceInvalidAccepted = 160 diff --git a/docs/schema-validity-burndown.md b/docs/schema-validity-burndown.md index 462a939..4423bd7 100644 --- a/docs/schema-validity-burndown.md +++ b/docs/schema-validity-burndown.md @@ -9,8 +9,8 @@ irreducible tail. Do not deviate to new features while this is open. | Bucket | Count | Reading | |---|---|---| -| valid-schemas-rejected | 72 | we rarely reject a good schema | -| **invalid-schemas-accepted** | **2461 -> 527** | we rarely catch a bad one (facet definition + applicability, default/fixed value validity, particle-restriction name+type, id, structural content-model, UPA determinism, complexContent + complexType content order, identity-constraint XPath-subset, and wildcard-namespace validity landed) | +| valid-schemas-rejected | **72 -> 45** | production target is 0: rejecting a correct schema is the most user-hostile defect (#148). Particle-restriction over-rejection fixed: pointless `maxOccurs=0` particles removed, content-free restriction of an EMPTY base, empty-derived emptiable-base. | +| **invalid-schemas-accepted** | **2461 -> 528** | we rarely catch a bad one (facet, default/fixed value, particle-restriction name+type, id, structural content-model, UPA determinism, content order, identity-constraint XPath-subset, wildcard-namespace, final/block, simpleType content landed). The 527->528 step is `addB32`, invalid only via a malformed import (#161), unmasked by removing a wrong restriction rejection. | | valid-instances-rejected | 233 | (instance side, separately tracked) | | invalid-instances-accepted | 165 | |