From 9affc6f44839d49cf438cfbcf1279648df58f1ce Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Fri, 13 Jul 2018 15:46:21 -0700 Subject: [PATCH 1/7] repl: support mult-line string-keyed objects isRecoverableError is completely reimplemented using acorn and an acorn plugin that examines the state of the parser at the time of the error to determine if the code could be completed on a subsequent line. --- lib/repl.js | 113 +++++++++++++++++-------------------- test/parallel/test-repl.js | 21 ++++--- 2 files changed, 65 insertions(+), 69 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 92c90de7bb1646..1d2a306f9285a1 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -47,10 +47,11 @@ const { makeRequireFunction, addBuiltinLibsToObject } = require('internal/modules/cjs/helpers'); +const acorn = require('internal/deps/acorn/dist/acorn'); const { isIdentifierStart, isIdentifierChar -} = require('internal/deps/acorn/dist/acorn'); +} = acorn; const internalUtil = require('internal/util'); const { isTypedArray } = require('internal/util/types'); const util = require('util'); @@ -1507,72 +1508,60 @@ function regexpEscape(s) { // If the error is that we've unexpectedly ended the input, // then let the user try to recover by adding more input. +// Note: `e` (the original exception is not used by the current implemention, +// but may be needed in the future.) function isRecoverableError(e, code) { - if (e && e.name === 'SyntaxError') { - var message = e.message; - if (message === 'Unterminated template literal' || - message === 'Unexpected end of input') { - return true; - } + let recoverable = false; + + // Determine if the point of the any error raised is at the end of the input. + // There are two cases to consider: + // 1. Any error raised after we have encountered the 'eof' token. + // This prevents us from declaring partial tokens (like '2e') as + // recoverable. + // 2. Three cases where tokens can legally span lines. This is + // template, comment, and strings with a backslash at the end of + // the line, indicating a continuation. + acorn.plugins.repl = (parser) => { + parser.extend('nextToken', (nextToken) => { + return function(pos, message) { + nextToken.call(this, pos, message); + + if (parser.type === acorn.tokTypes.eof) recoverable = true; + }; + }); - if (message === 'missing ) after argument list') { - const frames = e.stack.split(/\r?\n/); - const pos = frames.findIndex((f) => f.match(/^\s*\^+$/)); - return pos > 0 && frames[pos - 1].length === frames[pos].length; - } + parser.extend('raise', (raise) => { + return function(pos, message) { + switch (message) { + case 'Unterminated template': + case 'Unterminated comment': + recoverable = true; + break; + + case 'Unterminated string constant': + const token = parser.input.slice(parser.lastTokStart, parser.pos); + recoverable = token.endsWith('\\\n'); + } - if (message === 'Invalid or unexpected token') - return isCodeRecoverable(code); - } - return false; -} + raise.call(this, pos, message); + }; + }); + }; -// Check whether a code snippet should be forced to fail in the REPL. -function isCodeRecoverable(code) { - var current, previous, stringLiteral; - var isBlockComment = false; - var isSingleComment = false; - var isRegExpLiteral = false; - var lastChar = code.charAt(code.length - 2); - var prevTokenChar = null; - - for (var i = 0; i < code.length; i++) { - previous = current; - current = code[i]; - - if (previous === '\\' && (stringLiteral || isRegExpLiteral)) { - current = null; - } else if (stringLiteral) { - if (stringLiteral === current) { - stringLiteral = null; - } - } else if (isRegExpLiteral && current === '/') { - isRegExpLiteral = false; - } else if (isBlockComment && previous === '*' && current === '/') { - isBlockComment = false; - } else if (isSingleComment && current === '\n') { - isSingleComment = false; - } else if (!isBlockComment && !isRegExpLiteral && !isSingleComment) { - if (current === '/' && previous === '/') { - isSingleComment = true; - } else if (previous === '/') { - if (current === '*') { - isBlockComment = true; - // Distinguish between a division operator and the start of a regex - // by examining the non-whitespace character that precedes the / - } else if ([null, '(', '[', '{', '}', ';'].includes(prevTokenChar)) { - isRegExpLiteral = true; - } - } else { - if (current.trim()) prevTokenChar = current; - if (current === '\'' || current === '"') { - stringLiteral = current; - } - } - } + // For similar reasons as `defaultEval`, wrap expressions starting with a + // curly brace with parenthesis. Note: only the open parenthesis is added + // here as the point is to test for potentially valid but incomplete + // expressions. + if (/^\s*\{/.test(code) && isRecoverableError(e, `(${code}`)) return true; + + // Try to parse the code with acorn. If the parse fails, ignore the acorn + // error and return the recoverable status. + try { + acorn.parse(code, { plugins: { repl: true } }); + return true; + } catch (e) { + return recoverable; } - - return stringLiteral ? lastChar === '\\' : isBlockComment; } function Recoverable(err) { diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 35cd3e11afab53..8cb4b686b85e40 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -162,13 +162,11 @@ const errorTests = [ // Template expressions { send: '`io.js ${"1.0"', - expect: [ - kSource, - kArrow, - '', - /^SyntaxError: /, - '' - ] + expect: '... ' + }, + { + send: '+ ".2"}`', + expect: '\'io.js 1.0.2\'' }, { send: '`io.js ${', @@ -315,6 +313,15 @@ const errorTests = [ send: '1 }', expect: '{ a: 1 }' }, + // Multiline string-keyed object (e.g. JSON) + { + send: '{ "a": ', + expect: '... ' + }, + { + send: '1 }', + expect: '{ a: 1 }' + }, // Multiline anonymous function with comment { send: '(function() {', From 7bb9bf17ea6a368be4ee847904d12b85b0ff2ace Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sat, 14 Jul 2018 13:26:39 -0700 Subject: [PATCH 2/7] parser. => this. and acorn.tokTypes => tt --- lib/repl.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 1d2a306f9285a1..be55a5695b2912 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -49,6 +49,7 @@ const { } = require('internal/modules/cjs/helpers'); const acorn = require('internal/deps/acorn/dist/acorn'); const { + tokTypes: tt, isIdentifierStart, isIdentifierChar } = acorn; @@ -1526,7 +1527,7 @@ function isRecoverableError(e, code) { return function(pos, message) { nextToken.call(this, pos, message); - if (parser.type === acorn.tokTypes.eof) recoverable = true; + if (this.type === tt.eof) recoverable = true; }; }); @@ -1539,7 +1540,7 @@ function isRecoverableError(e, code) { break; case 'Unterminated string constant': - const token = parser.input.slice(parser.lastTokStart, parser.pos); + const token = this.input.slice(this.lastTokStart, this.pos); recoverable = token.endsWith('\\\n'); } From d426b00a27070329b2396c003129a44aff726d5d Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sun, 15 Jul 2018 08:01:10 -0700 Subject: [PATCH 3/7] split out isRecoverableError, changed call to Reflect.apply. Added comments for regular expressions that need to match and comparisons against exception message strings produced by Acorn. --- lib/internal/repl/recoverable.js | 78 ++++++++++++++++++++++++++++++++ lib/repl.js | 66 ++------------------------- node.gyp | 1 + 3 files changed, 83 insertions(+), 62 deletions(-) create mode 100644 lib/internal/repl/recoverable.js diff --git a/lib/internal/repl/recoverable.js b/lib/internal/repl/recoverable.js new file mode 100644 index 00000000000000..02c35229f35f6f --- /dev/null +++ b/lib/internal/repl/recoverable.js @@ -0,0 +1,78 @@ +'use strict'; + +const acorn = require('internal/deps/acorn/dist/acorn'); +const { tokTypes: tt } = acorn; + +// If the error is that we've unexpectedly ended the input, +// then let the user try to recover by adding more input. +// Note: `e` (the original exception) is not used by the current implemention, +// but may be needed in the future. +function isRecoverableError(e, code) { + let recoverable = false; + + // Determine if the point of the any error raised is at the end of the input. + // There are two cases to consider: + // + // 1. Any error raised after we have encountered the 'eof' token. + // This prevents us from declaring partial tokens (like '2e') as + // recoverable. + // + // 2. Three cases where tokens can legally span lines. This is + // template, comment, and strings with a backslash at the end of + // the line, indicating a continuation. Note that we need to look + // for the specific errors of 'unterminated' kind (not, for example, + // a syntax error in a ${} expression in a template), and the only + // way to do that currently is to look at the message. Should Acorn + // change these messages in the future, this will lead to a test + // failure, indicating that this code needs to be updated. + // + acorn.plugins.repl = (parser) => { + parser.extend('nextToken', (nextToken) => { + return function() { + Reflect.apply(nextToken, this, []); + + if (this.type === tt.eof) recoverable = true; + }; + }); + + parser.extend('raise', (raise) => { + return function(pos, message) { + switch (message) { + case 'Unterminated template': + case 'Unterminated comment': + recoverable = true; + break; + + case 'Unterminated string constant': + const token = this.input.slice(this.lastTokStart, this.pos); + recoverable = token.endsWith('\\\n'); + } + + Reflect.apply(raise, this, [pos, message]); + }; + }); + }; + + // For similar reasons as `defaultEval`, wrap expressions starting with a + // curly brace with parenthesis. Note: only the open parenthesis is added + // here as the point is to test for potentially valid but incomplete + // expressions. + if (/^\s*\{/.test(code) && isRecoverableError(e, `(${code}`)) return true; + + // Try to parse the code with acorn. If the parse fails, ignore the acorn + // error and return the recoverable status. + try { + acorn.parse(code, { plugins: { repl: true } }); + + // Odd case: the underlying JS engine (V8, Chakra) rejected this input + // but Acorn detected no issue. Presume that additional text won't + // address this issue. + return false; + } catch (e) { + return recoverable; + } +} + +module.exports = { + isRecoverableError +}; diff --git a/lib/repl.js b/lib/repl.js index be55a5695b2912..4a01595ce1b72b 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -47,12 +47,10 @@ const { makeRequireFunction, addBuiltinLibsToObject } = require('internal/modules/cjs/helpers'); -const acorn = require('internal/deps/acorn/dist/acorn'); const { - tokTypes: tt, isIdentifierStart, isIdentifierChar -} = acorn; +} = require('internal/deps/acorn/dist/acorn'); const internalUtil = require('internal/util'); const { isTypedArray } = require('internal/util/types'); const util = require('util'); @@ -75,6 +73,7 @@ const { } = require('internal/errors').codes; const { sendInspectorCommand } = require('internal/util/inspector'); const { experimentalREPLAwait } = process.binding('config'); +const { isRecoverableError } = require('internal/repl/recoverable'); // Lazy-loaded. let processTopLevelAwait; @@ -229,7 +228,8 @@ function REPLServer(prompt, // It's confusing for `{ a : 1 }` to be interpreted as a block // statement rather than an object literal. So, we first try // to wrap it in parentheses, so that it will be interpreted as - // an expression. + // an expression. Note that if the above condition changes, + // lib/internal/repl/recoverable.js needs to be changed to match. code = `(${code.trim()})\n`; wrappedCmd = true; } @@ -1507,64 +1507,6 @@ function regexpEscape(s) { return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&'); } -// If the error is that we've unexpectedly ended the input, -// then let the user try to recover by adding more input. -// Note: `e` (the original exception is not used by the current implemention, -// but may be needed in the future.) -function isRecoverableError(e, code) { - let recoverable = false; - - // Determine if the point of the any error raised is at the end of the input. - // There are two cases to consider: - // 1. Any error raised after we have encountered the 'eof' token. - // This prevents us from declaring partial tokens (like '2e') as - // recoverable. - // 2. Three cases where tokens can legally span lines. This is - // template, comment, and strings with a backslash at the end of - // the line, indicating a continuation. - acorn.plugins.repl = (parser) => { - parser.extend('nextToken', (nextToken) => { - return function(pos, message) { - nextToken.call(this, pos, message); - - if (this.type === tt.eof) recoverable = true; - }; - }); - - parser.extend('raise', (raise) => { - return function(pos, message) { - switch (message) { - case 'Unterminated template': - case 'Unterminated comment': - recoverable = true; - break; - - case 'Unterminated string constant': - const token = this.input.slice(this.lastTokStart, this.pos); - recoverable = token.endsWith('\\\n'); - } - - raise.call(this, pos, message); - }; - }); - }; - - // For similar reasons as `defaultEval`, wrap expressions starting with a - // curly brace with parenthesis. Note: only the open parenthesis is added - // here as the point is to test for potentially valid but incomplete - // expressions. - if (/^\s*\{/.test(code) && isRecoverableError(e, `(${code}`)) return true; - - // Try to parse the code with acorn. If the parse fails, ignore the acorn - // error and return the recoverable status. - try { - acorn.parse(code, { plugins: { repl: true } }); - return true; - } catch (e) { - return recoverable; - } -} - function Recoverable(err) { this.err = err; } diff --git a/node.gyp b/node.gyp index 7460d73c0b578a..682111051cb800 100644 --- a/node.gyp +++ b/node.gyp @@ -147,6 +147,7 @@ 'lib/internal/readline.js', 'lib/internal/repl.js', 'lib/internal/repl/await.js', + 'lib/internal/repl/recoverable.js', 'lib/internal/socket_list.js', 'lib/internal/test/binding.js', 'lib/internal/test/heap.js', From 974b33721006782d2e518e2a196a2fb7c6ba8e34 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sun, 15 Jul 2018 10:33:01 -0700 Subject: [PATCH 4/7] s/plugins.repl/plugins.replRecoverable/g --- lib/internal/repl/recoverable.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/repl/recoverable.js b/lib/internal/repl/recoverable.js index 02c35229f35f6f..8bfbd7a76a2e5d 100644 --- a/lib/internal/repl/recoverable.js +++ b/lib/internal/repl/recoverable.js @@ -26,7 +26,7 @@ function isRecoverableError(e, code) { // change these messages in the future, this will lead to a test // failure, indicating that this code needs to be updated. // - acorn.plugins.repl = (parser) => { + acorn.plugins.replRecoverable = (parser) => { parser.extend('nextToken', (nextToken) => { return function() { Reflect.apply(nextToken, this, []); @@ -62,7 +62,7 @@ function isRecoverableError(e, code) { // Try to parse the code with acorn. If the parse fails, ignore the acorn // error and return the recoverable status. try { - acorn.parse(code, { plugins: { repl: true } }); + acorn.parse(code, { plugins: { replRecoverable: true } }); // Odd case: the underlying JS engine (V8, Chakra) rejected this input // but Acorn detected no issue. Presume that additional text won't From 02c5827c32da84770a6697af65d66e076b4312a1 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Wed, 18 Jul 2018 15:31:11 -0400 Subject: [PATCH 5/7] all Line Terminators matter --- lib/internal/repl/recoverable.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/repl/recoverable.js b/lib/internal/repl/recoverable.js index 8bfbd7a76a2e5d..81a8fb7e61eaf3 100644 --- a/lib/internal/repl/recoverable.js +++ b/lib/internal/repl/recoverable.js @@ -45,7 +45,8 @@ function isRecoverableError(e, code) { case 'Unterminated string constant': const token = this.input.slice(this.lastTokStart, this.pos); - recoverable = token.endsWith('\\\n'); + // see https://www.ecma-international.org/ecma-262/#sec-line-terminators + recoverable = token.match(/\\(\r\n?|\n|\u2028|\u2029)$/); } Reflect.apply(raise, this, [pos, message]); From e58b861daaffca2f12e23c722448fe308babe435 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Thu, 19 Jul 2018 09:19:45 -0400 Subject: [PATCH 6/7] token.match() => .test(token) --- lib/internal/repl/recoverable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/repl/recoverable.js b/lib/internal/repl/recoverable.js index 81a8fb7e61eaf3..29411d65347830 100644 --- a/lib/internal/repl/recoverable.js +++ b/lib/internal/repl/recoverable.js @@ -46,7 +46,7 @@ function isRecoverableError(e, code) { case 'Unterminated string constant': const token = this.input.slice(this.lastTokStart, this.pos); // see https://www.ecma-international.org/ecma-262/#sec-line-terminators - recoverable = token.match(/\\(\r\n?|\n|\u2028|\u2029)$/); + recoverable = /\\(?:\r\n?|\n|\u2028|\u2029)$/.test(token); } Reflect.apply(raise, this, [pos, message]); From d426d86e0c6777910255c1df05f43905da94b9d4 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sat, 28 Jul 2018 11:32:19 -0400 Subject: [PATCH 7/7] remove unused exception variable --- lib/internal/repl/recoverable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/repl/recoverable.js b/lib/internal/repl/recoverable.js index 29411d65347830..465d77451a5b82 100644 --- a/lib/internal/repl/recoverable.js +++ b/lib/internal/repl/recoverable.js @@ -69,7 +69,7 @@ function isRecoverableError(e, code) { // but Acorn detected no issue. Presume that additional text won't // address this issue. return false; - } catch (e) { + } catch { return recoverable; } }