REPL bug fixes

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 = [];
 
@@ -217,7 +220,7 @@ function REPLServer(prompt,
 
   self.setPrompt(prompt !== undefined ? prompt : '> ');
 
-  this.commands = {};
+  this.commands = Object.create(null);
   defineDefaultCommands(this);
 
   // figure out which "writer" function to use
@@ -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,
@@ -289,7 +357,7 @@ function REPLServer(prompt,
       }
     }
 
-    if (!skipCatchall) {
+    if (!skipCatchall && (cmd || (!cmd && self.bufferedCommand))) {
       var evalCmd = self.bufferedCommand + cmd;
       if (/^\s*\{/.test(evalCmd) && /\}\s*$/.test(evalCmd)) {
         // It's confusing for `{ a : 1 }` to be interpreted as a block
@@ -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,10 +408,16 @@ function REPLServer(prompt,
       }
 
       // Clear buffer if no SyntaxErrors
+      self._currentStringLiteral = null;
       self.bufferedCommand = '';
 
       // If we got any output - print it (if no error)
-      if (!e && (!self.ignoreUndefined || ret !== undefined)) {
+      if (!e &&
+          // When an invalid REPL command is used, error message is printed
+          // immediately. We don't have to print anything else. So, only when
+          // the second argument to this function is there, print it.
+          arguments.length === 2 &&
+          (!self.ignoreUndefined || ret !== undefined)) {
         self.context._ = ret;
         self.outputStream.write(self.writer(ret) + '\n');
       }
@@ -865,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();
     }
@@ -879,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');
@@ -944,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, '\\$&');
 }

test/parallel/test-repl.js

@@ -189,7 +189,49 @@ function error_test() {
     { client: client_unix, send: 'url.format("http://google.com")',
       expect: 'http://google.com/' },
     { client: client_unix, send: 'var path = 42; path',
-      expect: '42' }
+      expect: '42' },
+    // this makes sure that we don't print `undefined` when we actually print
+    // the error message
+    { client: client_unix, send: '.invalid_repl_command',
+      expect: 'Invalid REPL keyword\n' + prompt_unix },
+    // this makes sure that we don't crash when we use an inherited property as
+    // 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'/ },
+    // empty lines in the REPL should be allowed
+    { client: client_unix, send: '\n\r\n\r\n',
+      expect: prompt_unix + prompt_unix + prompt_unix },
+    // empty lines in the string literals should not affect the string
+    { client: client_unix, send: '\'the\\\n\\\nfourtheye\'\n',
+      expect: prompt_multiline + prompt_multiline +
+              '\'thefourtheye\'\n' + prompt_unix },
   ]);
 }