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
Properly serialize non-json values in parser tests #10858
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,39 +111,7 @@ export function runThrowTestsWithEstree(fixturesPath, parseFunction) { | |
} | ||
|
||
function save(test, ast) { | ||
overrideToJSON(() => | ||
fs.writeFileSync(test.expect.loc, JSON.stringify(ast, null, 2)), | ||
); | ||
} | ||
|
||
// Ensure that RegExp, BigInt, and Errors are serialized as strings | ||
function overrideToJSON(cb) { | ||
const originalToJSONMap = new Map(); | ||
const notJSONparseableObj = [RegExp, Error]; | ||
|
||
if (typeof BigInt !== "undefined") { | ||
notJSONparseableObj.push(BigInt); | ||
} | ||
|
||
for (const obj of notJSONparseableObj) { | ||
const { toJSON } = obj.prototype; | ||
originalToJSONMap.set(obj, toJSON); | ||
obj.prototype.toJSON = function() { | ||
if (typeof this === "bigint") { | ||
return { [serialized]: "BigInt", value: serialize(this) }; | ||
} | ||
|
||
return this.toString(); | ||
}; | ||
} | ||
|
||
const result = cb(); | ||
|
||
for (const obj of notJSONparseableObj) { | ||
obj.prototype.toJSON = originalToJSONMap.get(obj); | ||
} | ||
|
||
return result; | ||
fs.writeFileSync(test.expect.loc, JSON.stringify(ast, serialize, 2)); | ||
} | ||
|
||
function runTest(test, parseFunction) { | ||
|
@@ -221,22 +189,43 @@ function runTest(test, parseFunction) { | |
} | ||
} | ||
|
||
function serialize(value) { | ||
function serialize(key, value) { | ||
if (typeof value === "bigint") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it imply that the serialization result is dependent on node versions? Because I am surprised that they do not bail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the AST should contain a bigint, This is also true before this PR: if you delete |
||
return value.toString() + "n"; | ||
return { | ||
[serialized]: "BigInt", | ||
value: value.toString(), | ||
}; | ||
} else if (value instanceof RegExp) { | ||
return { | ||
[serialized]: "RegExp", | ||
source: value.source, | ||
flags: value.flags, | ||
}; | ||
} else if (value instanceof Error) { | ||
// Errors are serialized to a simple string, because are used frequently | ||
return value.toString(); | ||
} | ||
return JSON.stringify(value, null, 2); | ||
return value; | ||
} | ||
|
||
function ppJSON(v) { | ||
if (typeof v === "bigint" || v instanceof Error || v instanceof RegExp) { | ||
return ppJSON(serialize("", v)); | ||
} | ||
|
||
if (v && typeof v === "object" && v[serialized]) { | ||
switch (v[serialized]) { | ||
case "BigInt": | ||
return typeof BigInt === "undefined" ? "null" : v.value; | ||
return typeof BigInt === "undefined" ? "null" : v.value + "n"; | ||
case "RegExp": | ||
return `/${v.source}/${v.flags}`; | ||
} | ||
} else if (typeof v === "string" && /^[A-Z][a-z]+Error: /.test(v)) { | ||
// Errors are serialized to a simple string, because are used frequently | ||
return v; | ||
} | ||
|
||
return serialize(v); | ||
return JSON.stringify(v, serialize, 2); | ||
} | ||
|
||
function addPath(str, pt) { | ||
|
@@ -248,49 +237,42 @@ function addPath(str, pt) { | |
} | ||
|
||
function misMatch(exp, act) { | ||
return overrideToJSON(() => { | ||
if ( | ||
act instanceof RegExp || | ||
act instanceof Error || | ||
typeof act === "bigint" || | ||
(exp && typeof exp === "object" && exp[serialized]) | ||
) { | ||
const left = ppJSON(exp); | ||
const right = ppJSON(act); | ||
if (left !== right) return left + " !== " + right; | ||
} else if (Array.isArray(exp)) { | ||
if (!Array.isArray(act)) return ppJSON(exp) + " != " + ppJSON(act); | ||
if (act.length != exp.length) { | ||
return "array length mismatch " + exp.length + " != " + act.length; | ||
} | ||
for (let i = 0; i < act.length; ++i) { | ||
const mis = misMatch(exp[i], act[i]); | ||
if (mis) return addPath(mis, i); | ||
} | ||
} else if ( | ||
!exp || | ||
!act || | ||
typeof exp != "object" || | ||
typeof act != "object" | ||
) { | ||
if (exp !== act && typeof exp != "function") { | ||
return ppJSON(exp) + " !== " + ppJSON(act); | ||
} | ||
} else { | ||
for (const prop of Object.keys(exp)) { | ||
const mis = misMatch(exp[prop], act[prop]); | ||
if (mis) return addPath(mis, prop); | ||
} | ||
if ( | ||
act instanceof RegExp || | ||
act instanceof Error || | ||
typeof act === "bigint" || | ||
(exp && typeof exp === "object" && exp[serialized]) | ||
) { | ||
const left = ppJSON(exp); | ||
const right = ppJSON(act); | ||
if (left !== right) return left + " !== " + right; | ||
} else if (Array.isArray(exp)) { | ||
if (!Array.isArray(act)) return ppJSON(exp) + " != " + ppJSON(act); | ||
if (act.length != exp.length) { | ||
return "array length mismatch " + exp.length + " != " + act.length; | ||
} | ||
for (let i = 0; i < act.length; ++i) { | ||
const mis = misMatch(exp[i], act[i]); | ||
if (mis) return addPath(mis, i); | ||
} | ||
} else if (!exp || !act || typeof exp != "object" || typeof act != "object") { | ||
if (exp !== act && typeof exp != "function") { | ||
return ppJSON(exp) + " !== " + ppJSON(act); | ||
} | ||
} else { | ||
for (const prop of Object.keys(exp)) { | ||
const mis = misMatch(exp[prop], act[prop]); | ||
if (mis) return addPath(mis, prop); | ||
} | ||
|
||
for (const prop of Object.keys(act)) { | ||
if (typeof act[prop] === "function") { | ||
continue; | ||
} | ||
for (const prop of Object.keys(act)) { | ||
if (typeof act[prop] === "function") { | ||
continue; | ||
} | ||
|
||
if (!(prop in exp) && act[prop] !== undefined) { | ||
return `Did not expect a property '${prop}'`; | ||
} | ||
if (!(prop in exp) && act[prop] !== undefined) { | ||
return `Did not expect a property '${prop}'`; | ||
} | ||
} | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
consider explicitly specify the ReplaceFunction: