Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8a2c4ae

Browse filesBrowse files
thefourtheyejasnell
thefourtheye
authored and
jasnell
committed
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: #3421 PR-URL: #3515 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
1 parent 8d78d68 commit 8a2c4ae
Copy full SHA for 8a2c4ae

File tree

Expand file treeCollapse file tree

2 files changed

+122
-78
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+122
-78
lines changed
Open diff view settings
Collapse file

‎lib/repl.js‎

Copy file name to clipboardExpand all lines: lib/repl.js
+94-78Lines changed: 94 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,88 @@ const BLOCK_SCOPED_ERROR = 'Block-scoped declarations (let, ' +
7070
'const, function, class) not yet supported outside strict mode';
7171

7272

73+
class LineParser {
74+
75+
constructor() {
76+
this.reset();
77+
}
78+
79+
reset() {
80+
this._literal = null;
81+
this.shouldFail = false;
82+
this.blockComment = false;
83+
}
84+
85+
parseLine(line) {
86+
var previous = null;
87+
this.shouldFail = false;
88+
const wasWithinStrLiteral = this._literal !== null;
89+
90+
for (const current of line) {
91+
if (previous === '\\') {
92+
// valid escaping, skip processing. previous doesn't matter anymore
93+
previous = null;
94+
continue;
95+
}
96+
97+
if (!this._literal) {
98+
if (previous === '*' && current === '/') {
99+
if (this.blockComment) {
100+
this.blockComment = false;
101+
previous = null;
102+
continue;
103+
} else {
104+
this.shouldFail = true;
105+
break;
106+
}
107+
}
108+
109+
// ignore rest of the line if `current` and `previous` are `/`s
110+
if (previous === current && previous === '/' && !this.blockComment) {
111+
break;
112+
}
113+
114+
if (previous === '/' && current === '*') {
115+
this.blockComment = true;
116+
previous = null;
117+
}
118+
}
119+
120+
if (this.blockComment) continue;
121+
122+
if (current === this._literal) {
123+
this._literal = null;
124+
} else if (current === '\'' || current === '"') {
125+
this._literal = this._literal || current;
126+
}
127+
128+
previous = current;
129+
}
130+
131+
const isWithinStrLiteral = this._literal !== null;
132+
133+
if (!wasWithinStrLiteral && !isWithinStrLiteral) {
134+
// Current line has nothing to do with String literals, trim both ends
135+
line = line.trim();
136+
} else if (wasWithinStrLiteral && !isWithinStrLiteral) {
137+
// was part of a string literal, but it is over now, trim only the end
138+
line = line.trimRight();
139+
} else if (isWithinStrLiteral && !wasWithinStrLiteral) {
140+
// was not part of a string literal, but it is now, trim only the start
141+
line = line.trimLeft();
142+
}
143+
144+
const lastChar = line.charAt(line.length - 1);
145+
146+
this.shouldFail = this.shouldFail ||
147+
((!this._literal && lastChar === '\\') ||
148+
(this._literal && lastChar !== '\\'));
149+
150+
return line;
151+
}
152+
}
153+
154+
73155
function REPLServer(prompt,
74156
stream,
75157
eval_,
@@ -193,7 +275,7 @@ function REPLServer(prompt,
193275
debug('domain error');
194276
const top = replMap.get(self);
195277
top.outputStream.write((e.stack || e) + '\n');
196-
top._currentStringLiteral = null;
278+
top.lineParser.reset();
197279
top.bufferedCommand = '';
198280
top.lines.level = [];
199281
top.displayPrompt();
@@ -220,8 +302,7 @@ function REPLServer(prompt,
220302
self.outputStream = output;
221303

222304
self.resetContext();
223-
// Initialize the current string literal found, to be null
224-
self._currentStringLiteral = null;
305+
self.lineParser = new LineParser();
225306
self.bufferedCommand = '';
226307
self.lines.level = [];
227308

@@ -280,87 +361,22 @@ function REPLServer(prompt,
280361
sawSIGINT = false;
281362
}
282363

283-
self._currentStringLiteral = null;
364+
self.lineParser.reset();
284365
self.bufferedCommand = '';
285366
self.lines.level = [];
286367
self.displayPrompt();
287368
});
288369

289-
function parseLine(line, currentStringLiteral) {
290-
var previous = null, current = null;
291-
292-
for (var i = 0; i < line.length; i += 1) {
293-
if (previous === '\\') {
294-
// if it is a valid escaping, then skip processing and the previous
295-
// character doesn't matter anymore.
296-
previous = null;
297-
continue;
298-
}
299-
300-
current = line.charAt(i);
301-
if (current === currentStringLiteral) {
302-
currentStringLiteral = null;
303-
} else if (current === '\'' ||
304-
current === '"' &&
305-
currentStringLiteral === null) {
306-
currentStringLiteral = current;
307-
}
308-
previous = current;
309-
}
310-
311-
return currentStringLiteral;
312-
}
313-
314-
function getFinisherFunction(cmd, defaultFn) {
315-
if ((self._currentStringLiteral === null &&
316-
cmd.charAt(cmd.length - 1) === '\\') ||
317-
(self._currentStringLiteral !== null &&
318-
cmd.charAt(cmd.length - 1) !== '\\')) {
319-
320-
// If the line continuation is used outside string literal or if the
321-
// string continuation happens with out line continuation, then fail hard.
322-
// Even if the error is recoverable, get the underlying error and use it.
323-
return function(e, ret) {
324-
var error = e instanceof Recoverable ? e.err : e;
325-
326-
if (arguments.length === 2) {
327-
// using second argument only if it is actually passed. Otherwise
328-
// `undefined` will be printed when invalid REPL commands are used.
329-
return defaultFn(error, ret);
330-
}
331-
332-
return defaultFn(error);
333-
};
334-
}
335-
return defaultFn;
336-
}
337-
338370
self.on('line', function(cmd) {
339371
debug('line %j', cmd);
340372
sawSIGINT = false;
341373
var skipCatchall = false;
342-
var finisherFn = finish;
343374

344375
// leading whitespaces in template literals should not be trimmed.
345376
if (self._inTemplateLiteral) {
346377
self._inTemplateLiteral = false;
347378
} else {
348-
const wasWithinStrLiteral = self._currentStringLiteral !== null;
349-
self._currentStringLiteral = parseLine(cmd, self._currentStringLiteral);
350-
const isWithinStrLiteral = self._currentStringLiteral !== null;
351-
352-
if (!wasWithinStrLiteral && !isWithinStrLiteral) {
353-
// Current line has nothing to do with String literals, trim both ends
354-
cmd = cmd.trim();
355-
} else if (wasWithinStrLiteral && !isWithinStrLiteral) {
356-
// was part of a string literal, but it is over now, trim only the end
357-
cmd = cmd.trimRight();
358-
} else if (isWithinStrLiteral && !wasWithinStrLiteral) {
359-
// was not part of a string literal, but it is now, trim only the start
360-
cmd = cmd.trimLeft();
361-
}
362-
363-
finisherFn = getFinisherFunction(cmd, finish);
379+
cmd = self.lineParser.parseLine(cmd);
364380
}
365381

366382
// Check to see if a REPL keyword was used. If it returns true,
@@ -393,9 +409,9 @@ function REPLServer(prompt,
393409
}
394410

395411
debug('eval %j', evalCmd);
396-
self.eval(evalCmd, self.context, 'repl', finisherFn);
412+
self.eval(evalCmd, self.context, 'repl', finish);
397413
} else {
398-
finisherFn(null);
414+
finish(null);
399415
}
400416

401417
function finish(e, ret) {
@@ -406,15 +422,15 @@ function REPLServer(prompt,
406422
self.outputStream.write('npm should be run outside of the ' +
407423
'node repl, in your normal shell.\n' +
408424
'(Press Control-D to exit.)\n');
409-
self._currentStringLiteral = null;
425+
self.lineParser.reset();
410426
self.bufferedCommand = '';
411427
self.displayPrompt();
412428
return;
413429
}
414430

415431
// If error was SyntaxError and not JSON.parse error
416432
if (e) {
417-
if (e instanceof Recoverable) {
433+
if (e instanceof Recoverable && !self.lineParser.shouldFail) {
418434
// Start buffering data like that:
419435
// {
420436
// ... x: 1
@@ -423,12 +439,12 @@ function REPLServer(prompt,
423439
self.displayPrompt();
424440
return;
425441
} else {
426-
self._domain.emit('error', e);
442+
self._domain.emit('error', e.err || e);
427443
}
428444
}
429445

430446
// Clear buffer if no SyntaxErrors
431-
self._currentStringLiteral = null;
447+
self.lineParser.reset();
432448
self.bufferedCommand = '';
433449

434450
// If we got any output - print it (if no error)
@@ -971,7 +987,7 @@ function defineDefaultCommands(repl) {
971987
repl.defineCommand('break', {
972988
help: 'Sometimes you get stuck, this gets you out',
973989
action: function() {
974-
this._currentStringLiteral = null;
990+
this.lineParser.reset();
975991
this.bufferedCommand = '';
976992
this.displayPrompt();
977993
}
@@ -986,7 +1002,7 @@ function defineDefaultCommands(repl) {
9861002
repl.defineCommand('clear', {
9871003
help: clearMessage,
9881004
action: function() {
989-
this._currentStringLiteral = null;
1005+
this.lineParser.reset();
9901006
this.bufferedCommand = '';
9911007
if (!this.useGlobal) {
9921008
this.outputStream.write('Clearing context...\n');
Collapse file

‎test/parallel/test-repl.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-repl.js
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,34 @@ function error_test() {
250250
{ client: client_unix, send: 'function x() {\nreturn \'\\\\\';\n }',
251251
expect: prompt_multiline + prompt_multiline +
252252
'undefined\n' + prompt_unix },
253+
// regression tests for https://github.com/nodejs/node/issues/3421
254+
{ client: client_unix, send: 'function x() {\n//\'\n }',
255+
expect: prompt_multiline + prompt_multiline +
256+
'undefined\n' + prompt_unix },
257+
{ client: client_unix, send: 'function x() {\n//"\n }',
258+
expect: prompt_multiline + prompt_multiline +
259+
'undefined\n' + prompt_unix },
260+
{ client: client_unix, send: 'function x() {//\'\n }',
261+
expect: prompt_multiline + 'undefined\n' + prompt_unix },
262+
{ client: client_unix, send: 'function x() {//"\n }',
263+
expect: prompt_multiline + 'undefined\n' + prompt_unix },
264+
{ client: client_unix, send: 'function x() {\nvar i = "\'";\n }',
265+
expect: prompt_multiline + prompt_multiline +
266+
'undefined\n' + prompt_unix },
267+
{ client: client_unix, send: 'function x(/*optional*/) {}',
268+
expect: 'undefined\n' + prompt_unix },
269+
{ client: client_unix, send: 'function x(/* // 5 */) {}',
270+
expect: 'undefined\n' + prompt_unix },
271+
{ client: client_unix, send: '// /* 5 */',
272+
expect: 'undefined\n' + prompt_unix },
273+
{ client: client_unix, send: '"//"',
274+
expect: '\'//\'\n' + prompt_unix },
275+
{ client: client_unix, send: '"data /*with*/ comment"',
276+
expect: '\'data /*with*/ comment\'\n' + prompt_unix },
277+
{ client: client_unix, send: 'function x(/*fn\'s optional params*/) {}',
278+
expect: 'undefined\n' + prompt_unix },
279+
{ client: client_unix, send: '/* \'\n"\n\'"\'\n*/',
280+
expect: 'undefined\n' + prompt_unix },
253281
]);
254282
}
255283

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.