From d29acdcb8755e3fa8def7cefaf911c1a1a5abdc2 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 5 Dec 2024 21:06:38 +0400 Subject: [PATCH] Improve advanced reverse logic --- IDEAS.md | 2 +- .../tests/src/core/S_object_flatten_test.res | 10 +- .../tests/src/core/S_object_nested_test.res | 63 ++++++++++ src/S_Core.bs.mjs | 101 +++++++-------- src/S_Core.res | 115 +++++++++--------- test.js | 26 ++-- 6 files changed, 184 insertions(+), 133 deletions(-) diff --git a/IDEAS.md b/IDEAS.md index 949941fe..d663ddbe 100644 --- a/IDEAS.md +++ b/IDEAS.md @@ -42,11 +42,11 @@ let trimContract: S.contract string> = S.contract(s => { - Changed asyncParser from (i) => () => promise to (i) => promise - Look at the discriminant in unions - error message improvements - Rename S.strict to S.strict (the same for strip) +- Added S.deepStrict ## v9.1 - Add s.strict s.strip to ppx -- Add S.deepStrict - Add S.test ## v10 diff --git a/packages/tests/src/core/S_object_flatten_test.res b/packages/tests/src/core/S_object_flatten_test.res index 7faa4c98..4f8c90df 100644 --- a/packages/tests/src/core/S_object_flatten_test.res +++ b/packages/tests/src/core/S_object_flatten_test.res @@ -146,12 +146,8 @@ test("Flatten schema with duplicated field of the same type (flatten last)", t = ~op=#Parse, `i=>{if(!i||i.constructor!==Object){e[1](i)}let v0=i["foo"];if(typeof v0!=="string"){e[0](v0)}return {"foo":v0,"bar":v0,}}`, ) - // FIXME: Should validate that the fields are equal - t->U.assertCompiledCode( - ~schema, - ~op=#ReverseConvert, - `i=>{return {"foo":i["foo"],"foo":i["bar"],}}`, - ) + // FIXME: Should validate that the fields are equal and choose the right one depending on the order + t->U.assertCompiledCode(~schema, ~op=#ReverseConvert, `i=>{return {"foo":i["bar"],}}`) }) test("Flatten schema with duplicated field of different type", t => { @@ -254,7 +250,7 @@ test("Successfully serializes simple object with flatten", t => { t->U.assertCompiledCode( ~op=#ReverseConvert, ~schema, - `i=>{return {"foo":i["foo"],"bar":i["bar"],}}`, + `i=>{return {"bar":i["bar"],"foo":i["foo"],}}`, ) }) diff --git a/packages/tests/src/core/S_object_nested_test.res b/packages/tests/src/core/S_object_nested_test.res index fc467579..bd4cd055 100644 --- a/packages/tests/src/core/S_object_nested_test.res +++ b/packages/tests/src/core/S_object_nested_test.res @@ -312,6 +312,45 @@ test("S.schema object with a deep strict applied to the nested field parent", t t->U.assertCompiledCode(~schema, ~op=#ReverseConvert, `i=>{let v0=i["nested"];return i}`) }) +test("S.schema object with a deep strict applied to the nested field parent + reverse", t => { + let schema = + S.schema(s => + { + "nested": { + "foo": s.matches(S.null(S.string)), + }, + } + ) + ->S.reverse + ->S.deepStrict + + t->U.unsafeAssertEqualSchemas( + schema, + S.object(s => + s.field( + "nested", + S.schema( + s => + { + "foo": s.matches(S.option(S.string)), + }, + )->S.strict, + ) + )->S.strict, + ) + + t->U.assertCompiledCode( + ~schema, + ~op=#Parse, + `i=>{if(!i||i.constructor!==Object){e[4](i)}let v0=i["nested"],v4;if(!v0||v0.constructor!==Object){e[0](v0)}let v1=v0["foo"],v2,v3;if(v1!==void 0&&(typeof v1!=="string")){e[1](v1)}if(v1!==void 0){v2=v1}else{v2=null}for(v3 in v0){if(v3!=="foo"){e[2](v3)}}for(v4 in i){if(v4!=="nested"){e[3](v4)}}return {"nested":{"foo":v2,},}}`, + ) + t->U.assertCompiledCode( + ~schema, + ~op=#ReverseConvert, + `i=>{let v0=i["nested"],v1=v0["foo"],v2;if(v1!==null){v2=v1}else{v2=void 0}return {"nested":{"foo":v2,},}}`, + ) +}) + test("Object with a deep strict applied to the nested field parent", t => { let schema = S.object(s => s.nested("nested").field("foo", S.string))->S.deepStrict @@ -338,6 +377,30 @@ test("Object with a deep strict applied to the nested field parent", t => { t->U.assertCompiledCode(~schema, ~op=#ReverseConvert, `i=>{return {"nested":{"foo":i,},}}`) }) +test("Object with a deep strict applied to the nested field parent + reverse", t => { + let schema = + S.object(s => {"foo": s.nested("nested").field("foo", S.string)}) + ->S.reverse + ->S.deepStrict + + t->U.unsafeAssertEqualSchemas( + schema, + S.schema(s => + { + "foo": s.matches(S.string), + } + )->S.strict, + ) + + t->U.assertCompiledCode( + ~schema, + ~op=#Parse, + // FIXME: Missing type validation and strictness check + `i=>{if(!i||i.constructor!==Object){e[0](i)}return {"nested":{"foo":i["foo"],},}}`, + ) + t->U.assertCompiledCode(~schema, ~op=#ReverseConvert, `i=>{return {"nested":{"foo":i["foo"],},}}`) +}) + test("Object with nested field together with flatten", t => { let schema = S.object(s => { diff --git a/src/S_Core.bs.mjs b/src/S_Core.bs.mjs index a90c5d73..ea82528f 100644 --- a/src/S_Core.bs.mjs +++ b/src/S_Core.bs.mjs @@ -1928,13 +1928,14 @@ function description(schema) { return schema.m[descriptionMetadataId]; } -function getFullItemPath(item) { - switch (item.k) { - case 1 : - return getFullItemPath(item.of) + item.p; +function getFullDitemPath(ditem) { + switch (ditem.k) { case 0 : + return "[" + ditem.inlinedLocation + "]"; + case 1 : + return getFullDitemPath(ditem.of) + ditem.p; case 2 : - return item.p; + return ditem.p; } } @@ -2060,7 +2061,7 @@ function definitionToRitem(definition, path, ritems, ritemsByItemPath) { s: ritem_2 }; item.r = ritem; - ritemsByItemPath[getFullItemPath(item)] = ritem; + ritemsByItemPath[getFullDitemPath(item)] = ritem; return ritem; } if (Array.isArray(definition)) { @@ -2286,8 +2287,9 @@ function nested(fieldName) { return ctx$1; } -function advancedReverse(definition, kind, ditems) { +function advancedReverse(definition, to, flattened) { return function () { + var originalSchema = this; var ritemsByItemPath = {}; var ritems = []; var ritem = definitionToRitem(definition, "", ritems, ritemsByItemPath); @@ -2381,7 +2383,7 @@ function advancedReverse(definition, kind, ditems) { } }; - var getItemOutput = function (item) { + var getItemOutput = function (item, itemPath) { var ritem = item.r; if (ritem !== undefined) { var reversed = ritem.s; @@ -2395,38 +2397,32 @@ function advancedReverse(definition, kind, ditems) { return reversed.b(b, itemInput, reversed, path$2); } var reversed$1 = item.schema["~r"](); - var input = reversedToInput(reversed$1, item.p); + var input = reversedToInput(reversed$1, itemPath); var prevFlag = b.g.o; b.g.o = (prevFlag | 1) ^ 1; var output = reversed$1.b(b, input, reversed$1, path); b.g.o = prevFlag; return output; }; - var schemaOutput = function (ditems, isArray) { - var objectVal = make(b, isArray); - for(var idx = 0 ,idx_finish = ditems.length; idx < idx_finish; ++idx){ - var item = ditems[idx]; - switch (item.k) { - case 0 : - add(objectVal, item.inlinedLocation, getItemOutput(item)); - break; - case 1 : - break; - case 2 : - merge(objectVal, getItemOutput(item)); - break; - - } + if (to !== undefined) { + return getItemOutput(to, ""); + } + var isArray = originalSchema.t.TAG === "tuple"; + var items = originalSchema.t.items; + var objectVal = make(b, isArray); + if (flattened !== undefined) { + for(var idx$1 = 0 ,idx_finish$1 = flattened.length; idx$1 < idx_finish$1; ++idx$1){ + merge(objectVal, getItemOutput(flattened[idx$1], "")); } - return complete(objectVal, isArray); - }; - if (kind === "To") { - return getItemOutput(ditems[0]); - } else if (kind === "Array") { - return schemaOutput(ditems, true); - } else { - return schemaOutput(ditems, false); } + for(var idx$2 = 0 ,idx_finish$2 = items.length; idx$2 < idx_finish$2; ++idx$2){ + var item = items[idx$2]; + if (!objectVal[item.inlinedLocation]) { + add(objectVal, item.inlinedLocation, getItemOutput(item, "[" + item.inlinedLocation + "]")); + } + + } + return complete(objectVal, isArray); }); return reversed; }; @@ -2521,11 +2517,10 @@ function to(schema, definer) { var output = definitionToOutput(bb, definition, getItemOutput); b.c = b.c + allocateScope(bb); return output; - }), schema.f, advancedReverse(definition, "To", [item])); + }), schema.f, advancedReverse(definition, item, undefined)); } function object(definer) { - var ditems = []; var flattened = (void 0); var items = []; var fields = {}; @@ -2546,9 +2541,10 @@ function object(definer) { } else { var item$1 = { + k: 0, schema: flattenedSchema, - location: $$location, - inlinedLocation: inlinedLocation + inlinedLocation: inlinedLocation, + location: $$location }; items.push(item$1); fields[$$location] = item$1; @@ -2563,7 +2559,6 @@ function object(definer) { i: item_2 }; f.push(item$2); - ditems.push(item$2); return proxify(item$2); } var message = "The '" + schema.n() + "' schema can't be flattened"; @@ -2574,17 +2569,14 @@ function object(definer) { if (fields[fieldName]) { throw new Error("[rescript-schema] " + ("The field " + inlinedLocation + " defined twice with incompatible schemas")); } - var ditem_3 = "[" + inlinedLocation + "]"; var ditem = { k: 0, schema: schema, inlinedLocation: inlinedLocation, - location: fieldName, - p: ditem_3 + location: fieldName }; fields[fieldName] = ditem; items.push(ditem); - ditems.push(ditem); return proxify(ditem); }; var tag = function (tag$1, asValue) { @@ -2611,7 +2603,7 @@ function object(definer) { advanced: true }, n: name$1, - "~r": advancedReverse(definition, "Object", ditems), + "~r": advancedReverse(definition, undefined, flattened), b: advancedBuilder(definition, flattened), f: typeFilter$1, i: 0, @@ -2620,22 +2612,20 @@ function object(definer) { } function tuple(definer) { - var ditems = []; + var items = []; var item = function (idx, schema) { var $$location = idx.toString(); var inlinedLocation = "\"" + $$location + "\""; - if (ditems[idx]) { + if (items[idx]) { throw new Error("[rescript-schema] " + ("The item [" + inlinedLocation + "] is defined multiple times")); } - var ditem_3 = "[" + inlinedLocation + "]"; var ditem = { k: 0, schema: schema, inlinedLocation: inlinedLocation, - location: $$location, - p: ditem_3 + location: $$location }; - ditems[idx] = ditem; + items[idx] = ditem; return proxify(ditem); }; var tag = function (idx, asValue) { @@ -2646,26 +2636,23 @@ function tuple(definer) { tag: tag }; var definition = definer(ctx); - for(var idx = 0 ,idx_finish = ditems.length; idx < idx_finish; ++idx){ - if (!ditems[idx]) { + for(var idx = 0 ,idx_finish = items.length; idx < idx_finish; ++idx){ + if (!items[idx]) { var $$location = idx.toString(); var inlinedLocation = "\"" + $$location + "\""; - var ditem_3 = "[" + inlinedLocation + "]"; var ditem = { - k: 0, schema: unit, - inlinedLocation: inlinedLocation, location: $$location, - p: ditem_3 + inlinedLocation: inlinedLocation }; - ditems[idx] = ditem; + items[idx] = ditem; } } return makeSchema(name$2, { TAG: "tuple", - items: ditems - }, empty, advancedBuilder(definition, undefined), typeFilter$2, advancedReverse(definition, "Array", ditems)); + items: items + }, empty, advancedBuilder(definition, undefined), typeFilter$2, advancedReverse(definition, undefined, undefined)); } function definitionToSchema(definition) { diff --git a/src/S_Core.res b/src/S_Core.res index 267abec1..7957a052 100644 --- a/src/S_Core.res +++ b/src/S_Core.res @@ -2791,14 +2791,7 @@ module Schema = { // Definition item @tag("k") type rec ditem = - | @as(0) - Item({ - schema: schema, - inlinedLocation: string, - location: string, // Needed only for ditemToItem - @as("p") - path: string, - }) + | @as(0) Item({schema: schema, inlinedLocation: string, location: string}) // Needed only for ditemToItem | @as(1) ItemField({ inlinedLocation: string, @@ -2855,13 +2848,14 @@ module Schema = { @inline let getRitemPath = (ritem: ritem): string => (ritem->Obj.magic)["p"] - @inline - let getItemPath = (item: ditem): string => (item->Obj.magic)["p"] + external ditemToItem: ditem => item = "%identity" + external itemToDitem: item => ditem = "%identity" - let rec getFullItemPath = (item: ditem) => { - switch item { - | ItemField({target, path}) => Path.concat(target->getFullItemPath, path) - | _ => item->getItemPath + let rec getFullDitemPath = (ditem: ditem) => { + switch ditem { + | ItemField({target, path}) => Path.concat(target->getFullDitemPath, path) + | Item({inlinedLocation}) => inlinedLocation->Path.fromInlinedLocation + | Root({path}) => path } } @@ -2874,7 +2868,6 @@ module Schema = { let getUnsafeDitemSchema = (item: ditem) => (item->Obj.magic)["schema"] @inline let getUnsafeDitemIndex = (item: ditem): string => (item->Obj.magic)["i"] - external ditemToItem: ditem => item = "%identity" let rec getItemReversed = item => { switch item { @@ -2982,7 +2975,7 @@ module Schema = { reversed: item->getItemReversed, }) item->setItemRitem(ritem) - ritemsByItemPath->Js.Dict.set(item->getFullItemPath, ritem) + ritemsByItemPath->Js.Dict.set(item->getFullDitemPath, ritem) ritem | None => { let node = definition->Definition.toNode @@ -3246,7 +3239,9 @@ module Schema = { output } - and advancedReverse = (~definition, ~kind, ~ditems) => () => { + and advancedReverse = (~definition, ~to=?, ~flattened=?) => () => { + let originalSchema = %raw(`this`) + let definition = definition->(Obj.magic: unknown => Definition.t) let ritemsByItemPath = Js.Dict.empty() @@ -3331,7 +3326,7 @@ module Schema = { } } - let getItemOutput = item => { + let getItemOutput = (item, ~itemPath) => { switch item->getItemRitem { | Some(ritem) => { let reversed = ritem->getRitemReversed @@ -3350,7 +3345,7 @@ module Schema = { | None => // It's fine to use getUnsafeDitemSchema here, because this will never be called on ItemField let reversed = (item->getUnsafeDitemSchema).reverse() - let input = reversedToInput(reversed, ~originalPath=item->getItemPath) + let input = reversedToInput(reversed, ~originalPath=itemPath) let prevFlag = b.global.flag @@ -3362,27 +3357,38 @@ module Schema = { } } - let schemaOutput = (~ditems, ~isArray) => { - let objectVal = b->B.Val.Object.make(~isArray) - for idx in 0 to ditems->Js.Array2.length - 1 { - let item = ditems->Js.Array2.unsafe_get(idx) - switch item { - | ItemField(_) => () - | Root(_) => objectVal->B.Val.Object.merge(item->getItemOutput) - | Item({inlinedLocation}) => - objectVal->B.Val.Object.add(inlinedLocation, item->getItemOutput) + switch to { + | Some(ditem) => ditem->getItemOutput(~itemPath=Path.empty) + | None => { + let isArray = originalSchema->classify->unsafeGetVarianTag === tupleTag + let items = (originalSchema->classify->Obj.magic)["items"] + let objectVal = b->B.Val.Object.make(~isArray) + switch flattened { + | None => () + | Some(rootItems) => + for idx in 0 to rootItems->Js.Array2.length - 1 { + objectVal->B.Val.Object.merge( + rootItems->Js.Array2.unsafe_get(idx)->getItemOutput(~itemPath=Path.empty), + ) + } + } + for idx in 0 to items->Js.Array2.length - 1 { + let item: item = items->Js.Array2.unsafe_get(idx) + + // FIXME: A hack to ignore items belonging to a flattened schema + if !(objectVal->Obj.magic->Stdlib.Dict.has(item.inlinedLocation)) { + objectVal->B.Val.Object.add( + item.inlinedLocation, + item + ->itemToDitem + ->getItemOutput(~itemPath=item.inlinedLocation->Path.fromInlinedLocation), + ) + } } - } - objectVal->B.Val.Object.complete(~isArray) - } - let output = switch kind { - | #To => ditems->Js.Array2.unsafe_get(0)->getItemOutput - | #Object => schemaOutput(~ditems, ~isArray=false) - | #Array => schemaOutput(~ditems, ~isArray=true) + objectVal->B.Val.Object.complete(~isArray) + } } - - output }) reversed @@ -3423,7 +3429,7 @@ module Schema = { }), ~maybeTypeFilter=schema.maybeTypeFilter, ~metadataMap=schema.metadataMap, - ~reverse=advancedReverse(~definition, ~ditems=[item], ~kind=#To), + ~reverse=advancedReverse(~definition, ~to=item), ) } } @@ -3533,8 +3539,6 @@ module Schema = { and object: type value. (Object.s => value) => schema = definer => { - // FIXME: Get rid of it - let ditems = [] let flattened = %raw(`void 0`) let items = [] let fields = Js.Dict.empty() @@ -3553,11 +3557,11 @@ module Schema = { `The field ${inlinedLocation} defined twice with incompatible schemas`, ) | None => - let item = { + let item = Item({ schema: flattenedSchema, location, inlinedLocation, - } + })->ditemToItem items->Js.Array2.push(item)->ignore fields->Js.Dict.set(location, item) } @@ -3569,7 +3573,6 @@ module Schema = { idx: f->Js.Array2.length, }) f->Js.Array2.push(item)->ignore - ditems->Js.Array2.push(item)->ignore item->proxify } | _ => InternalError.panic(`The '${schema.name()}' schema can't be flattened`) @@ -3590,12 +3593,10 @@ module Schema = { schema, inlinedLocation, location: fieldName, - path: Path.fromInlinedLocation(inlinedLocation), }) let item = ditem->ditemToItem fields->Js.Dict.set(fieldName, item) items->Js.Array2.push(item)->ignore - ditems->Js.Array2.push(ditem)->ignore ditem->proxify } @@ -3632,11 +3633,11 @@ module Schema = { maybeTypeFilter: Some(Object.typeFilter), name: Object.name, metadataMap: Metadata.Map.empty, - reverse: advancedReverse(~definition, ~ditems, ~kind=#Object), + reverse: advancedReverse(~definition, ~flattened), } } and tuple = definer => { - let ditems = [] + let items = [] let ctx: Tuple.s = { let item: @@ -3645,16 +3646,15 @@ module Schema = { let schema = schema->toUnknown let location = idx->Js.Int.toString let inlinedLocation = `"${location}"` - if ditems->Stdlib.Array.has(idx) { + if items->Stdlib.Array.has(idx) { InternalError.panic(`The item [${inlinedLocation}] is defined multiple times`) } else { let ditem = Item({ schema, location, inlinedLocation, - path: Path.fromInlinedLocation(inlinedLocation), }) - ditems->Js.Array2.unsafe_set(idx, ditem) + items->Js.Array2.unsafe_set(idx, ditem->ditemToItem) ditem->proxify } } @@ -3670,30 +3670,29 @@ module Schema = { } let definition = definer(ctx)->(Obj.magic: 'any => unknown) - for idx in 0 to ditems->Js.Array2.length - 1 { - if ditems->Js.Array2.unsafe_get(idx)->Obj.magic->not { + for idx in 0 to items->Js.Array2.length - 1 { + if items->Js.Array2.unsafe_get(idx)->Obj.magic->not { let location = idx->Js.Int.toString let inlinedLocation = `"${location}"` - let ditem = Item({ + let ditem = { location, inlinedLocation, schema: unit->toUnknown, - path: Path.fromInlinedLocation(inlinedLocation), - }) + } - ditems->Js.Array2.unsafe_set(idx, ditem) + items->Js.Array2.unsafe_set(idx, ditem) } } makeSchema( ~name=Tuple.name, ~tagged=Tuple({ - items: ditems->(Obj.magic: array => array), + items: items, }), ~builder=advancedBuilder(~definition), ~maybeTypeFilter=Some(Tuple.typeFilter), ~metadataMap=Metadata.Map.empty, - ~reverse=advancedReverse(~definition, ~ditems, ~kind=#Array), + ~reverse=advancedReverse(~definition), ) } diff --git a/test.js b/test.js index 9d45a188..6554f6ef 100644 --- a/test.js +++ b/test.js @@ -3,24 +3,30 @@ e[4](i); } let v0 = i["nested"], - v3; + v4; if (!v0 || v0.constructor !== Object) { e[0](v0); } let v1 = v0["foo"], - v2; - if (typeof v1 !== "string") { + v2, + v3; + if (v1 !== void 0 && typeof v1 !== "string") { e[1](v1); } - for (v2 in v0) { - if (v2 !== "foo") { - e[2](v2); + if (v1 !== void 0) { + v2 = v1; + } else { + v2 = null; + } + for (v3 in v0) { + if (v3 !== "foo") { + e[2](v3); } } - for (v3 in i) { - if (v3 !== "nested") { - e[3](v3); + for (v4 in i) { + if (v4 !== "nested") { + e[3](v4); } } - return i; + return { nested: { foo: v2 } }; };