Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(encoding/toml): serializes mixed array #1001

Merged
merged 5 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 104 additions & 35 deletions encoding/toml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,39 +549,41 @@ function joinKeys(keys: string[]): string {
.join(".");
}

enum ArrayType {
ONLY_PRIMITIVE,
ONLY_OBJECT_EXCLUDING_ARRAY,
MIXED,
}

class Dumper {
maxPad = 0;
srcObject: Record<string, unknown>;
output: string[] = [];
_arrayTypeCache = new Map<unknown[], ArrayType>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use private field (prefixed with # instead of _)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 Do you think we should update methods starting with _ too? Maybe for the whole project?

Copy link
Member

@kt3k kt3k Jul 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use # for methods too (at least for Dumper class).

constructor(srcObjc: Record<string, unknown>) {
this.srcObject = srcObjc;
}
dump(): string[] {
// deno-lint-ignore no-explicit-any
this.output = this._parse(this.srcObject as any);
this.output = this._printObject(this.srcObject as any);
this.output = this._format();
return this.output;
}
_parse(obj: Record<string, unknown>, keys: string[] = []): string[] {
_printObject(obj: Record<string, unknown>, keys: string[] = []): string[] {
const out = [];
const props = Object.keys(obj);
const propObj = props.filter((e: string): boolean => {
if (obj[e] instanceof Array) {
const d: unknown[] = obj[e] as unknown[];
return !this._isSimplySerializable(d[0]);
}
return !this._isSimplySerializable(obj[e]);
});
const propPrim = props.filter((e: string): boolean => {
if (obj[e] instanceof Array) {
const d: unknown[] = obj[e] as unknown[];
return this._isSimplySerializable(d[0]);
const inlineProps = [];
const multilinePorps = [];
for (const prop of props) {
if (this._isSimplySerializable(obj[prop])) {
inlineProps.push(prop);
} else {
multilinePorps.push(prop);
}
return this._isSimplySerializable(obj[e]);
});
const k = propPrim.concat(propObj);
for (let i = 0; i < k.length; i++) {
const prop = k[i];
}
const sortedProps = inlineProps.concat(multilinePorps);
for (let i = 0; i < sortedProps.length; i++) {
const prop = sortedProps[i];
const value = obj[prop];
if (value instanceof Date) {
out.push(this._dateDeclaration([prop], value));
Expand All @@ -592,42 +594,106 @@ class Dumper {
} else if (typeof value === "boolean") {
out.push(this._boolDeclaration([prop], value));
} else if (
value instanceof Array &&
this._isSimplySerializable(value[0])
) {
// only if primitives types in the array
out.push(this._arrayDeclaration([prop], value));
} else if (
value instanceof Array &&
!this._isSimplySerializable(value[0])
value instanceof Array
) {
// array of objects
for (let i = 0; i < value.length; i++) {
out.push("");
out.push(this._headerGroup([...keys, prop]));
out.push(...this._parse(value[i], [...keys, prop]));
const arrayType = this._getTypeOfArray(value);
if (arrayType === ArrayType.ONLY_PRIMITIVE) {
out.push(this._arrayDeclaration([prop], value));
} else if (arrayType === ArrayType.ONLY_OBJECT_EXCLUDING_ARRAY) {
// array of objects
for (let i = 0; i < value.length; i++) {
out.push("");
out.push(this._headerGroup([...keys, prop]));
out.push(...this._printObject(value[i], [...keys, prop]));
}
} else {
// this is a complex array, use the inline format.
const str = value.map((x) => this._printAsInlineValue(x)).join(",");
out.push(`${prop} = [${str}]`);
}
} else if (typeof value === "object") {
out.push("");
out.push(this._header([...keys, prop]));
if (value) {
const toParse = value as Record<string, unknown>;
out.push(...this._parse(toParse, [...keys, prop]));
out.push(...this._printObject(toParse, [...keys, prop]));
}
// out.push(...this._parse(value, `${path}${prop}.`));
}
}
out.push("");
return out;
}
_isPrimitive(value: unknown): boolean {
return value instanceof Date ||
value instanceof RegExp ||
["string", "number", "boolean"].includes(typeof value);
}
_getTypeOfArray(arr: unknown[]): ArrayType {
if (this._arrayTypeCache.has(arr)) {
return this._arrayTypeCache.get(arr)!;
}
const type = this._doGetTypeOfArray(arr);
this._arrayTypeCache.set(arr, type);
return type;
}
_doGetTypeOfArray(arr: unknown[]): ArrayType {
if (!arr.length) {
// any type should be fine
return ArrayType.ONLY_PRIMITIVE;
}

const onlyPrimitive = this._isPrimitive(arr[0]);
if (arr[0] instanceof Array) {
return ArrayType.MIXED;
}
for (let i = 1; i < arr.length; i++) {
if (
onlyPrimitive !== this._isPrimitive(arr[i]) || arr[i] instanceof Array
) {
return ArrayType.MIXED;
}
}
return onlyPrimitive
? ArrayType.ONLY_PRIMITIVE
: ArrayType.ONLY_OBJECT_EXCLUDING_ARRAY;
}
_printAsInlineValue(value: unknown): string | number {
if (value instanceof Date) {
return `"${this._printDate(value)}"`;
} else if (typeof value === "string" || value instanceof RegExp) {
return JSON.stringify(value.toString());
} else if (typeof value === "number") {
return value;
} else if (typeof value === "boolean") {
return value.toString();
} else if (
value instanceof Array
) {
const str = value.map((x) => this._printAsInlineValue(x)).join(",");
return `[${str}]`;
} else if (typeof value === "object") {
if (!value) {
throw new Error("should never reach");
}
const str = Object.keys(value).map((key) => {
// deno-lint-ignore no-explicit-any
return `${key} = ${this._printAsInlineValue((value as any)[key])}`;
}).join(",");
return `{${str}}`;
}

throw new Error("should never reach");
}
_isSimplySerializable(value: unknown): boolean {
return (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean" ||
value instanceof RegExp ||
value instanceof Date ||
value instanceof Array
(value instanceof Array &&
this._getTypeOfArray(value) !== ArrayType.ONLY_OBJECT_EXCLUDING_ARRAY)
);
}
_header(keys: string[]): string {
Expand Down Expand Up @@ -662,7 +728,7 @@ class Dumper {
_boolDeclaration(keys: string[], value: boolean): string {
return `${this._declaration(keys)}${value}`;
}
_dateDeclaration(keys: string[], value: Date): string {
_printDate(value: Date): string {
function dtPad(v: string, lPad = 2): string {
return v.padStart(lPad, "0");
}
Expand All @@ -674,7 +740,10 @@ class Dumper {
const ms = dtPad(value.getUTCMilliseconds().toString(), 3);
// formatted date
const fData = `${value.getUTCFullYear()}-${m}-${d}T${h}:${min}:${s}.${ms}`;
return `${this._declaration(keys)}${fData}`;
return fData;
}
_dateDeclaration(keys: string[], value: Date): string {
return `${this._declaration(keys)}${this._printDate(value)}`;
}
_format(): string[] {
const rDeclaration = /(.*)\s=/;
Expand Down
27 changes: 27 additions & 0 deletions encoding/toml_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,33 @@ the = "array"
},
});

Deno.test({
name: "[TOML] Mixed Array",
fn(): void {
const src = {
mixedArray1: [1, { b: 2 }],
mixedArray2: [{ b: 2 }, 1],
nestedArray1: [[{ b: 1 }]],
nestedArray2: [[[{ b: 1 }]]],
deepNested: {
a: {
b: [1, { c: 2, d: [{ e: 3 }, true] }],
},
},
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure that this test exercises all variants from ArrayType enum

const expected = `mixedArray1 = [1,{b = 2}]
mixedArray2 = [{b = 2},1]
nestedArray1 = [[{b = 1}]]
nestedArray2 = [[[{b = 1}]]]

[deepNested.a]
b = [1,{c = 2,d = [{e = 3},true]}]
`;
const actual = stringify(src);
assertEquals(actual, expected);
},
});

Deno.test({
name: "[TOML] Comments",
fn: () => {
Expand Down