From 6956816dff19d52023b1320b265b3438af5d3aae Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 25 Jun 2021 11:12:21 -0700 Subject: [PATCH 1/6] repl: ensure correct syntax error for await parsing --- lib/internal/repl/await.js | 34 ++++++- lib/repl.js | 91 ++++++++++--------- .../test-repl-preprocess-top-level-await.js | 40 ++++---- test/parallel/test-repl-top-level-await.js | 10 ++ 4 files changed, 110 insertions(+), 65 deletions(-) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index fdf6366860969e..4ae0644adf5144 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -8,6 +8,7 @@ const { ArrayPrototypePush, FunctionPrototype, ObjectKeys, + SyntaxError, } = primordials; const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; @@ -80,13 +81,40 @@ for (const nodeType of ObjectKeys(walk.base)) { } function processTopLevelAwait(src) { - const wrapped = `(async () => { ${src} })()`; + const wrapPrefix = '(async () => {\n'; + const wrapped = `${wrapPrefix}${src}\n})()`; const wrappedArray = ArrayFrom(wrapped); let root; try { root = parser.parse(wrapped, { ecmaVersion: 'latest' }); - } catch { - return null; + } catch (e) { + // If the parse error is before the first "await", then use the execution + // error. Otherwise we must emit this parse error, making it look like a + // proper syntax error. + const awaitPos = src.indexOf('await'); + const errPos = e.pos - wrapPrefix.length; + if (awaitPos > errPos) + return null; + // Convert keyword parse errors on await into their original errors when + // possible. + if (errPos === awaitPos + 6 && + src.slice(errPos - 6, errPos - 1) === 'await' && + e.message.includes('Expecting Unicode escape sequence')) + return null; + if (errPos === awaitPos + 7 && + src.slice(errPos - 7, errPos - 2) === 'await' && + e.message.includes('Unexpected token')) + return null; + const { line, column } = e.loc; + let message = '\n' + src.split('\n')[line - 2] + '\n'; + let i = 0; + while (i++ < column) message += ' '; + message += '^\n\n' + e.message.replace(/ \([^)]+\)/, ''); + // V8 unexpected token errors include the token string. + if (message.endsWith('Unexpected token')) + message += " '" + src[e.pos - wrapPrefix.length] + "'"; + // eslint-disable-next-line no-restricted-syntax + throw new SyntaxError(message); } const body = root.body[0].expression.callee.body; const state = { diff --git a/lib/repl.js b/lib/repl.js index 2e80d652669ec5..74fee4c9434129 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -426,11 +426,16 @@ function REPLServer(prompt, ({ processTopLevelAwait } = require('internal/repl/await')); } - const potentialWrappedCode = processTopLevelAwait(code); - if (potentialWrappedCode !== null) { - code = potentialWrappedCode; - wrappedCmd = true; - awaitPromise = true; + try { + const potentialWrappedCode = processTopLevelAwait(code); + if (potentialWrappedCode !== null) { + code = potentialWrappedCode; + wrappedCmd = true; + awaitPromise = true; + } + } catch (e) { + decorateErrorStack(e); + err = e; } } @@ -438,47 +443,49 @@ function REPLServer(prompt, if (code === '\n') return cb(null); - let parentURL; - try { - const { pathToFileURL } = require('url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - } - while (true) { + if (err === null) { + let parentURL; try { - if (self.replMode === module.exports.REPL_MODE_STRICT && - !RegExpPrototypeTest(/^\s*$/, code)) { - // "void 0" keeps the repl from returning "use strict" as the result - // value for statements and declarations that don't return a value. - code = `'use strict'; void 0;\n${code}`; - } - script = vm.createScript(code, { - filename: file, - displayErrors: true, - importModuleDynamically: async (specifier) => { - return asyncESM.ESMLoader.import(specifier, parentURL); + const { pathToFileURL } = require('url'); + // Adding `/repl` prevents dynamic imports from loading relative + // to the parent of `process.cwd()`. + parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; + } catch { + } + while (true) { + try { + if (self.replMode === module.exports.REPL_MODE_STRICT && + !RegExpPrototypeTest(/^\s*$/, code)) { + // "void 0" keeps the repl from returning "use strict" as the result + // value for statements and declarations that don't return a value. + code = `'use strict'; void 0;\n${code}`; } - }); - } catch (e) { - debug('parse error %j', code, e); - if (wrappedCmd) { - // Unwrap and try again - wrappedCmd = false; - awaitPromise = false; - code = input; - wrappedErr = e; - continue; + script = vm.createScript(code, { + filename: file, + displayErrors: true, + importModuleDynamically: async (specifier) => { + return asyncESM.ESMLoader.import(specifier, parentURL); + } + }); + } catch (e) { + debug('parse error %j', code, e); + if (wrappedCmd) { + // Unwrap and try again + wrappedCmd = false; + awaitPromise = false; + code = input; + wrappedErr = e; + continue; + } + // Preserve original error for wrapped command + const error = wrappedErr || e; + if (isRecoverableError(error, code)) + err = new Recoverable(error); + else + err = error; } - // Preserve original error for wrapped command - const error = wrappedErr || e; - if (isRecoverableError(error, code)) - err = new Recoverable(error); - else - err = error; + break; } - break; } // This will set the values from `savedRegExMatches` to corresponding diff --git a/test/parallel/test-repl-preprocess-top-level-await.js b/test/parallel/test-repl-preprocess-top-level-await.js index ed1fe90e43e459..93e8fc91f00f76 100644 --- a/test/parallel/test-repl-preprocess-top-level-await.js +++ b/test/parallel/test-repl-preprocess-top-level-await.js @@ -13,13 +13,13 @@ const testCases = [ [ '0', null ], [ 'await 0', - '(async () => { return (await 0) })()' ], + '(async () => {\nreturn (await 0)\n})()' ], [ 'await 0;', - '(async () => { return (await 0); })()' ], + '(async () => {\nreturn (await 0);\n})()' ], [ '(await 0)', - '(async () => { return ((await 0)) })()' ], + '(async () => {\nreturn ((await 0))\n})()' ], [ '(await 0);', - '(async () => { return ((await 0)); })()' ], + '(async () => {\nreturn ((await 0));\n})()' ], [ 'async function foo() { await 0; }', null ], [ 'async () => await 0', @@ -29,38 +29,38 @@ const testCases = [ [ 'await 0; return 0;', null ], [ 'var a = await 1', - '(async () => { void (a = await 1) })()' ], + '(async () => {\nvoid (a = await 1)\n})()' ], [ 'let a = await 1', - '(async () => { void (a = await 1) })()' ], + '(async () => {\nvoid (a = await 1)\n})()' ], [ 'const a = await 1', - '(async () => { void (a = await 1) })()' ], + '(async () => {\nvoid (a = await 1)\n})()' ], [ 'for (var i = 0; i < 1; ++i) { await i }', - '(async () => { for (void (i = 0); i < 1; ++i) { await i } })()' ], + '(async () => {\nfor (void (i = 0); i < 1; ++i) { await i }\n})()' ], [ 'for (let i = 0; i < 1; ++i) { await i }', - '(async () => { for (let i = 0; i < 1; ++i) { await i } })()' ], + '(async () => {\nfor (let i = 0; i < 1; ++i) { await i }\n})()' ], [ 'var {a} = {a:1}, [b] = [1], {c:{d}} = {c:{d: await 1}}', - '(async () => { void ( ({a} = {a:1}), ([b] = [1]), ' + - '({c:{d}} = {c:{d: await 1}})) })()' ], + '(async () => {\nvoid ( ({a} = {a:1}), ([b] = [1]), ' + + '({c:{d}} = {c:{d: await 1}}))\n})()' ], /* eslint-disable no-template-curly-in-string */ [ 'console.log(`${(await { a: 1 }).a}`)', - '(async () => { return (console.log(`${(await { a: 1 }).a}`)) })()' ], + '(async () => {\nreturn (console.log(`${(await { a: 1 }).a}`))\n})()' ], /* eslint-enable no-template-curly-in-string */ [ 'await 0; function foo() {}', - '(async () => { await 0; foo=function foo() {} })()' ], + '(async () => {\nawait 0; foo=function foo() {}\n})()' ], [ 'await 0; class Foo {}', - '(async () => { await 0; Foo=class Foo {} })()' ], + '(async () => {\nawait 0; Foo=class Foo {}\n})()' ], [ 'if (await true) { function foo() {} }', - '(async () => { if (await true) { foo=function foo() {} } })()' ], + '(async () => {\nif (await true) { foo=function foo() {} }\n})()' ], [ 'if (await true) { class Foo{} }', - '(async () => { if (await true) { class Foo{} } })()' ], + '(async () => {\nif (await true) { class Foo{} }\n})()' ], [ 'if (await true) { var a = 1; }', - '(async () => { if (await true) { void (a = 1); } })()' ], + '(async () => {\nif (await true) { void (a = 1); }\n})()' ], [ 'if (await true) { let a = 1; }', - '(async () => { if (await true) { let a = 1; } })()' ], + '(async () => {\nif (await true) { let a = 1; }\n})()' ], [ 'var a = await 1; let b = 2; const c = 3;', - '(async () => { void (a = await 1); void (b = 2); void (c = 3); })()' ], + '(async () => {\nvoid (a = await 1); void (b = 2); void (c = 3);\n})()' ], [ 'let o = await 1, p', - '(async () => { void ( (o = await 1), (p=undefined)) })()' ], + '(async () => {\nvoid ( (o = await 1), (p=undefined))\n})()' ], ]; for (const [input, expected] of testCases) { diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js index 39227d4f8bad29..319633838bd358 100644 --- a/test/parallel/test-repl-top-level-await.js +++ b/test/parallel/test-repl-top-level-await.js @@ -142,6 +142,16 @@ async function ordinaryTests() { 'undefined', ], ], + ['await Promise..resolve()', + [ + 'await Promise..resolve()\r', + 'Uncaught SyntaxError: ', + 'await Promise..resolve()', + ' ^', + '', + 'Unexpected token \'.\'', + ], + ], ]; for (const [input, expected = [`${input}\r`], options = {}] of testCases) { From c162852854a60431f0212a7e8b902e6b4be3aa61 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 25 Jun 2021 11:21:13 -0700 Subject: [PATCH 2/6] fixup: remove redundant check --- lib/internal/repl/await.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index 4ae0644adf5144..302356ee240090 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -8,6 +8,10 @@ const { ArrayPrototypePush, FunctionPrototype, ObjectKeys, + StringPrototypeEndsWith, + StringPrototypeIncludes, + StringPrototypeReplace, + StringPrototypeSplit, SyntaxError, } = primordials; @@ -98,20 +102,18 @@ function processTopLevelAwait(src) { // Convert keyword parse errors on await into their original errors when // possible. if (errPos === awaitPos + 6 && - src.slice(errPos - 6, errPos - 1) === 'await' && - e.message.includes('Expecting Unicode escape sequence')) + StringPrototypeIncludes(e.message, 'Expecting Unicode escape sequence')) return null; if (errPos === awaitPos + 7 && - src.slice(errPos - 7, errPos - 2) === 'await' && - e.message.includes('Unexpected token')) + StringPrototypeIncludes(e.message, 'Unexpected token')) return null; const { line, column } = e.loc; - let message = '\n' + src.split('\n')[line - 2] + '\n'; + let message = '\n' + StringPrototypeSplit(src, '\n')[line - 2] + '\n'; let i = 0; while (i++ < column) message += ' '; - message += '^\n\n' + e.message.replace(/ \([^)]+\)/, ''); + message += '^\n\n' + StringPrototypeReplace(e.message, / \([^)]+\)/, ''); // V8 unexpected token errors include the token string. - if (message.endsWith('Unexpected token')) + if (StringPrototypeEndsWith(message, 'Unexpected token')) message += " '" + src[e.pos - wrapPrefix.length] + "'"; // eslint-disable-next-line no-restricted-syntax throw new SyntaxError(message); From 0cb7f22b10b4feb766da0b0e0c50c9958f3e9e9b Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 25 Jun 2021 13:37:03 -0700 Subject: [PATCH 3/6] fixup: pr feedback --- lib/internal/repl/await.js | 20 +++++----- .../test-repl-preprocess-top-level-await.js | 40 +++++++++---------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index 302356ee240090..9799d38d5ecbc0 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -8,9 +8,11 @@ const { ArrayPrototypePush, FunctionPrototype, ObjectKeys, + RegExpPrototypeSymbolReplace, StringPrototypeEndsWith, StringPrototypeIncludes, - StringPrototypeReplace, + StringPrototypeIndexOf, + StringPrototypeRepeat, StringPrototypeSplit, SyntaxError, } = primordials; @@ -85,8 +87,8 @@ for (const nodeType of ObjectKeys(walk.base)) { } function processTopLevelAwait(src) { - const wrapPrefix = '(async () => {\n'; - const wrapped = `${wrapPrefix}${src}\n})()`; + const wrapPrefix = '(async () => { '; + const wrapped = `${wrapPrefix}${src} })()`; const wrappedArray = ArrayFrom(wrapped); let root; try { @@ -95,7 +97,7 @@ function processTopLevelAwait(src) { // If the parse error is before the first "await", then use the execution // error. Otherwise we must emit this parse error, making it look like a // proper syntax error. - const awaitPos = src.indexOf('await'); + const awaitPos = StringPrototypeIndexOf(src, 'await'); const errPos = e.pos - wrapPrefix.length; if (awaitPos > errPos) return null; @@ -107,11 +109,11 @@ function processTopLevelAwait(src) { if (errPos === awaitPos + 7 && StringPrototypeIncludes(e.message, 'Unexpected token')) return null; - const { line, column } = e.loc; - let message = '\n' + StringPrototypeSplit(src, '\n')[line - 2] + '\n'; - let i = 0; - while (i++ < column) message += ' '; - message += '^\n\n' + StringPrototypeReplace(e.message, / \([^)]+\)/, ''); + const line = e.loc.line; + const column = line === 1 ? e.loc.column - wrapPrefix.length : e.loc.column; + let message = '\n' + StringPrototypeSplit(src, '\n')[line - 1] + '\n' + + StringPrototypeRepeat(' ', column) + + '^\n\n' + RegExpPrototypeSymbolReplace(/ \([^)]+\)/, e.message, ''); // V8 unexpected token errors include the token string. if (StringPrototypeEndsWith(message, 'Unexpected token')) message += " '" + src[e.pos - wrapPrefix.length] + "'"; diff --git a/test/parallel/test-repl-preprocess-top-level-await.js b/test/parallel/test-repl-preprocess-top-level-await.js index 93e8fc91f00f76..ed1fe90e43e459 100644 --- a/test/parallel/test-repl-preprocess-top-level-await.js +++ b/test/parallel/test-repl-preprocess-top-level-await.js @@ -13,13 +13,13 @@ const testCases = [ [ '0', null ], [ 'await 0', - '(async () => {\nreturn (await 0)\n})()' ], + '(async () => { return (await 0) })()' ], [ 'await 0;', - '(async () => {\nreturn (await 0);\n})()' ], + '(async () => { return (await 0); })()' ], [ '(await 0)', - '(async () => {\nreturn ((await 0))\n})()' ], + '(async () => { return ((await 0)) })()' ], [ '(await 0);', - '(async () => {\nreturn ((await 0));\n})()' ], + '(async () => { return ((await 0)); })()' ], [ 'async function foo() { await 0; }', null ], [ 'async () => await 0', @@ -29,38 +29,38 @@ const testCases = [ [ 'await 0; return 0;', null ], [ 'var a = await 1', - '(async () => {\nvoid (a = await 1)\n})()' ], + '(async () => { void (a = await 1) })()' ], [ 'let a = await 1', - '(async () => {\nvoid (a = await 1)\n})()' ], + '(async () => { void (a = await 1) })()' ], [ 'const a = await 1', - '(async () => {\nvoid (a = await 1)\n})()' ], + '(async () => { void (a = await 1) })()' ], [ 'for (var i = 0; i < 1; ++i) { await i }', - '(async () => {\nfor (void (i = 0); i < 1; ++i) { await i }\n})()' ], + '(async () => { for (void (i = 0); i < 1; ++i) { await i } })()' ], [ 'for (let i = 0; i < 1; ++i) { await i }', - '(async () => {\nfor (let i = 0; i < 1; ++i) { await i }\n})()' ], + '(async () => { for (let i = 0; i < 1; ++i) { await i } })()' ], [ 'var {a} = {a:1}, [b] = [1], {c:{d}} = {c:{d: await 1}}', - '(async () => {\nvoid ( ({a} = {a:1}), ([b] = [1]), ' + - '({c:{d}} = {c:{d: await 1}}))\n})()' ], + '(async () => { void ( ({a} = {a:1}), ([b] = [1]), ' + + '({c:{d}} = {c:{d: await 1}})) })()' ], /* eslint-disable no-template-curly-in-string */ [ 'console.log(`${(await { a: 1 }).a}`)', - '(async () => {\nreturn (console.log(`${(await { a: 1 }).a}`))\n})()' ], + '(async () => { return (console.log(`${(await { a: 1 }).a}`)) })()' ], /* eslint-enable no-template-curly-in-string */ [ 'await 0; function foo() {}', - '(async () => {\nawait 0; foo=function foo() {}\n})()' ], + '(async () => { await 0; foo=function foo() {} })()' ], [ 'await 0; class Foo {}', - '(async () => {\nawait 0; Foo=class Foo {}\n})()' ], + '(async () => { await 0; Foo=class Foo {} })()' ], [ 'if (await true) { function foo() {} }', - '(async () => {\nif (await true) { foo=function foo() {} }\n})()' ], + '(async () => { if (await true) { foo=function foo() {} } })()' ], [ 'if (await true) { class Foo{} }', - '(async () => {\nif (await true) { class Foo{} }\n})()' ], + '(async () => { if (await true) { class Foo{} } })()' ], [ 'if (await true) { var a = 1; }', - '(async () => {\nif (await true) { void (a = 1); }\n})()' ], + '(async () => { if (await true) { void (a = 1); } })()' ], [ 'if (await true) { let a = 1; }', - '(async () => {\nif (await true) { let a = 1; }\n})()' ], + '(async () => { if (await true) { let a = 1; } })()' ], [ 'var a = await 1; let b = 2; const c = 3;', - '(async () => {\nvoid (a = await 1); void (b = 2); void (c = 3);\n})()' ], + '(async () => { void (a = await 1); void (b = 2); void (c = 3); })()' ], [ 'let o = await 1, p', - '(async () => {\nvoid ( (o = await 1), (p=undefined))\n})()' ], + '(async () => { void ( (o = await 1), (p=undefined)) })()' ], ]; for (const [input, expected] of testCases) { From 96781d683e1fa7b652ae79ac543a09291811bb35 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 27 Jun 2021 07:10:45 -0700 Subject: [PATCH 4/6] fixup: recoverable handling --- lib/internal/repl/await.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index 9799d38d5ecbc0..65d397718bef10 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -19,6 +19,7 @@ const { const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; const walk = require('internal/deps/acorn/acorn-walk/dist/walk'); +const { Recoverable } = require('internal/repl'); const noop = FunctionPrototype; const visitorsWithoutAncestors = { @@ -94,6 +95,8 @@ function processTopLevelAwait(src) { try { root = parser.parse(wrapped, { ecmaVersion: 'latest' }); } catch (e) { + if (e.message.startsWith('Unterminated ')) + throw new Recoverable(e); // If the parse error is before the first "await", then use the execution // error. Otherwise we must emit this parse error, making it look like a // proper syntax error. From b75ad069e4a2ee09a1cd3013a7ace70afd2cd14f Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 27 Jun 2021 08:49:33 -0700 Subject: [PATCH 5/6] fixup: Update lib/internal/repl/await.js Co-authored-by: Jordan Harband --- lib/internal/repl/await.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index 65d397718bef10..b9e1b8725cc586 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -95,7 +95,7 @@ function processTopLevelAwait(src) { try { root = parser.parse(wrapped, { ecmaVersion: 'latest' }); } catch (e) { - if (e.message.startsWith('Unterminated ')) + if (StringPrototypeStartsWith(e.message, 'Unterminated ')) throw new Recoverable(e); // If the parse error is before the first "await", then use the execution // error. Otherwise we must emit this parse error, making it look like a From 2a3a06b08eb2d9211f0fb7c03d22e7a58b9ddda0 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 27 Jun 2021 17:50:02 +0200 Subject: [PATCH 6/6] fixup: add startswith import --- lib/internal/repl/await.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index b9e1b8725cc586..09547117a6565a 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -14,6 +14,7 @@ const { StringPrototypeIndexOf, StringPrototypeRepeat, StringPrototypeSplit, + StringPrototypeStartsWith, SyntaxError, } = primordials;