Merge pull request #712 from quicktype/schema-fixes

Schema fixes
2018-03-29 11:14:00 -07:00 · 2018-03-29 11:14:00 -07:00 · 4effed9218
--- a/src/FlattenUnions.ts
+++ b/src/FlattenUnions.ts
@ -3,33 +3,11 @@
 import { Set, Map, OrderedSet } from "immutable";

 import { TypeGraph } from "./TypeGraph";
-import { Type, UnionType, IntersectionType } from "./Type";
-import { assert, defined } from "./Support";
+import { Type, UnionType, IntersectionType, makeGroupsToFlatten } from "./Type";
+import { assert } from "./Support";
 import { TypeRef, GraphRewriteBuilder, StringTypeMapping } from "./TypeBuilder";
 import { unifyTypes, UnifyUnionBuilder } from "./UnifyClasses";

-function unionMembersRecursively(...unions: UnionType[]): OrderedSet<Type> {
-    let processedUnions = Set<UnionType>();
-    let members = OrderedSet<Type>();
-
-    function addMembers(u: UnionType): void {
-        if (processedUnions.has(u)) return;
-        processedUnions = processedUnions.add(u);
-        u.members.forEach(t => {
-            if (t instanceof UnionType) {
-                addMembers(t);
-            } else {
-                members = members.add(t);
-            }
-        });
-    }
-
-    for (const union of unions) {
-        addMembers(union);
-    }
-    return members;
-}
-
 export function flattenUnions(
    graph: TypeGraph,
    stringTypeMapping: StringTypeMapping,
@ -53,29 +31,21 @@ export function flattenUnions(

    const allUnions = graph.allTypesUnordered().filter(t => t instanceof UnionType) as Set<UnionType>;
    const nonCanonicalUnions = allUnions.filter(u => !u.isCanonical);
-    let singleTypeGroups = Map<Type, OrderedSet<Type>>();
-    const groups: Type[][] = [];
-    let foundIntersection: boolean = false;
-    nonCanonicalUnions.forEach(u => {
-        const members = unionMembersRecursively(u);
+    let foundIntersection = false;
+    const groups = makeGroupsToFlatten(nonCanonicalUnions, members => {
        assert(!members.isEmpty(), "We can't have an empty union");
-        if (members.some(m => m instanceof IntersectionType)) {
-            foundIntersection = true;
-            return;
-        }
-        if (members.size === 1) {
-            const t = defined(members.first());
-            let maybeSet = singleTypeGroups.get(t);
-            if (maybeSet === undefined) {
-                maybeSet = OrderedSet([t]);
-            }
-            maybeSet = maybeSet.add(u);
-            singleTypeGroups = singleTypeGroups.set(t, maybeSet);
-        } else {
-            groups.push([u]);
-        }
+        if (!members.some(m => m instanceof IntersectionType)) return true;
+
+        // FIXME: This is stupid.  `flattenUnions` returns true when no more union
+        // flattening is necessary, but `resolveIntersections` can introduce new
+        // unions that might require flattening, so now `flattenUnions` needs to take
+        // that into account.  Either change `resolveIntersections` such that it
+        // doesn't introduce non-canonical unions (by using `unifyTypes`), or have
+        // some other way to tell whether more work is needed that doesn't require
+        // the two passes to know about each other.
+        foundIntersection = true;
+        return false;
    });
-    singleTypeGroups.forEach(ts => groups.push(ts.toArray()));
    graph = graph.rewrite("flatten unions", stringTypeMapping, false, groups, replace);

    // console.log(`flattened ${nonCanonicalUnions.size} of ${unions.size} unions`);
--- a/src/JSONSchemaInput.ts
+++ b/src/JSONSchemaInput.ts
@ -104,8 +104,11 @@ export class Ref {
        }

        const uri = new URI(ref);
-        const path = uri.fragment();
+        let path = uri.fragment();
        uri.fragment("");
+        if ((uri.host() !== "" || uri.filename() !== "") && path === "") {
+            path = "/";
+        }
        const elements = Ref.parsePath(path);
        return new Ref(uri, elements);
    }
--- a/src/ResolveIntersections.ts
+++ b/src/ResolveIntersections.ts
@ -30,7 +30,9 @@ import {
    GenericClassProperty,
    TypeKind,
    combineTypeAttributesOfTypes,
-    ObjectType
+    ObjectType,
+    setOperationMembersRecursively,
+    makeGroupsToFlatten
 } from "./Type";
 import { assert, defined, panic } from "./Support";
 import {
@ -40,25 +42,8 @@ import {
    makeTypeAttributesInferred
 } from "./TypeAttributes";

-function intersectionMembersRecursively(intersection: IntersectionType): [OrderedSet<Type>, TypeAttributes] {
-    const types: Type[] = [];
-    let attributes = emptyTypeAttributes;
-    function process(t: Type): void {
-        if (t instanceof IntersectionType) {
-            attributes = combineTypeAttributes(attributes, t.getAttributes());
-            t.members.forEach(process);
-        } else if (t.kind !== "any") {
-            types.push(t);
-        } else {
-            attributes = combineTypeAttributes(attributes, t.getAttributes());
-        }
-    }
-    process(intersection);
-    return [OrderedSet(types), attributes];
-}
-
 function canResolve(t: IntersectionType): boolean {
-    const members = intersectionMembersRecursively(t)[0];
+    const members = setOperationMembersRecursively(t)[0];
    if (members.size <= 1) return true;
    return members.every(m => !(m instanceof UnionType) || m.isCanonical);
 }
@ -236,7 +221,7 @@ class IntersectionAccumulator
                return panic("There shouldn't be a none type");
            },
            _anyType => {
-                return panic("The any type should have been filtered out in intersectionMembersRecursively");
+                return panic("The any type should have been filtered out in setOperationMembersRecursively");
            },
            nullType => this.addUnionSet(OrderedSet([nullType])),
            boolType => this.addUnionSet(OrderedSet([boolType])),
@ -410,13 +395,9 @@ class IntersectionUnionBuilder extends UnionBuilder<
 export function resolveIntersections(graph: TypeGraph, stringTypeMapping: StringTypeMapping): [TypeGraph, boolean] {
    let needsRepeat = false;

-    function replace(
-        types: Set<IntersectionType>,
-        builder: GraphRewriteBuilder<IntersectionType>,
-        forwardingRef: TypeRef
-    ): TypeRef {
-        assert(types.size === 1);
-        const [members, intersectionAttributes] = intersectionMembersRecursively(defined(types.first()));
+    function replace(types: Set<Type>, builder: GraphRewriteBuilder<Type>, forwardingRef: TypeRef): TypeRef {
+        const intersections = types.filter(t => t instanceof IntersectionType) as Set<IntersectionType>;
+        const [members, intersectionAttributes] = setOperationMembersRecursively(intersections.toArray());
        if (members.isEmpty()) {
            const t = builder.getPrimitiveType("any", forwardingRef);
            builder.addAttributes(t, intersectionAttributes);
@ -443,17 +424,13 @@ export function resolveIntersections(graph: TypeGraph, stringTypeMapping: String
    }
    // FIXME: We need to handle intersections that resolve to the same set of types.
    // See for example the intersections-nested.schema example.
-    const intersections = graph.allTypesUnordered().filter(t => t instanceof IntersectionType) as Set<IntersectionType>;
-    if (intersections.isEmpty()) {
-        return [graph, true];
-    }
-    const resolvableIntersections = intersections.filter(canResolve);
-    if (resolvableIntersections.isEmpty()) {
-        return [graph, false];
-    }
-    const groups = resolvableIntersections.map(i => [i]).toArray();
+    const allIntersections = graph.allTypesUnordered().filter(t => t instanceof IntersectionType) as Set<
+        IntersectionType
+    >;
+    const resolvableIntersections = allIntersections.filter(canResolve);
+    const groups = makeGroupsToFlatten(resolvableIntersections, undefined);
    graph = graph.rewrite("resolve intersections", stringTypeMapping, false, groups, replace);

    // console.log(`resolved ${resolvableIntersections.size} of ${intersections.size} intersections`);
-    return [graph, !needsRepeat && intersections.size === resolvableIntersections.size];
+    return [graph, !needsRepeat && allIntersections.size === resolvableIntersections.size];
 }
--- a/src/Type.ts
+++ b/src/Type.ts
@ -1,11 +1,11 @@
 "use strict";

-import { OrderedSet, OrderedMap, Collection, Set, is, hash } from "immutable";
+import { OrderedSet, OrderedMap, Collection, Map, Set, is, hash } from "immutable";

 import { defined, panic, assert, assertNever } from "./Support";
 import { TypeRef, TypeReconstituter } from "./TypeBuilder";
 import { TypeNames, namesTypeAttributeKind } from "./TypeNames";
-import { TypeAttributes, combineTypeAttributes } from "./TypeAttributes";
+import { TypeAttributes, combineTypeAttributes, emptyTypeAttributes } from "./TypeAttributes";

 export type PrimitiveStringTypeKind = "string" | "date" | "time" | "date-time";
 export type PrimitiveTypeKind = "none" | "any" | "null" | "bool" | "integer" | "double" | PrimitiveStringTypeKind;
@ -568,6 +568,71 @@ export class UnionType extends SetOperationType {
    }
 }

+export function setOperationMembersRecursively<T extends SetOperationType>(
+    setOperation: T
+): [OrderedSet<Type>, TypeAttributes];
+export function setOperationMembersRecursively<T extends SetOperationType>(
+    setOperations: T[]
+): [OrderedSet<Type>, TypeAttributes];
+export function setOperationMembersRecursively<T extends SetOperationType>(
+    oneOrMany: T | T[]
+): [OrderedSet<Type>, TypeAttributes] {
+    const setOperations = Array.isArray(oneOrMany) ? oneOrMany : [oneOrMany];
+    const kind = setOperations[0].kind;
+    const includeAny = kind !== "intersection";
+    let processedSetOperations = Set<T>();
+    let members = OrderedSet<Type>();
+    let attributes = emptyTypeAttributes;
+
+    function process(t: Type): void {
+        if (t.kind === kind) {
+            const so = t as T;
+            if (processedSetOperations.has(so)) return;
+            processedSetOperations = processedSetOperations.add(so);
+            attributes = combineTypeAttributes(attributes, t.getAttributes());
+            so.members.forEach(process);
+        } else if (includeAny || t.kind !== "any") {
+            members = members.add(t);
+        } else {
+            attributes = combineTypeAttributes(attributes, t.getAttributes());
+        }
+    }
+
+    for (const so of setOperations) {
+        process(so);
+    }
+    return [members, attributes];
+}
+
+export function makeGroupsToFlatten<T extends SetOperationType>(
+    setOperations: Set<T>,
+    include: ((members: Set<Type>) => boolean) | undefined
+): Type[][] {
+    let typeGroups = Map<Set<Type>, OrderedSet<Type>>();
+    setOperations.forEach(u => {
+        const members = setOperationMembersRecursively(u)[0].toSet();
+
+        if (include !== undefined) {
+            if (!include(members)) return;
+        }
+
+        let maybeSet = typeGroups.get(members);
+        if (maybeSet === undefined) {
+            maybeSet = OrderedSet();
+            if (members.size === 1) {
+                maybeSet = maybeSet.add(defined(members.first()));
+            }
+        }
+        maybeSet = maybeSet.add(u);
+        typeGroups = typeGroups.set(members, maybeSet);
+    });
+
+    return typeGroups
+        .valueSeq()
+        .toArray()
+        .map(ts => ts.toArray());
+}
+
 export function combineTypeAttributesOfTypes(types: Collection<any, Type>): TypeAttributes {
    return combineTypeAttributes(
        types
--- a/src/TypeAttributes.ts
+++ b/src/TypeAttributes.ts
@ -96,10 +96,15 @@ export function makeTypeAttributesInferred(attr: TypeAttributes): TypeAttributes
    return attr.map((value, kind) => kind.makeInferred(value)).filter(v => v !== undefined);
 }

-export const descriptionTypeAttributeKind = new TypeAttributeKind<OrderedSet<string>>("description", setUnion, a => a, undefined);
+export const descriptionTypeAttributeKind = new TypeAttributeKind<OrderedSet<string>>(
+    "description",
+    setUnion,
+    _ => OrderedSet(),
+    undefined
+);
 export const propertyDescriptionsTypeAttributeKind = new TypeAttributeKind<Map<string, OrderedSet<string>>>(
    "propertyDescriptions",
    (a, b) => a.mergeWith(setUnion, b),
-    a => a,
+    _ => Map(),
    undefined
 );