Merge pull request #893 from quicktype/more-granular-decoding

More granular decoding

src/quicktype-core/GraphRewriting.ts

@@ -375,27 +375,6 @@ export class GraphRemapBuilder extends BaseGraphRewriteBuilder {
 
         assert(maybeForwardingRef === undefined, "We can't have a forwarding ref when we remap");
 
-        const [originalType, originalAttributes] = typeAndAttributesForTypeRef(originalRef, this.originalGraph);
-
-        const attributeSources = this._attributeSources.get(originalType);
-        if (attributes === undefined) {
-            attributes = emptyTypeAttributes;
-        }
-        if (attributeSources === undefined) {
-            attributes = combineTypeAttributes(
-                "union",
-                attributes,
-                this.reconstituteTypeAttributes(originalAttributes)
-            );
-        } else {
-            attributes = combineTypeAttributes(
-                "union",
-                attributes,
-                this.reconstituteTypeAttributes(combineTypeAttributesOfTypes("union", attributeSources))
-            );
-        }
-        const newAttributes = attributes;
-
         return this.withForwardingRef(undefined, forwardingRef => {
             this.reconstitutedTypes.set(index, forwardingRef);
 
@@ -404,6 +383,27 @@ export class GraphRemapBuilder extends BaseGraphRewriteBuilder {
                 this.changeDebugPrintIndent(1);
             }
 
+            const [originalType, originalAttributes] = typeAndAttributesForTypeRef(originalRef, this.originalGraph);
+
+            const attributeSources = this._attributeSources.get(originalType);
+            if (attributes === undefined) {
+                attributes = emptyTypeAttributes;
+            }
+            if (attributeSources === undefined) {
+                attributes = combineTypeAttributes(
+                    "union",
+                    attributes,
+                    this.reconstituteTypeAttributes(originalAttributes)
+                );
+            } else {
+                attributes = combineTypeAttributes(
+                    "union",
+                    attributes,
+                    this.reconstituteTypeAttributes(combineTypeAttributesOfTypes("union", attributeSources))
+                );
+            }
+            const newAttributes = attributes;
+
             const reconstituter = new TypeReconstituter(
                 this,
                 this.canonicalOrder,

src/quicktype-core/MakeTransformations.ts

@@ -80,8 +80,8 @@ function replaceUnion(
     function transformerForKind(kind: TypeKind) {
         const member = union.findMember(kind);
         if (member === undefined) return undefined;
-        const memberTypeRef = defined(reconstitutedMembersByKind.get(kind));
-        return new UnionInstantiationTransformer(graph, memberTypeRef);
+        const memberTypeRef = memberForKind(kind);
+        return new DecodingTransformer(graph, memberTypeRef, new UnionInstantiationTransformer(graph, memberTypeRef));
     }
 
     let maybeStringType: TypeRef | undefined = undefined;
@@ -126,12 +126,17 @@ function replaceUnion(
         transformerForString = undefined;
     } else if (stringTypes.size === 1) {
         const t = defined(iterableFirst(stringTypes));
-        transformerForString = new UnionInstantiationTransformer(graph, memberForKind(t.kind));
+        const memberTypeRef = memberForKind(t.kind);
+        transformerForString = new DecodingTransformer(
+            graph,
+            memberTypeRef,
+            new UnionInstantiationTransformer(graph, memberTypeRef)
+        );
     } else {
-        transformerForString = new ChoiceTransformer(
+        transformerForString = new DecodingTransformer(
             graph,
             getStringType(),
-            Array.from(stringTypes).map(transformerForStringType)
+            new ChoiceTransformer(graph, getStringType(), Array.from(stringTypes).map(transformerForStringType))
         );
     }
 

src/quicktype-core/Run.ts

@@ -321,6 +321,8 @@ class Run implements RunContext {
             );
         }
 
+        this.time("fixed point", () => (graph = graph.rewriteFixedPoint(false, debugPrintReconstitution)));
+
         this.time(
             "make transformations",
             () =>

src/quicktype-core/Transformers.ts

@@ -256,9 +256,7 @@ export class DecodingChoiceTransformer extends Transformer {
 
         const addCase = (transformer: Transformer | undefined) => {
             if (transformer === undefined) return;
-            transformers.push(
-                transformer.reverse(targetTypeRef, new EncodingTransformer(this.graph, transformer.sourceTypeRef))
-            );
+            transformers.push(transformer.reverse(targetTypeRef, undefined));
         };
 
         addCase(this.nullTransformer);

src/quicktype-core/TypeGraph.ts

@@ -2,7 +2,7 @@ import { iterableFirst, setFilter, setUnionManyInto, setSubtract, mapMap, mapSom
 
 import { Type, ClassType, UnionType, IntersectionType } from "./Type";
 import { separateNamedTypes, SeparatedNamedTypes, isNamedType, combineTypeAttributesOfTypes } from "./TypeUtils";
-import { defined, assert, mustNotBeCalled, panic } from "./support/Support";
+import { defined, assert, panic, mustNotHappen } from "./support/Support";
 import { TypeBuilder, StringTypeMapping, NoStringTypeMapping, provenanceTypeAttributeKind } from "./TypeBuilder";
 import { GraphRewriteBuilder, GraphRemapBuilder, BaseGraphRewriteBuilder } from "./GraphRewriting";
 import { TypeNames, namesTypeAttributeKind } from "./TypeNames";
@@ -335,7 +335,6 @@ export class TypeGraph {
 
         if (!builder.didAddForwardingIntersection) return newGraph;
 
-        assert(!force, "We shouldn't have introduced forwarding intersections in a forced rewrite");
         return removeIndirectionIntersections(newGraph, stringTypeMapping, debugPrintReconstitution);
     }
 
@@ -344,11 +343,12 @@ export class TypeGraph {
         stringTypeMapping: StringTypeMapping,
         alphabetizeProperties: boolean,
         map: ReadonlyMap<Type, Type>,
-        debugPrintRemapping: boolean
+        debugPrintRemapping: boolean,
+        force: boolean = false
     ): TypeGraph {
         this.printRewrite(title);
 
-        if (map.size === 0) return this;
+        if (!force && map.size === 0) return this;
 
         const builder = new GraphRemapBuilder(
             this,
@@ -373,19 +373,36 @@ export class TypeGraph {
     }
 
     garbageCollect(alphabetizeProperties: boolean, debugPrintReconstitution: boolean): TypeGraph {
-        const newGraph = this.rewrite(
+        const newGraph = this.remap(
             "GC",
             NoStringTypeMapping,
             alphabetizeProperties,
-            [],
+            new Map(),
             debugPrintReconstitution,
-            (_t, _b) => mustNotBeCalled(),
             true
         );
-        // console.log(`GC: ${defined(newGraph._types).length} types`);
         return newGraph;
     }
 
+    rewriteFixedPoint(alphabetizeProperties: boolean, debugPrintReconstitution: boolean): TypeGraph {
+        let graph: TypeGraph = this;
+        for (;;) {
+            const newGraph = this.rewrite(
+                "fixed-point",
+                NoStringTypeMapping,
+                alphabetizeProperties,
+                [],
+                debugPrintReconstitution,
+                mustNotHappen,
+                true
+            );
+            if (graph.allTypesUnordered().size === newGraph.allTypesUnordered().size) {
+                return graph;
+            }
+            graph = newGraph;
+        }
+    }
+
     allTypesUnordered(): ReadonlySet<Type> {
         assert(this.isFrozen, "Tried to get all graph types before it was frozen");
         return new Set(defined(this._types));

src/quicktype-core/language/CSharp.ts

@@ -819,7 +819,7 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer {
 
     private emitDecoderTransformerCase(
         tokenCases: string[],
-        varName: string,
+        variableName: string,
         xfer: Transformer | undefined,
         targetType: Type
     ): void {
@@ -828,11 +828,8 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer {
         for (const tokenCase of tokenCases) {
             this.emitTokenCase(tokenCase);
         }
-
-        const deserialized = this.deserializeTypeCode(this.csType(xfer.sourceType, followTargetType));
         this.indent(() => {
-            this.emitLine("var ", varName, " = ", deserialized, ";");
-            const allHandled = this.emitTransformer(varName, xfer, targetType);
+            const allHandled = this.emitDecodeTransformer(xfer, targetType, variableName);
             if (!allHandled) {
                 this.emitLine("break;");
             }
@@ -848,17 +845,19 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer {
         }
     }
 
-    private emitDecodeTransformer(xfer: Transformer, targetType: Type): boolean {
+    private emitDecodeTransformer(xfer: Transformer, targetType: Type, variableName: string = "value"): boolean {
         if (xfer instanceof DecodingTransformer) {
-            this.emitLine("var value = ", this.deserializeTypeCode(this.csType(xfer.sourceType, noFollow)), ";");
-            return this.emitConsume("value", xfer.consumer, targetType);
+            if (xfer.sourceType.kind !== "null") {
+                this.emitLine("var ", variableName, " = ", this.deserializeTypeCode(this.csType(xfer.sourceType)), ";");
+            }
+            return this.emitConsume(variableName, xfer.consumer, targetType);
         } else if (xfer instanceof DecodingChoiceTransformer) {
             this.emitDecoderSwitch(() => {
                 const nullTransformer = xfer.nullTransformer;
                 if (nullTransformer !== undefined) {
                     this.emitTokenCase("Null");
                     this.indent(() => {
-                        const allHandled = this.emitTransformer("null", nullTransformer, targetType);
+                        const allHandled = this.emitDecodeTransformer(nullTransformer, targetType, "nullValue");
                         if (!allHandled) {
                             this.emitLine("break");
                         }

src/quicktype-core/support/Support.ts

@@ -72,10 +72,6 @@ export function panic(message: string): never {
     return messageError("InternalError", { message });
 }
 
-export function mustNotBeCalled(): never {
-    return panic("This must not be called");
-}
-
 export function mustNotHappen(): never {
     return panic("This must not happen");
 }

test/fixtures.ts

@@ -21,7 +21,7 @@ import {
 } from "./utils";
 import * as languages from "./languages";
 import { RendererOptions } from "../dist/quicktype-core/Run";
-import { mustNotBeCalled } from "../dist/quicktype-core/support/Support";
+import { mustNotHappen } from "../dist/quicktype-core/support/Support";
 import { isDateTime } from "../dist/quicktype-core/DateTime";
 
 const chalk = require("chalk");
@@ -284,7 +284,7 @@ class JSONToXToYFixture extends JSONFixture {
       name: languageXName,
       base: language.base,
       setupCommand: language.setupCommand,
-      runCommand: (_sample: string) => mustNotBeCalled(),
+      runCommand: mustNotHappen,
       diffViaSchema: false,
       skipDiffViaSchema: [],
       allowMissingNull: language.allowMissingNull,