repl: Catch syntax errors better
authorisaacs <i@izs.me>
Wed, 28 Aug 2013 01:53:39 +0000 (18:53 -0700)
committerisaacs <i@izs.me>
Wed, 4 Sep 2013 18:13:41 +0000 (11:13 -0700)
Replace the growing list of 'isSyntaxError' whackamole conditions with a
smarter approach.  This creates a vm Script object *first*, which will
parse the code and raise a SyntaxError right away.

We still do need the test function, but only because strict mode syntax
errors are not recoverable, and should be raised right away.  Really, we
should probably *only* continue on "unexpected end of input" SyntaxErrors.

Also fixes a very difficult-to-test nit where the '...' indentation is
not properly cleared when you ^C out of a syntax error.

Closes #6093

lib/repl.js
test/simple/test-repl.js

index 82a3c70..c46db5e 100644 (file)
@@ -111,22 +111,29 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
 
   function defaultEval(code, context, file, cb) {
     var err, result;
+    // first, create the Script object to check the syntax
     try {
-      if (self.useGlobal) {
-        result = vm.runInThisContext(code, {
-          filename: file,
-          displayErrors: false
-        });
-      } else {
-        result = vm.runInContext(code, context, {
-          filename: file,
-          displayErrors: false
-        });
-      }
+      var script = vm.createScript(code, {
+        filename: file,
+        displayErrors: false
+      });
     } catch (e) {
       err = e;
+      err._isSyntaxError = isSyntaxError(err);
     }
-    if (err && process.domain && !isSyntaxError(err)) {
+    if (!err) {
+      try {
+        if (self.useGlobal) {
+          result = script.runInThisContext({ displayErrors: false });
+        } else {
+          result = script.runInContext(context, { displayErrors: false });
+        }
+      } catch (e) {
+        err = e;
+        err._isSyntaxError = false;
+      }
+    }
+    if (err && process.domain && !err._isSyntaxError) {
       process.domain.emit('error', err);
       process.domain.exit();
     }
@@ -140,6 +147,7 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
   self._domain.on('error', function(e) {
     self.outputStream.write((e.stack || e) + '\n');
     self.bufferedCommand = '';
+    self.lines.level = [];
     self.displayPrompt();
   });
 
@@ -165,6 +173,7 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
 
   self.resetContext();
   self.bufferedCommand = '';
+  self.lines.level = [];
 
   self.prompt = !util.isUndefined(prompt) ? prompt : '> ';
 
@@ -222,6 +231,7 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
     }
 
     self.bufferedCommand = '';
+    self.lines.level = [];
     self.displayPrompt();
   });
 
