Merge pull request #811 from quicktype/pimp-type-attributes

Use immutable for accessor names

src/AccessorNames.ts

@@ -3,13 +3,13 @@
 import { Map, Set } from "immutable";
 
 import { TypeAttributeKind, TypeAttributes } from "./TypeAttributes";
-import { checkStringMap, isStringMap, defined } from "./Support";
+import { defined } from "./Support";
 import { EnumType, UnionType, Type, ObjectType } from "./Type";
 import { messageAssert, ErrorMessage } from "./Messages";
 
-export type AccessorEntry = string | { [language: string]: string };
+export type AccessorEntry = string | Map<string, string>;
 
-export type AccessorNames = { [key: string]: AccessorEntry };
+export type AccessorNames = Map<string, AccessorEntry>;
 
 export const accessorNamesTypeAttributeKind = new TypeAttributeKind<AccessorNames>(
     "accessorNames",
@@ -18,34 +18,23 @@ export const accessorNamesTypeAttributeKind = new TypeAttributeKind<AccessorName
     undefined
 );
 
-export function isAccessorEntry(x: any): x is AccessorEntry {
-    if (typeof x === "string") {
-        return true;
-    }
-    return isStringMap(x, (v: any): v is string => typeof v === "string");
-}
-
-export function checkAccessorNames(x: any): AccessorNames {
-    return checkStringMap(x, isAccessorEntry);
-}
-
 // Returns [name, isFixed].
 function getFromEntry(entry: AccessorEntry, language: string): [string, boolean] | undefined {
     if (typeof entry === "string") return [entry, false];
 
-    const maybeForLanguage = entry[language];
+    const maybeForLanguage = entry.get(language);
     if (maybeForLanguage !== undefined) return [maybeForLanguage, true];
 
-    const maybeWildcard = entry["*"];
+    const maybeWildcard = entry.get("*");
     if (maybeWildcard !== undefined) return [maybeWildcard, false];
 
     return undefined;
 }
 
 function lookupKey(accessors: AccessorNames, key: string, language: string): [string, boolean] | undefined {
-    if (!Object.prototype.hasOwnProperty.call(accessors, key)) return undefined;
-
-    return getFromEntry(accessors[key], language);
+    const entry = accessors.get(key);
+    if (entry === undefined) return undefined;
+    return getFromEntry(entry, language);
 }
 
 export function objectPropertyNames(o: ObjectType, language: string): Map<string, [string, boolean] | undefined> {
@@ -71,6 +60,13 @@ export function getAccessorName(
     return maybeName;
 }
 
+// Union members can be recombined and reordered, and unions are combined as well, so
+// we can't just store an array of accessor entries in a union, one array entry for each
+// union member.  Instead, we give each union in the origin type graph a union identifier,
+// and each union member type gets a map from union identifiers to accessor entries.
+// That way, no matter how the types are recombined, if we find a union member, we can look
+// up its union's identifier(s), and then look up the member's accessor entries for that
+// identifier.  Of course we might find more than one, potentially conflicting.
 export const unionIdentifierTypeAttributeKind = new TypeAttributeKind<Set<number>>(
     "unionIdentifier",
     (a, b) => a.union(b),

src/JSONSchemaInput.ts

@@ -16,7 +16,9 @@ import {
     mapSync,
     forEachSync,
     checkArray,
-    mapOptional
+    mapOptional,
+    isStringMap,
+    checkStringMap
 } from "./Support";
 import { TypeBuilder, TypeRef } from "./TypeBuilder";
 import { TypeNames } from "./TypeNames";
@@ -31,10 +33,10 @@ import {
 import { JSONSchema, JSONSchemaStore } from "./JSONSchemaStore";
 import {
     accessorNamesTypeAttributeKind,
-    checkAccessorNames,
     makeUnionIdentifierAttribute,
-    isAccessorEntry,
-    makeUnionMemberNamesAttribute
+    makeUnionMemberNamesAttribute,
+    AccessorNames,
+    AccessorEntry
 } from "./AccessorNames";
 import { ErrorMessage, messageAssert, messageError } from "./Messages";
 
@@ -438,10 +440,28 @@ function makeAttributes(schema: StringMap, loc: Location, attributes: TypeAttrib
     });
 }
 
+function isAccessorEntry(x: any): x is string | { [language: string]: string } {
+    if (typeof x === "string") {
+        return true;
+    }
+    return isStringMap(x, (v: any): v is string => typeof v === "string");
+}
+
+function makeAccessorEntry(ae: string | { [language: string]: string }): AccessorEntry {
+    if (typeof ae === "string") return ae;
+    return Map(ae);
+}
+
+function makeAccessorNames(x: any): AccessorNames {
+    // FIXME: Do proper error reporting
+    const stringMap = checkStringMap(x, isAccessorEntry);
+    return Map(stringMap).map(makeAccessorEntry);
+}
+
 function makeNonUnionAccessorAttributes(schema: StringMap): TypeAttributes | undefined {
     const maybeAccessors = schema["qt-accessors"];
     if (maybeAccessors === undefined) return undefined;
-    return accessorNamesTypeAttributeKind.makeAttributes(checkAccessorNames(maybeAccessors));
+    return accessorNamesTypeAttributeKind.makeAttributes(makeAccessorNames(maybeAccessors));
 }
 
 function checkTypeList(typeOrTypes: any, loc: Location): OrderedSet<string> {
@@ -682,7 +702,7 @@ export async function addTypesInSchema(
                 for (let i = 0; i < typeRefs.length; i++) {
                     typeBuilder.addAttributes(
                         typeRefs[i],
-                        makeUnionMemberNamesAttribute(identifierAttribute, accessors[i])
+                        makeUnionMemberNamesAttribute(identifierAttribute, makeAccessorEntry(accessors[i]))
                     );
                 }
             }

src/TypeBuilder.ts

@@ -19,7 +19,7 @@ import {
 } from "./Type";
 import { removeNullFromUnion } from "./TypeUtils";
 import { TypeGraph } from "./TypeGraph";
-import { TypeAttributes, combineTypeAttributes, TypeAttributeKind } from "./TypeAttributes";
+import { TypeAttributes, combineTypeAttributes, TypeAttributeKind, emptyTypeAttributes } from "./TypeAttributes";
 import { defined, assert, panic, setUnion, mapOptional } from "./Support";
 
 export class TypeRef {
@@ -109,7 +109,7 @@ export class TypeBuilder {
         const tref = new TypeRef(this.typeGraph, index);
         const attributes: TypeAttributes = this._addProvenanceAttributes
             ? provenanceTypeAttributeKind.makeAttributes(Set([tref]))
-            : Map();
+            : emptyTypeAttributes;
         this.typeAttributes.push(attributes);
         return tref;
     }
@@ -148,10 +148,7 @@ export class TypeBuilder {
         return [maybeType, maybeNames];
     }
 
-    addAttributes(tref: TypeRef, attributes: TypeAttributes | undefined): void {
-        if (attributes === undefined) {
-            attributes = Map();
-        }
+    addAttributes(tref: TypeRef, attributes: TypeAttributes): void {
         const index = tref.index;
         this.typeAttributes[index] = combineTypeAttributes(this.typeAttributes[index], attributes);
     }
@@ -246,7 +243,9 @@ export class TypeBuilder {
             } else {
                 result = this.forwardIfNecessary(forwardingRef, this._noEnumStringType);
             }
-            this.addAttributes(this._noEnumStringType, attributes);
+            if (attributes !== undefined) {
+                this.addAttributes(this._noEnumStringType, attributes);
+            }
             return result;
         }
         return this.addType(forwardingRef, tr => new StringType(tr, cases), attributes);