Merge pull request #814 from quicktype/combine-numbers

Combine numbers

src/CombineClasses.ts

@@ -2,7 +2,7 @@
 
 import { Map, Set, OrderedSet } from "immutable";
 
-import { ClassType, Type, ClassProperty } from "./Type";
+import { ClassType, Type, ClassProperty, setOperationCasesEqual } from "./Type";
 import { nonNullTypeCases, combineTypeAttributesOfTypes } from "./TypeUtils";
 
 import { TypeRef, StringTypeMapping } from "./TypeBuilder";
@@ -22,15 +22,7 @@ type Clique = {
 // enums with strings, integers with doubles, maps with objects of
 // the correct type.
 function typeSetsCanBeCombined(s1: OrderedSet<Type>, s2: OrderedSet<Type>): boolean {
-    if (s1.size !== s2.size) return false;
-
-    const s2ByKind = Map(s2.map((t): [string, Type] => [t.kind, t]));
-    return s1.every(t => {
-        const kind = t.kind;
-        const other = s2ByKind.get(kind);
-        if (other === undefined) return false;
-        return t.structurallyCompatible(other);
-    });
+    return setOperationCasesEqual(s1, s2, true, (a, b) => a.structurallyCompatible(b, true));
 }
 
 function canBeCombined(c1: ClassType, c2: ClassType, onlyWithSameProperties: boolean): boolean {

src/InferMaps.ts

@@ -72,7 +72,7 @@ function shouldBeMap(properties: Map<string, ClassProperty>): Set<Type> | undefi
             if (firstNonNullCases !== undefined) {
                 // The set of non-null cases for all other properties must
                 // be the the same, otherwise we won't infer a map.
-                if (!setOperationCasesEqual(nn, firstNonNullCases, (a, b) => a.structurallyCompatible(b))) {
+                if (!setOperationCasesEqual(nn, firstNonNullCases, true, (a, b) => a.structurallyCompatible(b, true))) {
                     canBeMap = false;
                     return false;
                 }

src/Type.ts

@@ -89,11 +89,23 @@ export abstract class Type {
 
     // This will only ever be called when `this` and `other` are not
     // equal, but `this.kind === other.kind`.
-    protected abstract structuralEqualityStep(other: Type, queue: (a: Type, b: Type) => boolean): boolean;
+    protected abstract structuralEqualityStep(
+        other: Type,
+        conflateNumbers: boolean,
+        queue: (a: Type, b: Type) => boolean
+    ): boolean;
+
+    structurallyCompatible(other: Type, conflateNumbers: boolean = false): boolean {
+        function kindsCompatible(kind1: TypeKind, kind2: TypeKind): boolean {
+            if (kind1 === kind2) return true;
+            if (!conflateNumbers) return false;
+            if (kind1 === "integer") return kind2 === "double";
+            if (kind1 === "double") return kind2 === "integer";
+            return false;
+        }
 
-    structurallyCompatible(other: Type): boolean {
         if (triviallyStructurallyCompatible(this, other)) return true;
-        if (this.kind !== other.kind) return false;
+        if (!kindsCompatible(this.kind, other.kind)) return false;
 
         const workList: [Type, Type][] = [[this, other]];
         // This contains a set of pairs which are the type pairs
@@ -104,7 +116,7 @@ export abstract class Type {
         let failed: boolean;
         const queue = (x: Type, y: Type): boolean => {
             if (triviallyStructurallyCompatible(x, y)) return true;
-            if (x.kind !== y.kind) {
+            if (!kindsCompatible(x.kind, y.kind)) {
                 failed = true;
                 return false;
             }
@@ -134,7 +146,7 @@ export abstract class Type {
             }
 
             failed = false;
-            if (!a.structuralEqualityStep(b, queue)) return false;
+            if (!a.structuralEqualityStep(b, conflateNumbers, queue)) return false;
             if (failed) return false;
         }
 
@@ -198,7 +210,11 @@ export class PrimitiveType extends Type {
         builder.getPrimitiveType(this.kind);
     }
 
-    protected structuralEqualityStep(_other: Type, _queue: (a: Type, b: Type) => boolean): boolean {
+    protected structuralEqualityStep(
+        _other: Type,
+        _conflateNumbers: boolean,
+        _queue: (a: Type, b: Type) => boolean
+    ): boolean {
         return true;
     }
 }
@@ -212,7 +228,11 @@ export class StringType extends PrimitiveType {
         builder.getStringType(this.enumCases);
     }
 
-    protected structuralEqualityStep(_other: Type, _queue: (a: Type, b: Type) => boolean): boolean {
+    protected structuralEqualityStep(
+        _other: Type,
+        _conflateNumbers: boolean,
+        _queue: (a: Type, b: Type) => boolean
+    ): boolean {
         return true;
     }
 
@@ -273,7 +293,11 @@ export class ArrayType extends Type {
         }
     }
 
-    protected structuralEqualityStep(other: ArrayType, queue: (a: Type, b: Type) => boolean): boolean {
+    protected structuralEqualityStep(
+        other: ArrayType,
+        _conflateNumbers: boolean,
+        queue: (a: Type, b: Type) => boolean
+    ): boolean {
         return queue(this.items, other.items);
     }
 }
@@ -435,7 +459,11 @@ export class ObjectType extends Type {
         }
     }
 
-    protected structuralEqualityStep(other: ObjectType, queue: (a: Type, b: Type) => boolean): boolean {
+    protected structuralEqualityStep(
+        other: ObjectType,
+        _conflateNumbers: boolean,
+        queue: (a: Type, b: Type) => boolean
+    ): boolean {
         const pa = this.getProperties();
         const pb = other.getProperties();
         if (pa.size !== pb.size) return false;
@@ -504,7 +532,11 @@ export class EnumType extends Type {
         builder.getEnumType(this.cases);
     }
 
-    protected structuralEqualityStep(other: EnumType, _queue: (a: Type, b: Type) => void): boolean {
+    protected structuralEqualityStep(
+        other: EnumType,
+        _conflateNumbers: boolean,
+        _queue: (a: Type, b: Type) => void
+    ): boolean {
         return this.cases.toSet().equals(other.cases.toSet());
     }
 }
@@ -512,18 +544,21 @@ export class EnumType extends Type {
 export function setOperationCasesEqual(
     ma: OrderedSet<Type>,
     mb: OrderedSet<Type>,
+    conflateNumbers: boolean,
     membersEqual: (a: Type, b: Type) => boolean
 ): boolean {
     if (ma.size !== mb.size) return false;
-    let failed = false;
-    ma.forEach(ta => {
+    return ma.every(ta => {
         const tb = mb.find(t => t.kind === ta.kind);
-        if (tb === undefined || !membersEqual(ta, tb)) {
-            failed = true;
-            return false;
+        if (tb !== undefined) {
+            if (membersEqual(ta, tb)) return true;
         }
+        if (conflateNumbers) {
+            if (ta.kind === "integer" && mb.some(t => t.kind === "double")) return true;
+            if (ta.kind === "double" && mb.some(t => t.kind === "integer")) return true;
+        }
+        return false;
     });
-    return !failed;
 }
 
 export abstract class SetOperationType extends Type {
@@ -562,8 +597,12 @@ export abstract class SetOperationType extends Type {
         return false;
     }
 
-    protected structuralEqualityStep(other: SetOperationType, queue: (a: Type, b: Type) => boolean): boolean {
-        return setOperationCasesEqual(this.members, other.members, queue);
+    protected structuralEqualityStep(
+        other: SetOperationType,
+        conflateNumbers: boolean,
+        queue: (a: Type, b: Type) => boolean
+    ): boolean {
+        return setOperationCasesEqual(this.members, other.members, conflateNumbers, queue);
     }
 }
 

src/index.ts

@@ -221,7 +221,7 @@ export class Run {
                 graph,
                 stringTypeMapping,
                 this._options.alphabetizeProperties,
-                conflateNumbers,
+                true,
                 false,
                 debugPrintReconstitution
             );
@@ -232,7 +232,7 @@ export class Run {
                     combinedGraph,
                     stringTypeMapping,
                     this._options.alphabetizeProperties,
-                    conflateNumbers,
+                    false,
                     true,
                     debugPrintReconstitution
                 );
@@ -243,7 +243,7 @@ export class Run {
         }
         graph = flattenStrings(graph, stringTypeMapping, debugPrintReconstitution);
         if (this._options.inferMaps) {
-            graph = inferMaps(graph, stringTypeMapping, conflateNumbers, debugPrintReconstitution);
+            graph = inferMaps(graph, stringTypeMapping, true, debugPrintReconstitution);
         }
         graph = noneToAny(graph, stringTypeMapping, debugPrintReconstitution);
         if (!targetLanguage.supportsOptionalClassProperties) {

test/inputs/json/priority/number-map.json

@@ -0,0 +1,7 @@
+{
+    "foo": {
+        "^$#$%#@": 123,
+        "$%@##%": 123.3,
+        "$%#$%%#$%": 123
+    }
+}