repl: improving line continuation handling

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]>

Author: thefourtheye

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 = [];
 
@@ -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,
@@ -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,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)
@@ -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();
     }
@@ -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');
@@ -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, '\\$&');
 }

test/parallel/test-repl.js

@@ -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'/ },
   ]);
 }