Skip to content

Commit

Permalink
repl: improving line continuation handling
Browse files Browse the repository at this point in the history 
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: #2163
Reviewed-By: Jeremiah Senkpiel <[email protected]>
  • Loading branch information
@thefourtheye
thefourtheye committed Jul 24, 2015
1 parent 30edb5a commit 81ea52a
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 15 deletions.
90 changes: 75 additions & 15 deletions lib/repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 = [];

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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();
}
Expand All @@ -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');
Expand Down Expand Up @@ -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, '\\$&');
}
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'/ },
]);
}

Expand Down

0 comments on commit 81ea52a

Please sign in to comment.