daml2ts: Fix validation of nested optionals (digital-asset#4128)

Currently, the validator for `Optional (Optional _)` would allow the value `[null]`, which is not the JSON encoding for any value of this type. This PR fixes the issue. Since it detects companion objects for the `Optional` by means of their JavaScript class, we can also drop the `isOptional` property from the `Serializable` interface. CHANGELOG_BEGIN CHANGELOG_END
iggy-da · Jan 20, 2020 · b0ade66 · b0ade66
1 parent 92a3628
commit b0ade66
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 38 deletions.
diff --git a/language-support/ts/codegen/src/TsCodeGenMain.hs b/language-support/ts/codegen/src/TsCodeGenMain.hs
@@ -167,7 +167,6 @@ genDefDataType curPkgId conName mod tpls def =
                   body = map ("  " <>) $
                     -- The variant deserializer.
                     ["decoder: () => jtv.oneOf<" <> typ <> ">("] ++  sers ++ ["),"] ++
-                    ["isOptional: false,"] ++
                     -- Remember how we dropped the first line of each
                     -- associated serializer above? This replaces them.
                     concatMap (\(n, ser) -> n <> ": ({" : onLast (<> ",") ser) assocSers
@@ -230,7 +229,6 @@ genDefDataType curPkgId conName mod tpls def =
                             ,"  keyDecoder: " <> keySer <> ","
                             ] ++
                             map ("  " <>) (onLast (<> ",") (onHead ("decoder: " <>) serDesc)) ++
-                            ["  isOptional: false,"] ++
                             concat
                             [ ["  " <> x <> ": {"
                               ,"    template: () => " <> conName <> ","
@@ -269,14 +267,12 @@ genDefDataType curPkgId conName mod tpls def =
         makeSer serDesc =
             ["export const " <> conName <> serHeader <> " ({"] ++
             map ("  " <>) (onLast (<> ",") (onHead ("decoder: " <>) serDesc)) ++
-            ["  isOptional: false,"] ++
             ["})"]
         makeNameSpace serDesc =
             [ "// eslint-disable-next-line @typescript-eslint/no-namespace"
             , "export namespace " <> conName <> " {"
             ] ++
             map ("  " <>) (onHead ("export const decoder = " <>) serDesc) ++
-            ["  export const isOptional = false;"] ++
             ["}"]
         genBranch (VariantConName cons, t) =
           let (typ, ser) = genType (moduleName mod) t in

diff --git a/language-support/ts/daml-json-types/index.test.ts b/language-support/ts/daml-json-types/index.test.ts
@@ -19,7 +19,6 @@ describe('daml-json-types', () => {
     expect(dict.decoder().run('X').ok).toBe(false);
     expect(dict.decoder().run([['X']]).ok).toBe(false);
     expect(dict.decoder().run([[]]).ok).toBe(false);
-    // FIXME(MH): The decoder for `Optional` is slightly off in this case.
-    // expect(dict.decoder().run([null]).ok).toBe(false);
+    expect(dict.decoder().run([null]).ok).toBe(false);
   });
 });
diff --git a/language-support/ts/daml-json-types/index.ts b/language-support/ts/daml-json-types/index.ts
@@ -9,7 +9,6 @@ import * as jtv from '@mojotech/json-type-validation';
 export interface Serializable<T> {
   // NOTE(MH): This must be a function to allow for mutually recursive decoders.
   decoder: () => jtv.Decoder<T>;
-  isOptional: boolean;
 }
 
 /**
@@ -72,7 +71,6 @@ export type Unit = {};
  */
 export const Unit: Serializable<Unit> = {
   decoder: () => jtv.object({}),
-  isOptional: false,
 }
 
 /**
@@ -85,7 +83,6 @@ export type Bool = boolean;
  */
 export const Bool: Serializable<Bool> = {
   decoder: jtv.boolean,
-  isOptional: false,
 }
 
 /**
@@ -99,7 +96,6 @@ export type Int = string;
  */
 export const Int: Serializable<Int> = {
   decoder: jtv.string,
-  isOptional: false,
 }
 
 /**
@@ -117,7 +113,6 @@ export type Decimal = Numeric;
 export const Numeric = (_: number): Serializable<Numeric> =>
   ({
     decoder: jtv.string,
-    isOptional: false,
   })
 
 export const Decimal: Serializable<Decimal> = Numeric(10)
@@ -132,7 +127,6 @@ export type Text = string;
  */
 export const Text: Serializable<Text> = {
   decoder: jtv.string,
-  isOptional: false,
 }
 
 /**
@@ -146,7 +140,6 @@ export type Time = string;
  */
 export const Time: Serializable<Time> = {
   decoder: jtv.string,
-  isOptional: false,
 }
 
 /**
@@ -160,7 +153,6 @@ export type Party = string;
  */
 export const Party: Serializable<Party> = {
   decoder: jtv.string,
-  isOptional: false,
 }
 
 /**
@@ -173,7 +165,6 @@ export type List<T> = T[];
  */
 export const List = <T>(t: Serializable<T>): Serializable<T[]> => ({
   decoder: () => jtv.array(t.decoder()),
-  isOptional: false,
 });
 
 /**
@@ -187,7 +178,6 @@ export type Date = string;
  */
 export const Date: Serializable<Date> = {
   decoder: jtv.string,
-  isOptional: false,
 }
 
 /**
@@ -203,35 +193,55 @@ export type ContractId<T> = string;
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
 export const ContractId = <T>(_t: Serializable<T>): Serializable<ContractId<T>> => ({
   decoder: jtv.string,
-  isOptional: false,
 });
 
 /**
  * The counterpart of DAML's `Optional T` type.
  */
