combo: allow field renaming (#1819)

Summary:
This patch expands the API of `Combo.object` such that fields in the
input JSON may be renamed in the output JSON. This often occurs in
combination with `fmap`, as we convert a simple string field with a
user-facing name to a different structured representation. For example:

```javascript
C.object({
  repoIds: C.rename(
    "repositories",
    C.fmap(C.array(C.string), repoIdToString)
  ),
});
```

This is backward-compatible and invisible when not needed: the fields of
the argument to `C.object` may now be either parsers (as before) or
results of `C.rename`.

This patch also adds a check that the required and optional key sets
don’t overlap, which could technically have happened before but is more
important now that renames are possible.

Test Plan:
Unit tests included, retaining full coverage.

wchargin-branch: combo-rename-fields

src/util/combo.js

@@ -41,7 +41,6 @@ export class Parser<+T> {
 // Helper type to extract the underlying type of a parser: for instance,
 // `ParserOutput<Parser<string>>` is just `string`.
 export type ParserOutput<P: Parser<mixed>> = $PropertyType<P, "_phantomT">;
-type ExtractParserOutput = <P: Parser<mixed>>(P) => ParserOutput<P>;
 
 // Helper to make a successful parse result. For readability.
 function success<T>(t: T): ParseResult<T> {
@@ -167,13 +166,36 @@ export function array<T>(p: Parser<T>): Parser<T[]> {
   });
 }
 
-type Fields = {+[string]: Parser<mixed>};
+// Fields for an object type. Each is either a bare parser or the result
+// of `rename("oldFieldName", p)` for a parser `p`, to be used when the
+// field name in the output type is to be different from the field name
+// in the input type.
+export type Field<+T> = Parser<T> | RenameField<T>;
+export opaque type RenameField<+T>: {+_phantomT: T} = RenameFieldImpl<T>;
+export type Fields = {+[string]: Field<mixed>};
+
+// Like `ExtractParserOutput`, but works on `Field`s even when the
+// bound ascription is checked outside of this module.
+type FieldOutput<F: Field<mixed>> = $PropertyType<F, "_phantomT">;
+type ExtractFieldOutput = <F: Field<mixed>>(F) => FieldOutput<F>;
+
+export function rename<T>(oldKey: string, parser: Parser<T>): RenameField<T> {
+  return new RenameFieldImpl(oldKey, parser);
+}
+
+class RenameFieldImpl<+T> extends Parser<T> {
+  +oldKey: string;
+  constructor(oldKey: string, parser: Parser<T>) {
+    super(parser._f);
+    this.oldKey = oldKey;
+  }
+}
 
 // Parser combinator for an object type all of whose fields are
 // required.
 type PObjectAllRequired = <FReq: Fields>(
   required: FReq
-) => Parser<$ObjMap<FReq, ExtractParserOutput>>;
+) => Parser<$ObjMap<FReq, ExtractFieldOutput>>;
 
 // Parser combinator for an object type with some required fields (maybe
 // none) and some optional ones.
@@ -182,8 +204,8 @@ type PObjectWithOptionals = <FReq: Fields, FOpt: Fields>(
   optional: FOpt
 ) => Parser<
   $Exact<{
-    ...$Exact<$ObjMap<FReq, ExtractParserOutput>>,
-    ...$Rest<$Exact<$ObjMap<FOpt, ExtractParserOutput>>, {}>,
+    ...$Exact<$ObjMap<FReq, ExtractFieldOutput>>,
+    ...$Rest<$Exact<$ObjMap<FOpt, ExtractFieldOutput>>, {}>,
   }>
 >;
 