@@ -260,7 +270,7 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
                 self.context,
                 'repl',
                 function(e, ret) {
-            if (e && !isSyntaxError(e)) return finish(e);
+            if (e && !e._isSyntaxError) return finish(e);
 
             if (util.isFunction(ret) &&
                 /^[\r\n\s]*function/.test(evalCmd) || e) {
@@ -280,7 +290,7 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
       self.memory(cmd);
 
       // If error was SyntaxError and not JSON.parse error
-      if (isSyntaxError(e)) {
+      if (e && e._isSyntaxError) {
         if (!self.bufferedCommand && cmd.trim().match(/^npm /)) {
           self.outputStream.write('npm should be run outside of the ' +
                                   'node repl, in your normal shell.\n' +
@@ -932,18 +942,12 @@ REPLServer.prototype.convertToContext = function(cmd) {
 
 /**
  * Returns `true` if "e" is a SyntaxError, `false` otherwise.
- * This function filters out false positives likes JSON.parse() errors and
- * RegExp syntax errors.
+ * filters out strict-mode errors, which are not recoverable
  */
 function isSyntaxError(e) {
   // Convert error to string
   e = e && (e.stack || e.toString());
   return e && e.match(/^SyntaxError/) &&
-      // RegExp syntax error
-      !e.match(/^SyntaxError: Invalid regular expression/) &&
-      !e.match(/^SyntaxError: Invalid flags supplied to RegExp constructor/) &&
       // "strict mode" syntax errors
-      !e.match(/^SyntaxError: .*strict mode.*/i) &&
-      // JSON.parse() error
-      !e.match(/\n {4}at Object.parse \(native\)\n/);
+      !e.match(/^SyntaxError: .*strict mode.*/i);
 }
index 622ee41..52429da 100644 (file)
@@ -40,7 +40,7 @@ var net = require('net'),
 // absolute path to test/fixtures/a.js
 var moduleFilename = require('path').join(common.fixturesDir, 'a');
 
-common.error('repl test');
+console.error('repl test');
 
 // function for REPL to run
 invoke_me = function(arg) {
@@ -51,7 +51,7 @@ function send_expect(list) {
   if (list.length > 0) {
     var cur = list.shift();
 
-    common.error('sending ' + JSON.stringify(cur.send));
+    console.error('sending ' + JSON.stringify(cur.send));
 
     cur.client.expect = cur.expect;
     cur.client.list = list;
@@ -74,7 +74,7 @@ function error_test() {
 
   client_unix.on('data', function(data) {
     read_buffer += data.toString('ascii', 0, data.length);
-    common.error('Unix data: ' + JSON.stringify(read_buffer) + ', expecting ' +
+    console.error('Unix data: ' + JSON.stringify(read_buffer) + ', expecting ' +
                  (client_unix.expect.exec ?
                   client_unix.expect :
                   JSON.stringify(client_unix.expect)));
@@ -83,13 +83,13 @@ function error_test() {
       // if it's an exact match, then don't do the regexp
       if (read_buffer !== client_unix.expect) {
         assert.ok(read_buffer.match(client_unix.expect));
-        common.error('match');
+        console.error('match');
       }
       read_buffer = '';
       if (client_unix.list && client_unix.list.length > 0) {
         send_expect(client_unix.list);
       } else {
-        common.error('End of Error test, running TCP test.');
+        console.error('End of Error test, running TCP test.');
         tcp_test();
       }
 
@@ -100,12 +100,12 @@ function error_test() {
       if (client_unix.list && client_unix.list.length > 0) {
         send_expect(client_unix.list);
       } else {
-        common.error('End of Error test, running TCP test.\n');
+        console.error('End of Error test, running TCP test.\n');
         tcp_test();
       }
 
     } else {
-      common.error('didn\'t see prompt yet, buffering.');
+      console.error('didn\'t see prompt yet, buffering.');
     }
   });
 
@@ -119,6 +119,9 @@ function error_test() {
     // You can recover with the .break command
     { client: client_unix, send: '.break',
       expect: prompt_unix },
+    // But passing the same string to eval() should throw
+    { client: client_unix, send: 'eval("function test_func() {")',
+      expect: /^SyntaxError: Unexpected end of input/ },
     // Floating point numbers are not interpreted as REPL commands.
     { client: client_unix, send: '.1234',
       expect: '0.1234' },
@@ -233,20 +236,20 @@ function tcp_test() {
 
     client_tcp.on('data', function(data) {
       read_buffer += data.toString('ascii', 0, data.length);
-      common.error('TCP data: ' + JSON.stringify(read_buffer) +
+      console.error('TCP data: ' + JSON.stringify(read_buffer) +
                    ', expecting ' + JSON.stringify(client_tcp.expect));
       if (read_buffer.indexOf(prompt_tcp) !== -1) {
         assert.strictEqual(client_tcp.expect, read_buffer);
-        common.error('match');
+        console.error('match');
         read_buffer = '';
         if (client_tcp.list && client_tcp.list.length > 0) {
           send_expect(client_tcp.list);
         } else {
-          common.error('End of TCP test.\n');
+          console.error('End of TCP test.\n');
           clean_up();
         }
       } else {
-        common.error('didn\'t see prompt yet, buffering');
+        console.error('didn\'t see prompt yet, buffering');
       }
     });
 
@@ -302,20 +305,20 @@ function unix_test() {
 
     client_unix.on('data', function(data) {
       read_buffer += data.toString('ascii', 0, data.length);
-      common.error('Unix data: ' + JSON.stringify(read_buffer) +
+      console.error('Unix data: ' + JSON.stringify(read_buffer) +
                    ', expecting ' + JSON.stringify(client_unix.expect));
       if (read_buffer.indexOf(prompt_unix) !== -1) {
         assert.strictEqual(client_unix.expect, read_buffer);
-        common.error('match');
+        console.error('match');
         read_buffer = '';
         if (client_unix.list && client_unix.list.length > 0) {
           send_expect(client_unix.list);
         } else {
-          common.error('End of Unix test, running Error test.\n');
+          console.error('End of Unix test, running Error test.\n');
           process.nextTick(error_test);
         }
       } else {
-        common.error('didn\'t see prompt yet, buffering.');
+        console.error('didn\'t see prompt yet, buffering.');
       }
     });