-export type Optional<T> =
-  | null
-  | Optional.Inner<T>
-// eslint-disable-next-line @typescript-eslint/no-namespace
-namespace Optional {
-  export type Inner<T> = null extends T ? ([] | [Exclude<T, null>]) : T;
-}
+export type Optional<T> = null | OptionalInner<T>
+
+type OptionalInner<T> = null extends T ? [] | [Exclude<T, null>] : T
 
 /**
  * Companion function of the `Optional` type.
  */
-export const Optional = <T>(t: Serializable<T>): Serializable<Optional<T>> => ({
-  decoder : () => jtv.oneOf<Optional<T>>(jtv.constant(null), Optional.Inner(t)),
-  isOptional: true,
-});
-Optional.Inner = <T>(t: Serializable<T>): jtv.Decoder<Optional.Inner<T>> =>
-  ! t.isOptional
-  ? t.decoder() as jtv.Decoder<Optional.Inner<T>>
-  : jtv.oneOf(
-      jtv.constant([]) as jtv.Decoder<[] | [Exclude<T, null>]>,
-      jtv.tuple([t.decoder()]) as jtv.Decoder<[] | [Exclude<T, null>]>
-  ) as jtv.Decoder<Optional.Inner<T>>
-;
+export const Optional = <T>(t: Serializable<T>): Serializable<Optional<T>> =>
+  new OptionalWorker(t);
+
+/**
+ * This class does the actual work behind the `Optional` companion function.
+ * In addition to implementing the `Serializable` interface it also stores
+ * the `Serializable` instance of the payload of the `Optional` and uses it to
+ * provide a decoder for the `OptionalInner` type.
+ */
+class OptionalWorker<T> implements Serializable<Optional<T>> {
+  constructor(private payload: Serializable<T>) { }
+
+  decoder(): jtv.Decoder<Optional<T>> {
+    return jtv.oneOf(jtv.constant(null), this.innerDecoder());
+  }
+
+  private innerDecoder(): jtv.Decoder<OptionalInner<T>> {
+    if (this.payload instanceof OptionalWorker) {
+      // NOTE(MH): `T` is of the form `Optional<U>` for some `U` here, that is
+      // `T = Optional<U> = null | OptionalInner<U>`. Since `null` does not
+      // extend `OptionalInner<V>` for any `V`, this implies
+      // `OptionalInner<U> = Exclude<T, null>`. This also implies
+      // `OptionalInner<T> = [] | [Exclude<T, null>]`.
+      type OptionalInnerU = Exclude<T, null>
+      const payloadInnerDecoder =
+        this.payload.innerDecoder() as jtv.Decoder<unknown> as jtv.Decoder<OptionalInnerU>;
+      return jtv.oneOf<[] | [Exclude<T, null>]>(
+        jtv.constant<[]>([]),
+        jtv.tuple([payloadInnerDecoder]),
+      ) as jtv.Decoder<OptionalInner<T>>;
+    } else {
+      // NOTE(MH): `T` is not of the form `Optional<U>` here and hence `null`
+      // does not extend `T`. Thus, `OptionalInner<T> = T`.
+      return this.payload.decoder() as jtv.Decoder<OptionalInner<T>>;
+    }
+  }
+}
 
 /**
  * The counterpart of DAML's `TextMap T` type. We represent `TextMap`s as
@@ -244,7 +254,6 @@ export type TextMap<T> = { [key: string]: T };
  */
 export const TextMap = <T>(t: Serializable<T>): Serializable<TextMap<T>> => ({
   decoder: () => jtv.dict(t.decoder()),
-  isOptional: false,
 });
 
 // TODO(MH): `Map` type.
diff --git a/language-support/ts/daml-ledger-fetch/index.ts b/language-support/ts/daml-ledger-fetch/index.ts
@@ -40,7 +40,7 @@ const decodeCreateEventUnknown: jtv.Decoder<CreateEvent<object>> =
 
 const decodeArchiveEventUnknown: jtv.Decoder<ArchiveEvent<object>> = jtv.object({
   templateId: jtv.string(),
-  contractId: ContractId({decoder: jtv.unknownJson, isOptional: false}).decoder(),
+  contractId: ContractId({decoder: jtv.unknownJson}).decoder(),
 });
 
 const decodeEventUnknown: jtv.Decoder<Event<object>> = jtv.oneOf<Event<object>>(