Always use strings first, convert to enums at the end

This commit is contained in:
Mark Probst 2018-04-26 22:21:06 -07:00
Родитель 066fe73a8d
Коммит 20c9c56da8
10 изменённых файлов: 104 добавлений и 127 удалений

Просмотреть файл

@ -157,7 +157,7 @@ export function combineClasses(
clique,
attributes,
builder,
unionBuilderForUnification(builder, false, false, false, conflateNumbers),
unionBuilderForUnification(builder, false, false, conflateNumbers),
conflateNumbers,
forwardingRef
);

Просмотреть файл

@ -22,7 +22,7 @@ export function flattenUnions(
let needsRepeat = false;
function replace(types: Set<Type>, builder: GraphRewriteBuilder<Type>, forwardingRef: TypeRef): TypeRef {
const unionBuilder = new UnifyUnionBuilder(builder, true, makeObjectTypes, true, trefs => {
const unionBuilder = new UnifyUnionBuilder(builder, makeObjectTypes, true, trefs => {
assert(trefs.length > 0, "Must have at least one type to build union");
trefs = trefs.map(tref => builder.reconstituteType(tref.deref()[0]));
if (trefs.length === 1) {

Просмотреть файл

@ -13,32 +13,11 @@ import { stringTypesTypeAttributeKind, StringTypes } from "./StringTypes";
const MIN_LENGTH_FOR_ENUM = 10;
function shouldBeEnum(t: PrimitiveType): OrderedMap<string, number> | undefined {
const enumCases = stringEnumCases(t);
if (enumCases !== undefined) {
assert(enumCases.size > 0, "How did we end up with zero enum cases?");
const someCaseIsNotNumber = enumCases.keySeq().some(key => /^(\-|\+)?[0-9]+(\.[0-9]+)?$/.test(key) === false);
const numValues = enumCases.map(n => n).reduce<number>((a, b) => a + b);
if (numValues >= MIN_LENGTH_FOR_ENUM && enumCases.size < Math.sqrt(numValues) && someCaseIsNotNumber) {
return enumCases;
}
}
return undefined;
}
function replaceString(
group: Set<PrimitiveType>,
builder: GraphRewriteBuilder<PrimitiveType>,
forwardingRef: TypeRef
): TypeRef {
assert(group.size === 1);
const t = defined(group.first());
const attributes = t.getAttributes().filterNot((_, k) => k === stringTypesTypeAttributeKind);
const maybeEnumCases = shouldBeEnum(t);
if (maybeEnumCases !== undefined) {
return builder.getEnumType(attributes, maybeEnumCases.keySeq().toOrderedSet(), forwardingRef);
}
return builder.getStringType(attributes, StringTypes.unrestricted, forwardingRef);
function shouldBeEnum(enumCases: OrderedMap<string, number>): boolean {
assert(enumCases.size > 0, "How did we end up with zero enum cases?");
const someCaseIsNotNumber = enumCases.keySeq().some(key => /^(\-|\+)?[0-9]+(\.[0-9]+)?$/.test(key) === false);
const numValues = enumCases.map(n => n).reduce<number>((a, b) => a + b);
return numValues >= MIN_LENGTH_FOR_ENUM && enumCases.size < Math.sqrt(numValues) && someCaseIsNotNumber;
}
// A union needs replacing if it contains more than one string type, one of them being
@ -75,11 +54,27 @@ function replaceUnion(group: Set<UnionType>, builder: GraphRewriteBuilder<UnionT
export function inferEnums(
graph: TypeGraph,
stringTypeMapping: StringTypeMapping,
makeAllEnums: boolean,
debugPrintReconstitution: boolean
): TypeGraph {
function replaceString(
group: Set<PrimitiveType>,
builder: GraphRewriteBuilder<PrimitiveType>,
forwardingRef: TypeRef
): TypeRef {
assert(group.size === 1);
const t = defined(group.first());
const attributes = t.getAttributes().filterNot((_, k) => k === stringTypesTypeAttributeKind);
const enumCases = defined(stringEnumCases(t));
if (makeAllEnums || shouldBeEnum(enumCases)) {
return builder.getEnumType(attributes, enumCases.keySeq().toOrderedSet(), forwardingRef);
}
return builder.getStringType(attributes, StringTypes.unrestricted, forwardingRef);
}
const allStrings = graph
.allTypesUnordered()
.filter(t => t.kind === "string")
.filter(t => t.kind === "string" && stringEnumCases(t as PrimitiveType) !== undefined)
.map(t => [t])
.toArray() as PrimitiveType[][];
return graph.rewrite("infer enums", stringTypeMapping, false, allStrings, debugPrintReconstitution, replaceString);

Просмотреть файл

@ -127,7 +127,7 @@ export function inferMaps(
shouldBe,
c.getAttributes(),
builder,
unionBuilderForUnification(builder, true, false, false, conflateNumbers),
unionBuilderForUnification(builder, false, false, conflateNumbers),
conflateNumbers
),
forwardingRef

Просмотреть файл

@ -39,7 +39,7 @@ class InferenceUnionBuilder extends UnionBuilder<TypeBuilder, NestedValueArray,
private readonly _cjson: CompressedJSON,
private readonly _fixed: boolean
) {
super(typeBuilder, false);
super(typeBuilder);
}
protected makeObject(
@ -102,7 +102,7 @@ export class TypeInference {
if (this._inferEnums) {
const s = cjson.getStringForValue(value);
if (canBeEnumCase(s)) {
accumulator.addEnumCase(s, 1, emptyTypeAttributes);
accumulator.addStringCase(s, 1, emptyTypeAttributes);
} else {
accumulator.addStringType("string", emptyTypeAttributes);
}

Просмотреть файл

@ -35,7 +35,7 @@ import {
emptyTypeAttributes,
makeTypeAttributesInferred
} from "./TypeAttributes";
import { MutableStringTypes, StringTypes } from "./StringTypes";
import { MutableStringTypes } from "./StringTypes";
function canResolve(t: IntersectionType): boolean {
const members = setOperationMembersRecursively(t)[0];
@ -266,8 +266,8 @@ class IntersectionAccumulator
return [this._objectProperties, this._additionalPropertyTypes];
}
get stringTypes(): StringTypes {
return this._stringTypes.toImmutable();
get enumCases(): OrderedSet<string> {
return defined(this._stringTypes.enumCases);
}
getMemberKinds(): TypeAttributeMap<TypeKind> {
@ -407,7 +407,7 @@ export function resolveIntersections(
);
const attributes = combineTypeAttributes(intersectionAttributes, extraAttributes);
const unionBuilder = new IntersectionUnionBuilder(builder, true);
const unionBuilder = new IntersectionUnionBuilder(builder);
const tref = unionBuilder.buildUnion(accumulator, true, attributes, forwardingRef);
if (unionBuilder.createdNewIntersections) {
needsRepeat = true;

Просмотреть файл

@ -63,6 +63,8 @@ export class StringTypes {
}
}
// FIXME: Why do we have this, just for intersections? Either use it for unions, too,
// or get rid of it.
export class MutableStringTypes {
static get unrestricted(): MutableStringTypes {
return new MutableStringTypes({}, undefined);
@ -121,6 +123,11 @@ export class MutableStringTypes {
this._enumCases = newEnumCases;
}
get enumCases(): OrderedSet<string> | undefined {
if (this._enumCases === undefined) return undefined;
return OrderedSet(this._enumCases);
}
toImmutable(): StringTypes {
if (this._enumCases === undefined) {
return new StringTypes(undefined);

Просмотреть файл

@ -90,12 +90,11 @@ function countProperties(
export class UnifyUnionBuilder extends UnionBuilder<TypeBuilder & TypeLookerUp, TypeRef[], TypeRef[]> {
constructor(
typeBuilder: TypeBuilder & TypeLookerUp,
makeEnums: boolean,
private readonly _makeObjectTypes: boolean,
private readonly _makeClassesFixed: boolean,
private readonly _unifyTypes: (typesToUnify: TypeRef[]) => TypeRef
) {
super(typeBuilder, makeEnums);
super(typeBuilder);
}
protected makeObject(
@ -174,17 +173,16 @@ export class UnifyUnionBuilder extends UnionBuilder<TypeBuilder & TypeLookerUp,
export function unionBuilderForUnification<T extends Type>(
typeBuilder: GraphRewriteBuilder<T>,
makeEnums: boolean,
makeObjectTypes: boolean,
makeClassesFixed: boolean,
conflateNumbers: boolean
): UnionBuilder<TypeBuilder & TypeLookerUp, TypeRef[], TypeRef[]> {
return new UnifyUnionBuilder(typeBuilder, makeEnums, makeObjectTypes, makeClassesFixed, trefs =>
return new UnifyUnionBuilder(typeBuilder, makeObjectTypes, makeClassesFixed, trefs =>
unifyTypes(
Set(trefs.map(tref => tref.deref()[0])),
emptyTypeAttributes,
typeBuilder,
unionBuilderForUnification(typeBuilder, makeEnums, makeObjectTypes, makeClassesFixed, conflateNumbers),
unionBuilderForUnification(typeBuilder, makeObjectTypes, makeClassesFixed, conflateNumbers),
conflateNumbers
)
);

Просмотреть файл

@ -24,6 +24,8 @@ export interface UnionTypeProvider<TArrayData, TObjectData> {
readonly arrayData: TArrayData;
readonly objectData: TObjectData;
readonly enumCases: OrderedSet<string>;
getMemberKinds(): TypeAttributeMap<TypeKind>;
readonly lostTypeAttributes: boolean;
@ -60,36 +62,18 @@ export class UnionAccumulator<TArray, TObject> implements UnionTypeProvider<TArr
readonly arrayData: TArray[] = [];
readonly objectData: TObject[] = [];
private _enumCases: OrderedSet<string> = OrderedSet();
private _lostTypeAttributes: boolean = false;
constructor(private readonly _conflateNumbers: boolean) {}
private have(kind: TypeKind): boolean {
if (kind === "enum" || kind === "string") {
const maybeStringTypes = this.tryGetStringTypes();
if (maybeStringTypes === undefined) return false;
const isRestricted = maybeStringTypes[1].isRestricted;
if (kind === "enum") {
return isRestricted;
} else {
return !isRestricted;
}
}
return (
this._nonStringTypeAttributes.has(kind) || this._stringTypeAttributes.has(kind as PrimitiveStringTypeKind)
);
}
private tryGetStringTypes(): [TypeAttributes, StringTypes] | undefined {
const attributes = this._stringTypeAttributes.get("string");
if (attributes === undefined) return undefined;
const stringTypes = stringTypesTypeAttributeKind.tryGetInAttributes(attributes);
if (stringTypes === undefined) {
return panic("Must have string types attribute when we have a string in the union builder");
}
return [attributes, stringTypes];
}
addNone(_attributes: TypeAttributes): void {
// FIXME: Add them to all members? Or add them to the union, which means we'd have
// to change getMemberKinds() to also return the attributes for the union itself,
@ -113,21 +97,45 @@ export class UnionAccumulator<TArray, TObject> implements UnionTypeProvider<TArr
this._nonStringTypeAttributes = setAttributes(this._nonStringTypeAttributes, "double", attributes);
}
addStringType(kind: PrimitiveStringTypeKind, attributes: TypeAttributes): void {
const maybeStringTypes = stringTypesTypeAttributeKind.tryGetInAttributes(attributes);
if (this.have(kind) || kind !== "string" || (maybeStringTypes !== undefined && maybeStringTypes.isRestricted)) {
this._stringTypeAttributes = setAttributes(this._stringTypeAttributes, kind, attributes);
return;
private addFullStringType(attributes: TypeAttributes, stringTypes: StringTypes | undefined): void {
if (stringTypes === undefined) {
stringTypes = stringTypesTypeAttributeKind.tryGetInAttributes(attributes);
} else {
attributes = stringTypesTypeAttributeKind.setInAttributes(attributes, stringTypes);
}
if (stringTypes === undefined) {
stringTypes = StringTypes.unrestricted;
attributes = stringTypesTypeAttributeKind.setInAttributes(attributes, stringTypes);
}
// unrestricted string overrides all other string types, as well as enum
let newAttributes = combineTypeAttributes(this._stringTypeAttributes.valueSeq().toArray());
newAttributes = combineTypeAttributes(newAttributes, attributes);
if (maybeStringTypes === undefined) {
newAttributes = stringTypesTypeAttributeKind.setInAttributes(newAttributes, StringTypes.unrestricted);
const maybeEnumAttributes = this._nonStringTypeAttributes.get("enum");
if (stringTypes.isRestricted) {
assert(
maybeEnumAttributes === undefined,
"We can't add both an enum as well as a restricted string type to a union builder"
);
} else {
// unrestricted string overrides all other string types, as well as enum
let oldAttributes = combineTypeAttributes(this._stringTypeAttributes.valueSeq().toArray());
if (maybeEnumAttributes !== undefined) {
oldAttributes = combineTypeAttributes(oldAttributes, maybeEnumAttributes);
this._nonStringTypeAttributes = this._nonStringTypeAttributes.remove("enum");
}
attributes = combineTypeAttributes(oldAttributes, attributes);
this._stringTypeAttributes = this._stringTypeAttributes.clear();
}
this._stringTypeAttributes = this._stringTypeAttributes.clear().set(kind, newAttributes);
this._stringTypeAttributes = setAttributes(this._stringTypeAttributes, "string", attributes);
}
addStringType(kind: PrimitiveStringTypeKind, attributes: TypeAttributes): void {
if (kind === "string") {
this.addFullStringType(attributes, undefined);
return;
}
this._stringTypeAttributes = setAttributes(this._stringTypeAttributes, kind, attributes);
}
addArray(t: TArray, attributes: TypeAttributes): void {
this.arrayData.push(t);
this._nonStringTypeAttributes = setAttributes(this._nonStringTypeAttributes, "array", attributes);
@ -137,32 +145,32 @@ export class UnionAccumulator<TArray, TObject> implements UnionTypeProvider<TArr
this._nonStringTypeAttributes = setAttributes(this._nonStringTypeAttributes, "object", attributes);
}
private addStringOrEnumCases(attributes: TypeAttributes, stringTypes: StringTypes): void {
const enumAttributes = stringTypesTypeAttributeKind.makeAttributes(stringTypes);
this.addStringType("string", combineTypeAttributes(attributes, enumAttributes));
addEnum(cases: OrderedSet<string>, attributes: TypeAttributes): void {
const maybeStringAttributes = this._stringTypeAttributes.get("string");
if (maybeStringAttributes !== undefined) {
// FIXME: Assert that the string types are unrestricted
this._stringTypeAttributes = setAttributes(this._stringTypeAttributes, "string", attributes);
return;
}
this._nonStringTypeAttributes = setAttributes(this._nonStringTypeAttributes, "enum", attributes);
this._enumCases = this._enumCases.union(cases);
}
addEnumCases(cases: string[], attributes: TypeAttributes): void {
this.addStringOrEnumCases(attributes, StringTypes.fromCases(cases));
addStringCases(cases: string[], attributes: TypeAttributes): void {
this.addFullStringType(attributes, StringTypes.fromCases(cases));
}
addEnumCase(s: string, count: number, attributes: TypeAttributes): void {
this.addStringOrEnumCases(attributes, StringTypes.fromCase(s, count));
addStringCase(s: string, count: number, attributes: TypeAttributes): void {
this.addFullStringType(attributes, StringTypes.fromCase(s, count));
}
get enumCases(): OrderedSet<string> {
return this._enumCases;
}
getMemberKinds(): TypeAttributeMap<TypeKind> {
let merged = this._nonStringTypeAttributes;
assert(!(this.have("enum") && this.have("string")), "We can't have both strings and enums in the same union");
const maybeStringTypes = this.tryGetStringTypes();
if (maybeStringTypes !== undefined) {
const [attributesForString, stringTypes] = maybeStringTypes;
if (stringTypes.isRestricted) {
merged = merged.set("enum", attributesForString);
} else {
merged = merged.set("string", attributesForString);
}
}
merged = merged.merge(this._stringTypeAttributes.filterNot((_, k) => k === "string"));
let merged = this._nonStringTypeAttributes.merge(this._stringTypeAttributes);
if (merged.isEmpty()) {
return OrderedMap([["none", Map()] as [TypeKind, TypeAttributes]]);
@ -262,7 +270,7 @@ export class TypeRefUnionAccumulator extends UnionAccumulator<TypeRef, TypeRef>
// FIXME: We're not carrying counts, so this is not correct if we do enum
// inference. JSON Schema input uses this case, however, without enum
// inference, which is fine, but still a bit ugly.
enumType => this.addEnumCases(enumType.cases.toArray(), attributes),
enumType => this.addEnum(enumType.cases, attributes),
_unionType => {
return panic("The unions should have been eliminated in attributesForTypesInUnion");
},
@ -280,25 +288,7 @@ export class TypeRefUnionAccumulator extends UnionAccumulator<TypeRef, TypeRef>
}
export abstract class UnionBuilder<TBuilder extends TypeBuilder, TArrayData, TObjectData> {
constructor(protected readonly typeBuilder: TBuilder, private readonly _makeEnums: boolean) {}
private makeEnum(
stringTypes: StringTypes,
typeAttributes: TypeAttributes,
forwardingRef: TypeRef | undefined
): TypeRef {
if (this._makeEnums) {
return this.typeBuilder.getEnumType(
typeAttributes,
defined(stringTypes.cases)
.keySeq()
.toOrderedSet(),
forwardingRef
);
} else {
return this.typeBuilder.getStringType(typeAttributes, stringTypes, forwardingRef);
}
}
constructor(protected readonly typeBuilder: TBuilder) {}
protected abstract makeObject(
objects: TObjectData,
@ -330,19 +320,8 @@ export abstract class UnionBuilder<TBuilder extends TypeBuilder, TArrayData, TOb
return this.typeBuilder.getPrimitiveType(kind, typeAttributes, forwardingRef);
case "string":
return this.typeBuilder.getStringType(typeAttributes, undefined, forwardingRef);
case "enum": {
// FIXME: It's super ugly that we have to separate the string types
// attribute from the rest.
const stringTypes = stringTypesTypeAttributeKind.tryGetInAttributes(typeAttributes);
if (stringTypes === undefined) {
return panic("Must have string types attribute when we have a string in the type provider");
}
return this.makeEnum(
stringTypes,
typeAttributes.filterNot((_, ta) => ta === stringTypesTypeAttributeKind),
forwardingRef
);
}
case "enum":
return this.typeBuilder.getEnumType(typeAttributes, typeProvider.enumCases, forwardingRef);
case "object":
return this.makeObject(typeProvider.objectData, typeAttributes, forwardingRef);
case "array":

Просмотреть файл

@ -238,9 +238,7 @@ export class Run {
);
}
}
if (doInferEnums) {
graph = inferEnums(graph, stringTypeMapping, debugPrintReconstitution);
}
graph = inferEnums(graph, stringTypeMapping, !doInferEnums, debugPrintReconstitution);
graph = flattenStrings(graph, stringTypeMapping, debugPrintReconstitution);
if (this._options.inferMaps) {
for (;;) {