diff --git a/.changeset/fix-schema-encoding-checks.md b/.changeset/fix-schema-encoding-checks.md new file mode 100644 index 0000000000..ba56790cbb --- /dev/null +++ b/.changeset/fix-schema-encoding-checks.md @@ -0,0 +1,12 @@ +--- +"effect": patch +--- + +Fix Schema handling of encoded-side checks for container ASTs. + +Checks added after `flip` are now preserved as `encodingChecks` across +`Declaration`, `Arrays`, `Objects`, and `Union`, even when rebuilding the AST +does not change child nodes. `toType` now projects those checks consistently, +and parsing applies encoded-side checks to the local encoded value when an +encoding chain is present without allowing encoded-side `parseOptions` +annotations to affect the current parser side. diff --git a/packages/effect/src/SchemaAST.ts b/packages/effect/src/SchemaAST.ts index 7b6e8e15f0..3c7420dd13 100644 --- a/packages/effect/src/SchemaAST.ts +++ b/packages/effect/src/SchemaAST.ts @@ -680,7 +680,7 @@ export class Declaration extends Base { } private _rebuild(recur: (ast: AST) => AST, checks: Checks | undefined, encodingChecks: Checks | undefined) { const tps = mapOrSame(this.typeParameters, recur) - return tps === this.typeParameters ? + return tps === this.typeParameters && checks === this.checks && encodingChecks === this.encodingChecks ? this : new Declaration(tps, this.run, this.annotations, checks, undefined, this.context, encodingChecks) } @@ -1704,7 +1704,8 @@ export class Arrays extends Base { private _rebuild(recur: (ast: AST) => AST, checks: Checks | undefined, encodingChecks: Checks | undefined) { const elements = mapOrSame(this.elements, recur) const rest = mapOrSame(this.rest, recur) - return elements === this.elements && rest === this.rest ? + return elements === this.elements && rest === this.rest && checks === this.checks && + encodingChecks === this.encodingChecks ? this : new Arrays( this.isMutable, @@ -2245,7 +2246,8 @@ export class Objects extends Base { : new IndexSignature(p, t, merge) }) - return props === this.propertySignatures && indexes === this.indexSignatures + return props === this.propertySignatures && indexes === this.indexSignatures && checks === this.checks && + encodingChecks === this.encodingChecks ? this : new Objects( props, @@ -2661,7 +2663,7 @@ export class Union extends Base { } private _rebuild(recur: (ast: AST) => AST, checks: Checks | undefined, encodingChecks: Checks | undefined) { const types = mapOrSame(this.types, recur) - return types === this.types ? + return types === this.types && checks === this.checks && encodingChecks === this.encodingChecks ? this : new Union(types, this.mode, this.annotations, checks, undefined, this.context, encodingChecks) } @@ -2860,19 +2862,6 @@ export class Suspend extends Base { } } -/** @internal */ -export function getEncodingChecks(ast: AST): Checks | undefined { - switch (ast._tag) { - case "Declaration": - case "Arrays": - case "Objects": - case "Union": - return ast.encodingChecks - default: - return undefined - } -} - // ----------------------------------------------------------------------------- // Checks // ----------------------------------------------------------------------------- @@ -3392,9 +3381,13 @@ export const toType = memoize((ast: A): A => { } const out: any = ast const type = out.recur?.(toType) ?? out - if (getEncodingChecks(type)) { + const encodingChecks = type.encodingChecks + if (encodingChecks) { return modifyOwnPropertyDescriptors(type, (d) => { d.encodingChecks.value = undefined + if (type === ast) { + d.checks.value = combineChecks(type.checks, encodingChecks) + } }) } return type diff --git a/packages/effect/src/SchemaParser.ts b/packages/effect/src/SchemaParser.ts index cdcf28e78b..d0f2b3589b 100644 --- a/packages/effect/src/SchemaParser.ts +++ b/packages/effect/src/SchemaParser.ts @@ -1007,11 +1007,14 @@ export interface Parser { const recur = memoize( (ast: SchemaAST.AST): Parser => { let parser: Parser - const encodingChecks = SchemaAST.getEncodingChecks(ast) - const resolvedChecks = ast.checks ?? encodingChecks - const astOptions = (resolvedChecks ? resolvedChecks[resolvedChecks.length - 1].annotations : ast.annotations) + const checks = ast.checks + const encoding = ast.encoding + const links = encoding + const len = links?.length ?? 0 + const encodingChecks = (ast as any).encodingChecks + const astOptions = (checks ? checks[checks.length - 1].annotations : ast.annotations) ?.["parseOptions"] - if (!ast.context && !ast.encoding && !ast.checks && !encodingChecks) { + if (!ast.context && !encoding && !checks && !encodingChecks) { return (ou, options) => { parser ??= ast.getParser(recur) if (astOptions) { @@ -1022,15 +1025,15 @@ const recur = memoize( } const isStructural = SchemaAST.isArrays(ast) || SchemaAST.isObjects(ast) || (SchemaAST.isDeclaration(ast) && ast.typeParameters.length > 0) + const structuralChecks = checks && isStructural ? + checks.filter((check) => check.annotations?.[SchemaAST.STRUCTURAL_ANNOTATION_KEY]) : + undefined return (ou, options) => { if (astOptions) { options = { ...options, ...astOptions } } - const encoding = ast.encoding let srou: Effect.Effect, SchemaIssue.Issue, unknown> | undefined - if (encoding) { - const links = encoding - const len = links.length + if (links) { for (let i = len - 1; i >= 0; i--) { const link = links[i] const to = link.to @@ -1047,58 +1050,63 @@ const recur = memoize( } parser ??= ast.getParser(recur) - let sroa = srou ? Effect.flatMapEager(srou, (ou) => parser(ou, options)) : parser(ou, options) + const parseLocal = (localOu: Option.Option) => { + let sroa = parser(localOu, options) - if (encodingChecks && !options?.disableChecks) { - sroa = Effect.flatMapEager(sroa, (oa) => { - if (Option.isSome(ou) && Option.isSome(oa)) { - const issues: Array = [] + if (encodingChecks && !options?.disableChecks) { + sroa = Effect.flatMapEager(sroa, (oa) => { + if (Option.isSome(localOu) && Option.isSome(oa)) { + const issues: Array = [] - SchemaAST.collectIssues(encodingChecks, ou.value, issues, ast, options) + SchemaAST.collectIssues(encodingChecks, localOu.value, issues, ast, options) - if (Arr.isArrayNonEmpty(issues)) { - return Effect.fail(new SchemaIssue.Composite(ast, ou, issues)) + if (Arr.isArrayNonEmpty(issues)) { + return Effect.fail(new SchemaIssue.Composite(ast, localOu, issues)) + } } - } - return Effect.succeed(oa) - }) - } - - if (ast.checks && !options?.disableChecks) { - const checks = ast.checks - if (options?.errors === "all" && isStructural && Option.isSome(ou)) { - sroa = mapSchemaIssueEffect(sroa, (issue) => { - const issues: Array = [] - SchemaAST.collectIssues( - checks.filter((check) => check.annotations?.[SchemaAST.STRUCTURAL_ANNOTATION_KEY]), - ou.value, - issues, - ast, - options - ) - const out: SchemaIssue.Issue = Arr.isArrayNonEmpty(issues) - ? issue._tag === "Composite" && issue.ast === ast - ? new SchemaIssue.Composite(ast, issue.actual, [...issue.issues, ...issues]) - : new SchemaIssue.Composite(ast, ou, [issue, ...issues]) - : issue - return out + return Effect.succeed(oa) }) } - sroa = Effect.flatMapEager(sroa, (oa) => { - if (Option.isSome(oa)) { - const value = oa.value - const issues: Array = [] - SchemaAST.collectIssues(checks, value, issues, ast, options) + if (checks && !options?.disableChecks) { + if (options?.errors === "all" && structuralChecks && structuralChecks.length > 0 && Option.isSome(localOu)) { + sroa = mapSchemaIssueEffect(sroa, (issue) => { + const issues: Array = [] + SchemaAST.collectIssues( + structuralChecks, + localOu.value, + issues, + ast, + options + ) + const out: SchemaIssue.Issue = Arr.isArrayNonEmpty(issues) + ? issue._tag === "Composite" && issue.ast === ast + ? new SchemaIssue.Composite(ast, issue.actual, [...issue.issues, ...issues]) + : new SchemaIssue.Composite(ast, localOu, [issue, ...issues]) + : issue + return out + }) + } + sroa = Effect.flatMapEager(sroa, (oa) => { + if (Option.isSome(oa)) { + const value = oa.value + const issues: Array = [] + + SchemaAST.collectIssues(checks, value, issues, ast, options) - if (Arr.isArrayNonEmpty(issues)) { - return Effect.fail(new SchemaIssue.Composite(ast, oa, issues)) + if (Arr.isArrayNonEmpty(issues)) { + return Effect.fail(new SchemaIssue.Composite(ast, oa, issues)) + } } - } - return Effect.succeed(oa) - }) + return Effect.succeed(oa) + }) + } + + return sroa } + const sroa = srou ? Effect.flatMapEager(srou, parseLocal) : parseLocal(ou) + return sroa } } diff --git a/packages/effect/test/schema/Schema.test.ts b/packages/effect/test/schema/Schema.test.ts index f01f93c0b6..31fddafe37 100644 --- a/packages/effect/test/schema/Schema.test.ts +++ b/packages/effect/test/schema/Schema.test.ts @@ -111,6 +111,31 @@ Expected an integer, got -1.2` at ["b"]["c"]` ) }) + + it("should not read parseOptions from encodingChecks", async () => { + const schema = Schema.Struct({ + a: Schema.String, + b: Schema.String + }).pipe( + Schema.flip, + Schema.check(Schema.isMaxProperties(1)), + Schema.annotate({ parseOptions: { errors: "first" } }), + Schema.flip + ) + assertTrue(SchemaAST.isObjects(schema.ast)) + strictEqual(schema.ast.checks, undefined) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding({ parseOptions: { errors: "all" } }) + await decoding.fail( + {}, + `Missing key + at ["a"] +Missing key + at ["b"]` + ) + }) }) describe("parse options", () => { @@ -2441,6 +2466,182 @@ Expected a value with a size of at most 2, got Map([["a",1],["b",NaN],["c",3]])` await encoding.succeed("123", 123) }) + it("Struct & flip & check & flip should apply the check to the encoded side", async () => { + const schema = Schema.Struct({ a: Schema.String }).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((o) => o.a.length > 1, { expected: "a length > 1" })), + Schema.flip + ) + assertTrue(SchemaAST.isObjects(schema.ast)) + strictEqual(schema.ast.checks, undefined) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding() + await decoding.fail( + { a: "a" }, + `Expected a length > 1, got {"a":"a"}` + ) + await decoding.succeed({ a: "aa" }) + + const encoding = asserts.encoding() + await encoding.fail( + { a: "a" }, + `Expected a length > 1, got {"a":"a"}` + ) + await encoding.succeed({ a: "aa" }) + }) + + it("Tuple & flip & check & flip should apply the check to the encoded side", async () => { + const schema = Schema.Tuple([Schema.String]).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((tuple) => tuple[0].length > 1, { expected: "head length > 1" })), + Schema.flip + ) + assertTrue(SchemaAST.isArrays(schema.ast)) + strictEqual(schema.ast.checks, undefined) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding() + await decoding.fail( + ["a"], + `Expected head length > 1, got ["a"]` + ) + await decoding.succeed(["aa"]) + + const encoding = asserts.encoding() + await encoding.fail( + ["a"], + `Expected head length > 1, got ["a"]` + ) + await encoding.succeed(["aa"]) + }) + + it("Union & flip & check & flip should apply the check to the encoded side", async () => { + const schema = Schema.Union([Schema.Literal("a"), Schema.Literal("aa")]).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((s) => s === "aa", { expected: `"aa"` })), + Schema.flip + ) + assertTrue(SchemaAST.isUnion(schema.ast)) + strictEqual(schema.ast.checks, undefined) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding() + await decoding.fail( + "a", + `Expected "aa", got "a"` + ) + await decoding.succeed("aa") + + const encoding = asserts.encoding() + await encoding.fail( + "a", + `Expected "aa", got "a"` + ) + await encoding.succeed("aa") + }) + + it("Declaration & flip & check & flip should apply the check to the encoded side", async () => { + const schema = Schema.declare( + (u): u is string => typeof u === "string", + { expected: "string declaration" } + ).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((s) => s.length > 1, { expected: "a length > 1" })), + Schema.flip + ) + assertTrue(SchemaAST.isDeclaration(schema.ast)) + strictEqual(schema.ast.checks, undefined) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding() + await decoding.fail( + "a", + `Expected a length > 1, got "a"` + ) + await decoding.succeed("aa") + + const encoding = asserts.encoding() + await encoding.fail( + "a", + `Expected a length > 1, got "a"` + ) + await encoding.succeed("aa") + }) + + it("Struct & flip & check & flip with encoding chain should check the local value", async () => { + const local = Schema.Struct({ a: Schema.String }).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((o) => typeof o.a === "string" && o.a.length > 1, { + expected: "a length > 1" + })), + Schema.flip + ) + const schema = Schema.Struct({ b: Schema.String }).pipe( + Schema.decodeTo(local, { + decode: SchemaGetter.transform<{ readonly a: string }, { readonly b: string }>((o) => ({ a: o.b })), + encode: SchemaGetter.transform<{ readonly b: string }, { readonly a: string }>((o) => ({ b: o.a })) + }) + ) + assertTrue(SchemaAST.isObjects(schema.ast)) + strictEqual(schema.ast.encoding?.length, 1) + strictEqual(schema.ast.encodingChecks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding() + await decoding.fail( + { b: "a" }, + `Expected a length > 1, got {"a":"a"}` + ) + await decoding.succeed({ b: "aa" }, { a: "aa" }) + + const encoding = asserts.encoding() + await encoding.fail( + { a: "a" }, + `Expected a length > 1, got {"a":"a"}` + ) + await encoding.succeed({ a: "aa" }, { b: "aa" }) + }) + + it(`Struct & encoding chain & structural checks should check the local value with errors: "all"`, async () => { + const local = Schema.Struct({ a: Schema.Finite }).check(Schema.isMaxProperties(1)) + const schema = Schema.Struct({ b: Schema.Number, c: Schema.String }).pipe( + Schema.decodeTo(local, { + decode: SchemaGetter.transform< + { readonly a: number }, + { readonly b: number; readonly c: string } + >((o) => ({ a: o.b })), + encode: SchemaGetter.transform< + { readonly b: number; readonly c: string }, + { readonly a: number } + >((o) => ({ b: o.a, c: "" })) + }) + ) + assertTrue(SchemaAST.isObjects(schema.ast)) + strictEqual(schema.ast.encoding?.length, 1) + strictEqual(schema.ast.checks?.length, 1) + const asserts = new TestSchema.Asserts(schema) + + const decoding = asserts.decoding({ parseOptions: { errors: "all" } }) + await decoding.fail( + { b: NaN, c: "extra" }, + `Expected a finite number, got NaN + at ["a"]` + ) + + const encoding = asserts.encoding({ parseOptions: { errors: "all" } }) + await encoding.fail( + { a: NaN }, + `Expected a finite number, got NaN + at ["a"]` + ) + await encoding.succeed({ a: 1 }, { b: 1, c: "" }) + }) + it("should work with withConstructorDefault", async () => { const schema = Schema.Struct({ a: Schema.FiniteFromString.pipe(Schema.withConstructorDefault(Effect.succeed(-1))) diff --git a/packages/effect/test/schema/SchemaAST.test.ts b/packages/effect/test/schema/SchemaAST.test.ts index 6ef8837894..5ee3eda3c1 100644 --- a/packages/effect/test/schema/SchemaAST.test.ts +++ b/packages/effect/test/schema/SchemaAST.test.ts @@ -68,6 +68,36 @@ describe("SchemaAST", () => { strictEqual(SchemaAST.isStringTree(circular), false) }) + describe("toType", () => { + it("promotes encodingChecks when contained type shape is preserved", () => { + const schema = Schema.Struct({ a: Schema.String }).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((o) => o.a.length > 1)), + Schema.flip + ) + + const ast = SchemaAST.toType(schema.ast) + + strictEqual(SchemaAST.isObjects(ast), true) + strictEqual(ast.checks?.length, 1) + strictEqual(ast.encodingChecks, undefined) + }) + + it("drops encodingChecks when contained type shape changes", () => { + const schema = Schema.Struct({ a: Schema.FiniteFromString }).pipe( + Schema.flip, + Schema.check(Schema.makeFilter((o) => o.a.length > 1)), + Schema.flip + ) + + const ast = SchemaAST.toType(schema.ast) + + strictEqual(SchemaAST.isObjects(ast), true) + strictEqual(ast.checks, undefined) + strictEqual(ast.encodingChecks, undefined) + }) + }) + describe("collectSentinels", () => { describe("Declaration", () => { it("~sentinels", () => {