Skip to content

Commit 5f656db

Browse files
thefourtheyethefourtheye
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 string literals and the error is thrown in REPL. This patch refactors the existing logic and groups everything in a class. Fixes: #3421
1 parent f4c0ed4 commit 5f656db

File tree

2 files changed

+124
-77
lines changed

2 files changed

+124
-77
lines changed

‎lib/repl.js‎

Lines changed: 96 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,90 @@ 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, current = null;
87+
this.shouldFail = false;
88+
const wasWithinStrLiteral = this._literal !== null;
89+
90+
for (let i = 0; i < line.length; i += 1) {
91+
if (previous === '\\') {
92+
// valid escaping, skip processing. previous doesn't matter anymore
93+
previous = null;
94+
continue;
95+
}
96+
97+
current = line.charAt(i);
98+
99+
if (!this._literal) {
100+
if (previous === '*' && current === '/') {
101+
if (this.blockComment) {
102+
this.blockComment = false;
103+
previous = null;
104+
continue;
105+
} else {
106+
this.shouldFail = true;
107+
break;
108+
}
109+
}
110+
111+
// ignore rest of the line if `current` and `previous` are `/`s
112+
if (previous === current && previous === '/' && !this.blockComment) {
113+
break;
114+
}
115+
116+
if (previous === '/' && current === '*') {
117+
this.blockComment = true;
118+
previous = null;
119+
}
120+
}
121+
122+
if (this.blockComment) continue;
123+
124+
if (current === this._literal) {
125+
this._literal = null;
126+
} else if (current === '\'' || current === '"') {
127+
this._literal = this._literal || current;
128+
}
129+
130+
previous = current;
131+
}
132+
133+
const isWithinStrLiteral = this._literal !== null;
134+
135+
if (!wasWithinStrLiteral && !isWithinStrLiteral) {
136+
// Current line has nothing to do with String literals, trim both ends
137+
line = line.trim();
138+
} else if (wasWithinStrLiteral && !isWithinStrLiteral) {
139+
// was part of a string literal, but it is over now, trim only the end
140+
line = line.trimRight();
141+
} else if (isWithinStrLiteral && !wasWithinStrLiteral) {
142+
// was not part of a string literal, but it is now, trim only the start
143+
line = line.trimLeft();
144+
}
145+
146+
const lastChar = line.charAt(line.length - 1);
147+
148+
this.shouldFail = this.shouldFail ||
149+
((!this._literal && lastChar === '\\') ||
150+
(this._literal && lastChar !== '\\'));
151+
152+
return line;
153+
}
154+
}
155+
156+
73157
function REPLServer(prompt,
74158
stream,
75159
eval_,
@@ -193,7 +277,7 @@ function REPLServer(prompt,
193277
debug('domain error');
194278
const top = replMap.get(self);
195279
top.outputStream.write((e.stack || e) + '\n');
196-
top._currentStringLiteral = null;
280+
top.lineParser.reset();
197281
top.bufferedCommand = '';
198282
top.lines.level = [];
199283
top.displayPrompt();
@@ -221,7 +305,7 @@ function REPLServer(prompt,
221305

222306
self.resetContext();
223307
// Initialize the current string literal found, to be null
224-
self._currentStringLiteral = null;
308+
self.lineParser = new LineParser();
225309
self.bufferedCommand = '';
226310
self.lines.level = [];
227311

@@ -280,87 +364,22 @@ function REPLServer(prompt,
280364
sawSIGINT = false;
281365
}
282366

283-
self._currentStringLiteral = null;
367+
self.lineParser.reset();
284368
self.bufferedCommand = '';
285369
self.lines.level = [];
286370
self.displayPrompt();
287371
});
288372

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-
338373
self.on('line', function(cmd) {
339374
debug('line %j', cmd);
340375
sawSIGINT = false;
341376
var skipCatchall = false;
342-
var finisherFn = finish;
343377

344378
// leading whitespaces in template literals should not be trimmed.
345379
if (self._inTemplateLiteral) {
346380
self._inTemplateLiteral = false;
347381
} 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);
382+
cmd = self.lineParser.parseLine(cmd);
364383
}
365384

366385
// Check to see if a REPL keyword was used. If it returns true,
@@ -393,9 +412,9 @@ function REPLServer(prompt,
393412
}
394413

395414
debug('eval %j', evalCmd);
396-
self.eval(evalCmd, self.context, 'repl', finisherFn);
415+
self.eval(evalCmd, self.context, 'repl', finish);
397416
} else {
398-
finisherFn(null);
417+
finish(null);
399418
}
400419

401420
function finish(e, ret) {
@@ -406,15 +425,15 @@ function REPLServer(prompt,
406425
self.outputStream.write('npm should be run outside of the ' +
407426
'node repl, in your normal shell.\n' +
408427
'(Press Control-D to exit.)\n');
409-
self._currentStringLiteral = null;
428+
self.lineParser.reset();
410429
self.bufferedCommand = '';
411430
self.displayPrompt();
412431
return;
413432
}
414433

415434
// If error was SyntaxError and not JSON.parse error
416435
if (e) {
417-
if (e instanceof Recoverable) {
436+
if (e instanceof Recoverable && !self.lineParser.shouldFail) {
418437
// Start buffering data like that:
419438
// {
420439
// ... x: 1
@@ -423,12 +442,12 @@ function REPLServer(prompt,
423442
self.displayPrompt();
424443
return;
425444
} else {
426-
self._domain.emit('error', e);
445+
self._domain.emit('error', e.err || e);
427446
}
428447
}
429448

430449
// Clear buffer if no SyntaxErrors
431-
self._currentStringLiteral = null;
450+
self.lineParser.reset();
432451
self.bufferedCommand = '';
433452

434453
// If we got any output - print it (if no error)
@@ -985,7 +1004,7 @@ function defineDefaultCommands(repl) {
9851004
repl.defineCommand('break', {
9861005
help: 'Sometimes you get stuck, this gets you out',
9871006
action: function() {
988-
this._currentStringLiteral = null;
1007+
this.lineParser.reset();
9891008
this.bufferedCommand = '';
9901009
this.displayPrompt();
9911010
}
@@ -1000,7 +1019,7 @@ function defineDefaultCommands(repl) {
10001019
repl.defineCommand('clear', {
10011020
help: clearMessage,
10021021
action: function() {
1003-
this._currentStringLiteral = null;
1022+
this.lineParser.reset();
10041023
this.bufferedCommand = '';
10051024
if (!this.useGlobal) {
10061025
this.outputStream.write('Clearing context...\n');

‎test/parallel/test-repl.js‎

Lines 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)