lukechilds
/
node
mirror of https://github.com/lukechilds/node.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

repl: handle comments properly 

As it is, the comments are not handled properly in REPL. So, if the
comments have `'` or `"`, then they are treated as incomplete string
literals and the error is thrown in REPL.

This patch refactors the existing logic and groups everything in a
class.

Fixes: https://github.com/nodejs/node/issues/3421
PR-URL: https://github.com/nodejs/node/pull/3515
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
v4.x
Sakthipriyan Vairamani 9 years ago
committed by James M Snell
parent
8d78d687d5
commit
8a2c4aeeaa
2 changed files with 122 additions and 78 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 172
      lib/repl.js
  2. 28
      test/parallel/test-repl.js

172
lib/repl.js
View File

@ -70,6 +70,88 @@ const BLOCK_SCOPED_ERROR = 'Block-scoped declarations (let, ' +
'const, function, class) not yet supported outside strict mode';
class LineParser {
constructor() {
this.reset();
}
reset() {
this._literal = null;
this.shouldFail = false;
this.blockComment = false;
}
parseLine(line) {
var previous = null;
this.shouldFail = false;
const wasWithinStrLiteral = this._literal !== null;
for (const current of line) {
if (previous === '\\') {
// valid escaping, skip processing. previous doesn't matter anymore
previous = null;
continue;
}
if (!this._literal) {
if (previous === '*' && current === '/') {
if (this.blockComment) {
this.blockComment = false;
previous = null;
continue;
} else {
this.shouldFail = true;
break;
}
}
// ignore rest of the line if `current` and `previous` are `/`s
if (previous === current && previous === '/' && !this.blockComment) {
break;
}
if (previous === '/' && current === '*') {
this.blockComment = true;
previous = null;
}
}
if (this.blockComment) continue;
if (current === this._literal) {
this._literal = null;
} else if (current === '\'' || current === '"') {
this._literal = this._literal || current;
}
previous = current;
}
const isWithinStrLiteral = this._literal !== null;
if (!wasWithinStrLiteral && !isWithinStrLiteral) {
// Current line has nothing to do with String literals, trim both ends
line = line.trim();
} else if (wasWithinStrLiteral && !isWithinStrLiteral) {
// was part of a string literal, but it is over now, trim only the end
line = line.trimRight();
} else if (isWithinStrLiteral && !wasWithinStrLiteral) {
// was not part of a string literal, but it is now, trim only the start
line = line.trimLeft();
}
const lastChar = line.charAt(line.length - 1);
this.shouldFail = this.shouldFail ||
((!this._literal && lastChar === '\\') ||
(this._literal && lastChar !== '\\'));
return line;
}
}
function REPLServer(prompt,
stream,
eval_,
@ -193,7 +275,7 @@ function REPLServer(prompt,
debug('domain error');
const top = replMap.get(self);
top.outputStream.write((e.stack || e) + '\n');
top._currentStringLiteral = null;
top.lineParser.reset();
top.bufferedCommand = '';
top.lines.level = [];
top.displayPrompt();
@ -220,8 +302,7 @@ function REPLServer(prompt,
self.outputStream = output;
self.resetContext();
// Initialize the current string literal found, to be null
self._currentStringLiteral = null;
self.lineParser = new LineParser();
self.bufferedCommand = '';
self.lines.level = [];
@ -280,87 +361,22 @@ function REPLServer(prompt,
sawSIGINT = false;
}
self._currentStringLiteral = null;
self.lineParser.reset();
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 and the previous
// character doesn't matter anymore.
previous = null;
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 {
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);
cmd = self.lineParser.parseLine(cmd);
}
// Check to see if a REPL keyword was used. If it returns true,
@ -393,9 +409,9 @@ function REPLServer(prompt,
}
debug('eval %j', evalCmd);
self.eval(evalCmd, self.context, 'repl', finisherFn);
self.eval(evalCmd, self.context, 'repl', finish);
} else {
finisherFn(null);
finish(null);
}
function finish(e, ret) {
@ -406,7 +422,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.lineParser.reset();
self.bufferedCommand = '';
self.displayPrompt();
return;
@ -414,7 +430,7 @@ function REPLServer(prompt,
// If error was SyntaxError and not JSON.parse error
if (e) {
if (e instanceof Recoverable) {
if (e instanceof Recoverable && !self.lineParser.shouldFail) {
// Start buffering data like that:
// {
// ... x: 1
@ -423,12 +439,12 @@ function REPLServer(prompt,
self.displayPrompt();
return;
} else {
self._domain.emit('error', e);
self._domain.emit('error', e.err || e);
}
}
// Clear buffer if no SyntaxErrors
self._currentStringLiteral = null;
self.lineParser.reset();
self.bufferedCommand = '';
// If we got any output - print it (if no error)
@ -971,7 +987,7 @@ function defineDefaultCommands(repl) {
repl.defineCommand('break', {
help: 'Sometimes you get stuck, this gets you out',
action: function() {
this._currentStringLiteral = null;
this.lineParser.reset();
this.bufferedCommand = '';
this.displayPrompt();
}
@ -986,7 +1002,7 @@ function defineDefaultCommands(repl) {
repl.defineCommand('clear', {
help: clearMessage,
action: function() {
this._currentStringLiteral = null;
this.lineParser.reset();
this.bufferedCommand = '';
if (!this.useGlobal) {
this.outputStream.write('Clearing context...\n');

28
test/parallel/test-repl.js
View File

@ -250,6 +250,34 @@ function error_test() {
{ client: client_unix, send: 'function x() {\nreturn \'\\\\\';\n }',
expect: prompt_multiline + prompt_multiline +
'undefined\n' + prompt_unix },
// regression tests for https://github.com/nodejs/node/issues/3421
{ client: client_unix, send: 'function x() {\n//\'\n }',
expect: prompt_multiline + prompt_multiline +
'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x() {\n//"\n }',
expect: prompt_multiline + prompt_multiline +
'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x() {//\'\n }',
expect: prompt_multiline + 'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x() {//"\n }',
expect: prompt_multiline + 'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x() {\nvar i = "\'";\n }',
expect: prompt_multiline + prompt_multiline +
'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x(/*optional*/) {}',
expect: 'undefined\n' + prompt_unix },
{ client: client_unix, send: 'function x(/* // 5 */) {}',
expect: 'undefined\n' + prompt_unix },
{ client: client_unix, send: '// /* 5 */',
expect: 'undefined\n' + prompt_unix },
{ client: client_unix, send: '"//"',
expect: '\'//\'\n' + prompt_unix },
{ client: client_unix, send: '"data /*with*/ comment"',
expect: '\'data /*with*/ comment\'\n' + prompt_unix },
{ client: client_unix, send: 'function x(/*fn\'s optional params*/) {}',
expect: 'undefined\n' + prompt_unix },
{ client: client_unix, send: '/* \'\n"\n\'"\'\n*/',
expect: 'undefined\n' + prompt_unix },
]);
}

Write Preview
Loading…
Cancel
Save