diff --git a/CHANGELOG.md b/CHANGELOG.md index e8e9dd2..8af9f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Type-directed validation, the core of the validation-rules idiom: a `Check` carries a + positive description, a `when` predicate, and a Bool or multi-error check, and fires only on + values of its `Subject` type wherever they occur in the tree. `AnyCheck` erases the subject type + and dispatches by runtime type, guarding against an optional or a wrong type matching. The + validator gains a `checks` list and a `validating(_:)` overload for checks, and the traversal + offers each node the values it may specialise on (the value, and for a mapping the mapping and + its keys). `BuiltinCheck.mappingKeysAreUnique` is the type-directed counterpart of the legacy + rule, firing on `Model.Mapping` directly. The machinery has its own negative tests (wrong type, + optional, and false predicate yield nothing; the same type twice yields two errors) and the + builtin ships a failing and a near-miss test. The default validator is unchanged, so this is + additive; the legacy `Rule` is retired in a later step. - The validator is now configurable by rule identity, per the validation-rules idiom: `withoutValidating(_ description:)` removes a rule by its description (the removal identity, so descriptions must be unique and stable), and `validationDescriptions` lists the active rules in diff --git a/Sources/Validation/AnyCheck.swift b/Sources/Validation/AnyCheck.swift new file mode 100644 index 0000000..5d65cef --- /dev/null +++ b/Sources/Validation/AnyCheck.swift @@ -0,0 +1,27 @@ +public extension PureYAML.Validation { + /// A type-erased `Check`, so checks specialised on different subject types live in one list. It + /// filters by runtime type: a `Check` fires only on a `Subject` value, never on an + /// optional of it (the `type(of:)` guard) or a value of any other type. + struct AnyCheck: Sendable { + public let description: String + private let applyErased: @Sendable (any Validatable, Path, PureYAML.Model.Value) -> [Issue] + + public init(_ check: Check) { + description = check.description + applyErased = { subject, path, document in + guard let typed = subject as? Subject, type(of: subject) == Subject.self else { + return [] + } + return check.apply(SubjectContext(document: document, subject: typed, path: path)) + } + } + + public func apply( + to subject: any Validatable, + at path: Path, + in document: PureYAML.Model.Value, + ) -> [Issue] { + applyErased(subject, path, document) + } + } +} diff --git a/Sources/Validation/Check.Builtins.swift b/Sources/Validation/Check.Builtins.swift new file mode 100644 index 0000000..76a7bae --- /dev/null +++ b/Sources/Validation/Check.Builtins.swift @@ -0,0 +1,22 @@ +public extension PureYAML.Validation { + /// The caseless namespace of built-in type-directed checks, referenceable by name. + enum BuiltinCheck { + /// Reports each duplicate key within a single mapping. The type-directed counterpart of the + /// legacy `Rule.mappingKeysAreUnique`; fires on `Model.Mapping` values directly rather than + /// switching on a `Model.Value`. + public static var mappingKeysAreUnique: Check { + Check(description: "Mapping keys are unique") { context in + var seen = Set() + var issues: [Issue] = [] + for pair in context.subject.pairs where !seen.insert(pair.keyNode).inserted { + issues.append(Issue( + severity: .error, + reason: "Duplicate mapping key '\(pair.keyNode.description)'", + path: context.path.appending(pair.keyNode.pathComponent), + )) + } + return issues + } + } + } +} diff --git a/Sources/Validation/Check.swift b/Sources/Validation/Check.swift new file mode 100644 index 0000000..a3aedd7 --- /dev/null +++ b/Sources/Validation/Check.swift @@ -0,0 +1,48 @@ +public extension PureYAML.Validation { + /// A type-directed validation value (the `Validation` of the validation idiom): a + /// positive `description`, a `predicate` that gates application, and a multi-error `check`. It is + /// applied only to values of type `Subject`, anywhere they occur in the traversed tree. + struct Check: Sendable { + public let description: String + public let predicate: @Sendable (SubjectContext) -> Bool + public let check: @Sendable (SubjectContext) -> [Issue] + + /// Multi-error form: return one issue per problem. Use when a subject can fail several ways. + public init( + description: String, + check: @escaping @Sendable (SubjectContext) -> [Issue], + when predicate: @escaping @Sendable (SubjectContext) -> Bool = { _ in true }, + ) { + self.description = description + self.predicate = predicate + self.check = check + } + + /// Single-error Bool form: a positive description plus a check whose `false` auto-produces the + /// one issue `"Failed to satisfy: "`. The dominant form. + public init( + description: String, + severity: Severity = .error, + check: @escaping @Sendable (SubjectContext) -> Bool, + when predicate: @escaping @Sendable (SubjectContext) -> Bool = { _ in true }, + ) { + self.init( + description: description, + check: { context in + check(context) + ? [] + : [Issue(severity: severity, reason: "Failed to satisfy: \(description)", path: context.path)] + }, + when: predicate, + ) + } + + /// Gates on the predicate, then runs the check. + public func apply(_ context: SubjectContext) -> [Issue] { + guard predicate(context) else { + return [] + } + return check(context) + } + } +} diff --git a/Sources/Validation/SubjectContext.swift b/Sources/Validation/SubjectContext.swift new file mode 100644 index 0000000..2a3c0dc --- /dev/null +++ b/Sources/Validation/SubjectContext.swift @@ -0,0 +1,19 @@ +public extension PureYAML.Validation { + /// The read-only bundle a type-directed check receives: the whole `document` (for cross-cutting + /// checks), the current `subject` of the check's type, and the `path` to where it sits. + struct SubjectContext: Sendable { + public var document: PureYAML.Model.Value + public var subject: Subject + public var path: Path + + public init( + document: PureYAML.Model.Value, + subject: Subject, + path: Path, + ) { + self.document = document + self.subject = subject + self.path = path + } + } +} diff --git a/Sources/Validation/Validatable.swift b/Sources/Validation/Validatable.swift new file mode 100644 index 0000000..8d55c79 --- /dev/null +++ b/Sources/Validation/Validatable.swift @@ -0,0 +1,10 @@ +public extension PureYAML.Validation { + /// Marker for a value a type-directed validation can be applied to. A `Check` fires on + /// `Subject` values wherever they appear in the traversed tree; this protocol constrains the + /// subjects so the erased dispatch can filter by runtime type. + protocol Validatable: Sendable {} +} + +extension PureYAML.Model.Value: PureYAML.Validation.Validatable {} +extension PureYAML.Model.Mapping: PureYAML.Validation.Validatable {} +extension PureYAML.Model.Key: PureYAML.Validation.Validatable {} diff --git a/Sources/Validation/Validator.swift b/Sources/Validation/Validator.swift index 6ea1a40..ca8fbd7 100644 --- a/Sources/Validation/Validator.swift +++ b/Sources/Validation/Validator.swift @@ -2,13 +2,15 @@ public extension PureYAML.Validation { /// Traverses a YAML value tree and applies validation rules at each node. struct Validator: Sendable { public var rules: [Rule] + public var checks: [AnyCheck] - public init(rules: [Rule] = Self.defaultRules) { + public init(rules: [Rule] = Self.defaultRules, checks: [AnyCheck] = []) { self.rules = rules + self.checks = checks } public static var blank: Self { - Self(rules: []) + Self(rules: [], checks: []) } public static var defaultRules: [Rule] { @@ -64,19 +66,28 @@ public extension PureYAML.Validation { return copy } - /// Removes every rule whose description matches, the identity the validation idiom uses for - /// removal. Descriptions are therefore required to be unique and stable across the rule set. - public func withoutValidating(_ description: String) -> Self { + /// Adds a type-directed check, which fires only on values of its `Subject` type wherever they + /// occur in the tree. + public func validating(_ check: Check) -> Self { var copy = self - copy.rules.removeAll { $0.description == description } + copy.checks.append(AnyCheck(check)) return copy } - /// The descriptions of the active rules, in application order: the fixed point a - /// configuration-pin test asserts against, so adding, removing, or rewording a rule changes - /// it deliberately. + /// Removes every rule or check whose description matches, the identity the validation idiom + /// uses for removal. Descriptions are therefore required to be unique and stable. + public func withoutValidating(_ description: String) -> Self { + var copy = self + copy.rules.removeAll { $0.description == description } + copy.checks.removeAll { $0.description == description } + return copy + } + + /// The descriptions of the active rules and checks, in application order: the fixed point a + /// configuration-pin test asserts against, so adding, removing, or rewording one changes it + /// deliberately. public var validationDescriptions: [String] { - rules.map(\.description) + rules.map(\.description) + checks.map(\.description) } } } @@ -93,6 +104,17 @@ extension PureYAML.Validation.Validator { issues.append(contentsOf: rule.apply(context)) } + // Offer each type-directed check the values it may specialise on at this node: the value + // itself, and, for a mapping, the mapping and each of its keys. The erased check filters by + // runtime type, so a `Check` ignores the value and key offers. + offer(subject, at: path, root: root, issues: &issues) + if case let .mapping(mapping) = subject { + offer(mapping, at: path, root: root, issues: &issues) + for pair in mapping.pairs { + offer(pair.keyNode, at: path.appending(pair.keyNode.pathComponent), root: root, issues: &issues) + } + } + switch subject { case let .mapping(mapping): for pair in mapping.pairs { @@ -116,4 +138,15 @@ extension PureYAML.Validation.Validator { break } } + + func offer( + _ subject: any PureYAML.Validation.Validatable, + at path: PureYAML.Validation.Path, + root: PureYAML.Model.Value, + issues: inout [PureYAML.Validation.Issue], + ) { + for check in checks { + issues.append(contentsOf: check.apply(to: subject, at: path, in: root)) + } + } } diff --git a/Tests/ValidationCheckMachineryTests.swift b/Tests/ValidationCheckMachineryTests.swift new file mode 100644 index 0000000..241db40 --- /dev/null +++ b/Tests/ValidationCheckMachineryTests.swift @@ -0,0 +1,99 @@ +@testable import PureYAML +import Testing + +@Suite("Type-directed check machinery") +struct ValidationCheckMachineryTests { + private typealias Check = PureYAML.Validation.Check + private typealias AnyCheck = PureYAML.Validation.AnyCheck + private typealias Issue = PureYAML.Validation.Issue + + private var mapping: PureYAML.Model.Mapping { + .init([.init(key: "a", value: .int(1))]) + } + + private func alwaysFails(description: String = "always fails") -> Check { + Check(description: description) { context in + [Issue(severity: .error, reason: "boom", path: context.path)] + } + } + + // MARK: machinery negatives (the dispatch must not fire where it should not) + + @Test("A check fires on its subject type and produces its error") + func test_firesOnSubjectType() { + let erased = AnyCheck(alwaysFails()) + #expect(erased.apply(to: mapping, at: .root, in: .null).count == 1) + } + + @Test("A check does not fire on a value of another type") + func test_doesNotFireOnWrongType() { + let erased = AnyCheck(alwaysFails()) + // A Model.Value (even one wrapping a mapping) is not a Model.Mapping, so nothing fires. + #expect(erased.apply(to: PureYAML.Model.Value.mapping(mapping), at: .root, in: .null).isEmpty) + #expect(erased.apply(to: PureYAML.Model.Key.string("a"), at: .root, in: .null).isEmpty) + } + + @Test("A false `when` predicate yields no errors") + func test_falsePredicateYieldsNothing() { + let gated = Check(description: "gated", check: { _ in + [Issue(severity: .error, reason: "boom", path: .root)] + }, when: { _ in false }) + #expect(AnyCheck(gated).apply(to: mapping, at: .root, in: .null).isEmpty) + } + + // MARK: positive control (proving the negatives are not vacuous) + + @Test("The same subject type occurring twice yields two errors") + func test_sameTypeTwiceYieldsTwoErrors() { + // A document with two mappings; an always-failing Check must fire on both. + let document = PureYAML.Model.Value.sequence(.init([ + .mapping(.init([.init(key: "a", value: .int(1))])), + .mapping(.init([.init(key: "b", value: .int(2))])), + ])) + let validator = PureYAML.Validation.Validator.blank.validating(alwaysFails()) + let issues = validator.collect(document).issues + #expect(issues.count == 2) + } + + // MARK: the type-directed builtin (failing and a near miss) + + @Test("The type-directed mappingKeysAreUnique builtin catches duplicate keys") + func test_builtinCatchesDuplicateKeys() { + let validator = PureYAML.Validation.Validator.blank.validating( + PureYAML.Validation.BuiltinCheck.mappingKeysAreUnique, + ) + #expect(validator.validationDescriptions == ["Mapping keys are unique"]) + #expect(throws: PureYAML.Validation.Issue.Collection.self) { + try validator.validate(.mapping(.init([ + .init(key: "a", value: .int(1)), + .init(key: "a", value: .int(2)), + ]))) + } + } + + @Test("The builtin passes a near-miss document whose keys are unique") + func test_builtinPassesUniqueKeys() throws { + let validator = PureYAML.Validation.Validator.blank.validating( + PureYAML.Validation.BuiltinCheck.mappingKeysAreUnique, + ) + // The boundary the rule almost trips: two keys that differ by one character. + _ = try validator.validate(.mapping(.init([ + .init(key: "a", value: .int(1)), + .init(key: "b", value: .int(1)), + ]))) + } + + @Test("Type-directed dispatch reaches a nested mapping") + func test_dispatchReachesNestedMapping() { + let validator = PureYAML.Validation.Validator.blank.validating( + PureYAML.Validation.BuiltinCheck.mappingKeysAreUnique, + ) + let nested = PureYAML.Model.Value.mapping(.init([ + .init(key: "outer", value: .mapping(.init([ + .init(key: "a", value: .int(1)), + .init(key: "a", value: .int(2)), + ]))), + ])) + #expect(!validator.collect(nested).errors.isEmpty) + } +} diff --git a/scripts/validation-coverage.tsv b/scripts/validation-coverage.tsv index 19b9d54..626b77f 100644 --- a/scripts/validation-coverage.tsv +++ b/scripts/validation-coverage.tsv @@ -81,3 +81,8 @@ PureYAML.Validation.BatchReport excluded diagnostic report data-transfer type PureYAML.Validation.Diagnostic excluded diagnostic data-transfer type PureYAML.Validation.DiagnosticKind excluded closed enum; illegal states unrepresentable PureYAML.Validation.PreflightScanner excluded pre-parse scanner engine; not a validated subject +PureYAML.Validation.Validatable excluded marker protocol constraining check subjects; no instances or state of its own +PureYAML.Validation.SubjectContext excluded read-only context passed to a type-directed check; not a subject +PureYAML.Validation.Check excluded the type-directed validation value (Validation role); a rule, not a subject +PureYAML.Validation.AnyCheck excluded type-erased check; validation dispatch machinery +PureYAML.Validation.BuiltinCheck excluded caseless namespace of built-in checks