@@ -202,32 +224,47 @@ export const object: PObject = (function object(
   requiredFields,
   optionalFields?
 ) {
+  const newKeysSeen = new Set();
+  const fields: Array<{|
+    +oldKey: string,
+    +newKey: string,
+    +required: boolean,
+    +parser: Parser<mixed>,
+  |}> = [];
+  const fieldsets = [
+    {inputFields: requiredFields, required: true},
+    {inputFields: optionalFields || {}, required: false},
+  ];
+  for (const {inputFields, required} of fieldsets) {
+    for (const newKey of Object.keys(inputFields)) {
+      const parser = inputFields[newKey];
+      if (newKeysSeen.has(newKey)) {
+        throw new Error("duplicate key: " + JSON.stringify(newKey));
+      }
+      newKeysSeen.add(newKey);
+      const oldKey = parser instanceof RenameFieldImpl ? parser.oldKey : newKey;
+      fields.push({oldKey, newKey, parser, required});
+    }
+  }
   return new Parser((x) => {
     if (typeof x !== "object" || Array.isArray(x) || x == null) {
       return failure("expected object, got " + typename(x));
     }
     const result = {};
-    const fieldsets = [
-      {fields: requiredFields, required: true},
-      {fields: optionalFields || {}, required: false},
-    ];
-    for (const {fields, required} of fieldsets) {
-      for (const key of Object.keys(fields)) {
-        const raw = x[key];
-        if (raw === undefined) {
-          if (required) {
-            return failure("missing key: " + JSON.stringify(key));
-          } else {
-            continue;
-          }
+    for (const {oldKey, newKey, parser, required} of fields) {
+      const raw = x[oldKey];
+      if (raw === undefined) {
+        if (required) {
+          return failure("missing key: " + JSON.stringify(oldKey));
+        } else {
+          continue;
         }
-        const parser = fields[key];
-        const parsed = parser.parse(raw);
-        if (!parsed.ok) {
-          return failure(`key ${JSON.stringify(key)}: ${parsed.err}`);
-        }
-        result[key] = parsed.value;
       }
+      const parsed = parser.parse(raw);
+      if (!parsed.ok) {
+        return failure(`key ${JSON.stringify(oldKey)}: ${parsed.err}`);
+      }
+      result[newKey] = parsed.value;
     }
     return success(result);
   });

src/util/combo.test.js

@@ -297,5 +297,78 @@ describe("src/util/combo", () => {
         expect(p.parseOrThrow(v)).toEqual(v);
       });
     });
+    describe("with field renaming", () => {
+      const p: C.Parser<{|
+        +one: number,
+        +two: number,
+        +three?: number,
+        +four?: number,
+      |}> = C.object(
+        {one: C.number, two: C.rename("dos", C.number)},
+        {three: C.number, four: C.rename("cuatro", C.number)}
+      );
+      it("renames both required and optional fields", () => {
+        expect(p.parseOrThrow({one: 1, dos: 2, three: 3, cuatro: 4})).toEqual({
+          one: 1,
+          two: 2,
+          three: 3,
+          four: 4,
+        });
+      });
+      it("provides missing key errors using the user-facing name", () => {
+        const thunk = () => p.parseOrThrow({one: 1, cuatro: 4});
+        expect(thunk).toThrow('missing key: "dos"');
+      });
+      it("only accepts the user-facing keys", () => {
+        const thunk = () => p.parseOrThrow({one: 1, two: 2});
+        expect(thunk).toThrow('missing key: "dos"');
+      });
+      it("only accepts the user-facing keys for optionals", () => {
+        expect(p.parseOrThrow({one: 1, dos: 2, three: 3, four: 4})).toEqual({
+          one: 1,
+          two: 2,
+          three: 3,
+        });
+      });
+      it("allows mapping one old key to multiple new keys", () => {
+        // This makes it a bit harder to see how to turn `object` into
+        // an iso, but it's the intended behavior for now, so let's test
+        // it.
+        const p: C.Parser<{|
+          +value: number,
+          +valueAsString: string,
+        |}> = C.object({
+          value: C.number,
+          valueAsString: C.rename(
+            "value",
+            C.fmap(C.number, (n) => n.toFixed())
+          ),
+        });
+        expect(p.parseOrThrow({value: 7})).toEqual({
+          value: 7,
+          valueAsString: "7",
+        });
+      });
+    });
+    it("fails when `required` and `optional` have overlapping new keys", () => {
+      expect(() => {
+        // ...even if the parser types are compatible and the old keys
+        // are different
+        C.object({hmm: C.string}, {hmm: C.rename("hum", C.string)});
+      }).toThrow('duplicate key: "hmm"');
+    });
+    it("doesn't type a rename as a parser", () => {
+      // In the (current) implementation, a `C.rename(...)` actually is
+      // a parser, for weird typing reasons. This test ensures that that
+      // implementation detail doesn't leak past the opaque type
+      // boundary.
+      const rename = C.rename("old", C.string);
+      // $ExpectFlowError
+      (rename: C.Parser<string>);
+    });
+    it("forbids renaming a rename at the type level", () => {
+      // $ExpectFlowError
+      C.rename("hmm", C.rename("old", C.string));
+    });
   });
 });