From 81ea52aa01aa264b1424e00bc240b956b19ddaa5 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Sun, 12 Jul 2015 01:22:33 +0000 Subject: [PATCH] repl: improving line continuation handling As it is, REPL doesn't honour the line continuation feature very well. This patch 1. keeps track of the beginning of the string literals and if they don't end or current line doesn't end with line continuation, then error out. 2. monitors if the line continuation character is used without the string literal and errors out if that happens. PR-URL: https://github.com/nodejs/io.js/pull/2163 Reviewed-By: Jeremiah Senkpiel --- lib/repl.js | 90 +++++++++++++++++++++++++++++++------- test/parallel/test-repl.js | 27 ++++++++++++ 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 0f93bfdc40a734..d978f27dbf0d6b 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -174,6 +174,7 @@ function REPLServer(prompt, self._domain.on('error', function(e) { debug('domain error'); self.outputStream.write((e.stack || e) + '\n'); + self._currentStringLiteral = null; self.bufferedCommand = ''; self.lines.level = []; self.displayPrompt(); @@ -200,6 +201,8 @@ function REPLServer(prompt, self.outputStream = output; self.resetContext(); + // Initialize the current string literal found, to be null + self._currentStringLiteral = null; self.bufferedCommand = ''; self.lines.level = []; @@ -258,21 +261,86 @@ function REPLServer(prompt, sawSIGINT = false; } + self._currentStringLiteral = null; self.bufferedCommand = ''; self.lines.level = []; self.displayPrompt(); }); + function parseLine(line, currentStringLiteral) { + var previous = null, current = null; + + for (var i = 0; i < line.length; i += 1) { + if (previous === '\\') { + // if it is a valid escaping, then skip processing + previous = current; + continue; + } + + current = line.charAt(i); + if (current === currentStringLiteral) { + currentStringLiteral = null; + } else if (current === '\'' || + current === '"' && + currentStringLiteral === null) { + currentStringLiteral = current; + } + previous = current; + } + + return currentStringLiteral; + } + + function getFinisherFunction(cmd, defaultFn) { + if ((self._currentStringLiteral === null && + cmd.charAt(cmd.length - 1) === '\\') || + (self._currentStringLiteral !== null && + cmd.charAt(cmd.length - 1) !== '\\')) { + + // If the line continuation is used outside string literal or if the + // string continuation happens with out line continuation, then fail hard. + // Even if the error is recoverable, get the underlying error and use it. + return function(e, ret) { + var error = e instanceof Recoverable ? e.err : e; + + if (arguments.length === 2) { + // using second argument only if it is actually passed. Otherwise + // `undefined` will be printed when invalid REPL commands are used. + return defaultFn(error, ret); + } + + return defaultFn(error); + }; + } + return defaultFn; + } + self.on('line', function(cmd) { debug('line %j', cmd); sawSIGINT = false; var skipCatchall = false; + var finisherFn = finish; // leading whitespaces in template literals should not be trimmed. if (self._inTemplateLiteral) { self._inTemplateLiteral = false; } else { - cmd = trimWhitespace(cmd); + const wasWithinStrLiteral = self._currentStringLiteral !== null; + self._currentStringLiteral = parseLine(cmd, self._currentStringLiteral); + const isWithinStrLiteral = self._currentStringLiteral !== null; + + if (!wasWithinStrLiteral && !isWithinStrLiteral) { + // Current line has nothing to do with String literals, trim both ends + cmd = cmd.trim(); + } else if (wasWithinStrLiteral && !isWithinStrLiteral) { + // was part of a string literal, but it is over now, trim only the end + cmd = cmd.trimRight(); + } else if (isWithinStrLiteral && !wasWithinStrLiteral) { + // was not part of a string literal, but it is now, trim only the start + cmd = cmd.trimLeft(); + } + + finisherFn = getFinisherFunction(cmd, finish); } // Check to see if a REPL keyword was used. If it returns true, @@ -305,9 +373,9 @@ function REPLServer(prompt, } debug('eval %j', evalCmd); - self.eval(evalCmd, self.context, 'repl', finish); + self.eval(evalCmd, self.context, 'repl', finisherFn); } else { - finish(null); + finisherFn(null); } function finish(e, ret) { @@ -318,6 +386,7 @@ function REPLServer(prompt, self.outputStream.write('npm should be run outside of the ' + 'node repl, in your normal shell.\n' + '(Press Control-D to exit.)\n'); + self._currentStringLiteral = null; self.bufferedCommand = ''; self.displayPrompt(); return; @@ -339,6 +408,7 @@ function REPLServer(prompt, } // Clear buffer if no SyntaxErrors + self._currentStringLiteral = null; self.bufferedCommand = ''; // If we got any output - print it (if no error) @@ -870,6 +940,7 @@ function defineDefaultCommands(repl) { repl.defineCommand('break', { help: 'Sometimes you get stuck, this gets you out', action: function() { + this._currentStringLiteral = null; this.bufferedCommand = ''; this.displayPrompt(); } @@ -884,6 +955,7 @@ function defineDefaultCommands(repl) { repl.defineCommand('clear', { help: clearMessage, action: function() { + this._currentStringLiteral = null; this.bufferedCommand = ''; if (!this.useGlobal) { this.outputStream.write('Clearing context...\n'); @@ -949,18 +1021,6 @@ function defineDefaultCommands(repl) { }); } - -function trimWhitespace(cmd) { - const trimmer = /^\s*(.+)\s*$/m; - var matches = trimmer.exec(cmd); - - if (matches && matches.length === 2) { - return matches[1]; - } - return ''; -} - - function regexpEscape(s) { return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&'); } diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 65affa9b9480e8..27802c782e0f4e 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -198,6 +198,33 @@ function error_test() { // a REPL command { client: client_unix, send: '.toString', expect: 'Invalid REPL keyword\n' + prompt_unix }, + // fail when we are not inside a String and a line continuation is used + { client: client_unix, send: '[] \\', + expect: /^SyntaxError: Unexpected token ILLEGAL/ }, + // do not fail when a String is created with line continuation + { client: client_unix, send: '\'the\\\nfourth\\\neye\'', + expect: prompt_multiline + prompt_multiline + + '\'thefourtheye\'\n' + prompt_unix }, + // Don't fail when a partial String is created and line continuation is used + // with whitespace characters at the end of the string. We are to ignore it. + // This test is to make sure that we properly remove the whitespace + // characters at the end of line, unlike the buggy `trimWhitespace` function + { client: client_unix, send: ' \t .break \t ', + expect: prompt_unix }, + // multiline strings preserve whitespace characters in them + { client: client_unix, send: '\'the \\\n fourth\t\t\\\n eye \'', + expect: prompt_multiline + prompt_multiline + + '\'the fourth\\t\\t eye \'\n' + prompt_unix }, + // more than one multiline strings also should preserve whitespace chars + { client: client_unix, send: '\'the \\\n fourth\' + \'\t\t\\\n eye \'', + expect: prompt_multiline + prompt_multiline + + '\'the fourth\\t\\t eye \'\n' + prompt_unix }, + // using REPL commands within a string literal should still work + { client: client_unix, send: '\'\\\n.break', + expect: prompt_unix }, + // using REPL command "help" within a string literal should still work + { client: client_unix, send: '\'thefourth\\\n.help\neye\'', + expect: /'thefourtheye'/ }, ]); }