repl: show reference errors during preview

This aligns the behavior with the one in the Firefox console. It will visualize ReferenceErrors in case the input has no possible completion and no buffered input. That way typos can already be highlighted before being evaluated. Signed-off-by: Ruben Bridgewater <[email protected]> PR-URL: #33282 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · May 16, 2020 · d33dcf1 · d33dcf1
1 parent 1a9771a
commit d33dcf1
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 9 deletions.
diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
@@ -48,6 +48,8 @@ const previewOptions = {
   showHidden: false
 };
 
+const REPL_MODE_STRICT = Symbol('repl-strict');
+
 // If the error is that we've unexpectedly ended the input,
 // then let the user try to recover by adding more input.
 // Note: `e` (the original exception) is not used by the current implementation,
@@ -136,6 +138,8 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
   let previewCompletionCounter = 0;
   let completionPreview = null;
 
+  let hasCompletions = false;
+
   let wrapped = false;
 
   let escaped = null;
@@ -229,6 +233,8 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         return;
       }
 
+      hasCompletions = true;
+
       // If there is a common prefix to all matches, then apply that portion.
       const completions = rawCompletions.filter((e) => e);
       const prefix = commonPrefix(completions);
@@ -265,6 +271,12 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
     });
   }
 
+  function isInStrictMode(repl) {
+    return repl.replMode === REPL_MODE_STRICT || process.execArgv
+      .map((e) => e.toLowerCase().replace(/_/g, '-'))
+      .includes('--use-strict');
+  }
+
   // This returns a code preview for arbitrary input code.
   function getInputPreview(input, callback) {
     // For similar reasons as `defaultEval`, wrap expressions starting with a
@@ -292,8 +304,11 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         // may be inspected.
         } else if (preview.exceptionDetails &&
                    (result.className === 'EvalError' ||
-                     result.className === 'SyntaxError' ||
-                     result.className === 'ReferenceError')) {
+                    result.className === 'SyntaxError' ||
+                    // Report ReferenceError in case the strict mode is active
+                    // for input that has no completions.
+                    (result.className === 'ReferenceError' &&
+                     (hasCompletions || !isInStrictMode(repl))))) {
           callback(null, null);
         } else if (result.objectId) {
           // The writer options might change and have influence on the inspect
@@ -339,6 +354,8 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
       return;
     }
 
+    hasCompletions = false;
+
     // Add the autocompletion preview.
     const insertPreview = false;
     showCompletionPreview(repl.line, insertPreview);
@@ -703,6 +720,8 @@ function setupReverseSearch(repl) {
 }
 
 module.exports = {
+  REPL_MODE_SLOPPY: Symbol('repl-sloppy'),
+  REPL_MODE_STRICT,
   isRecoverableError,
   kStandaloneREPL: Symbol('kStandaloneREPL'),
   setupPreview,

diff --git a/lib/repl.js b/lib/repl.js
@@ -103,6 +103,8 @@ const experimentalREPLAwait = require('internal/options').getOptionValue(
   '--experimental-repl-await'
 );
 const {
+  REPL_MODE_SLOPPY,
+  REPL_MODE_STRICT,
   isRecoverableError,
   kStandaloneREPL,
   setupPreview,
@@ -363,8 +365,7 @@ function REPLServer(prompt,
     }
     while (true) {
       try {
-        if (!/^\s*$/.test(code) &&
-            self.replMode === exports.REPL_MODE_STRICT) {
+        if (self.replMode === exports.REPL_MODE_STRICT && !/^\s*$/.test(code)) {
           // "void 0" keeps the repl from returning "use strict" as the result
           // value for statements and declarations that don't return a value.
           code = `'use strict'; void 0;\n${code}`;
@@ -896,8 +897,8 @@ ObjectSetPrototypeOf(REPLServer, Interface);
 
 exports.REPLServer = REPLServer;
 
-exports.REPL_MODE_SLOPPY = Symbol('repl-sloppy');
-exports.REPL_MODE_STRICT = Symbol('repl-strict');
+exports.REPL_MODE_SLOPPY = REPL_MODE_SLOPPY;
+exports.REPL_MODE_STRICT = REPL_MODE_STRICT;
 
 // Prompt is a string to print on each line for the prompt,
 // source is a stream to use for I/O, defaulting to stdin/stdout.

diff --git a/test/parallel/test-repl-mode.js b/test/parallel/test-repl-mode.js
@@ -7,7 +7,8 @@ const repl = require('repl');
 const tests = [
   testSloppyMode,
   testStrictMode,
-  testAutoMode
+  testAutoMode,
+  testStrictModeTerminal,
 ];
 
 tests.forEach(function(test) {
@@ -37,6 +38,18 @@ function testStrictMode() {
   assert.strictEqual(cli.output.accumulator.join(''), 'undefined\n> ');
 }
 
+function testStrictModeTerminal() {
+  // Verify that ReferenceErrors are reported in strict mode previews.
+  const cli = initRepl(repl.REPL_MODE_STRICT, {
+    terminal: true
+  });
+
+  cli.input.emit('data', 'xyz ');
+  assert.ok(
+    cli.output.accumulator.includes('\n// ReferenceError: xyz is not defined')
+  );
+}
+
 function testAutoMode() {
   const cli = initRepl(repl.REPL_MODE_MAGIC);
 
@@ -48,7 +61,7 @@ function testAutoMode() {
   assert.strictEqual(cli.output.accumulator.join(''), 'undefined\n> ');
 }
 
-function initRepl(mode) {
+function initRepl(mode, options) {
   const input = new Stream();
   input.write = input.pause = input.resume = () => {};
   input.readable = true;
@@ -65,6 +78,7 @@ function initRepl(mode) {
     output: output,
     useColors: false,
     terminal: false,
-    replMode: mode
+    replMode: mode,
+    ...options
   });
 }
diff --git a/test/parallel/test-repl-strict-mode-previews.js b/test/parallel/test-repl-strict-mode-previews.js
@@ -0,0 +1,46 @@
+// Previews in strict mode should indicate ReferenceErrors.
+
+'use strict';
+
+const common = require('../common');
+
+common.skipIfInspectorDisabled();
+
+if (process.argv[2] === 'child') {
+  const stream = require('stream');
+  const repl = require('repl');
+  class ActionStream extends stream.Stream {
+    readable = true;
+    run(data) {
+      this.emit('data', `${data}`);
+      this.emit('keypress', '', { ctrl: true, name: 'd' });
+    }
+    resume() {}
+    pause() {}
+  }
+
+  repl.start({
+    input: new ActionStream(),
+    output: new stream.Writable({
+      write(chunk, _, next) {
+        console.log(chunk.toString());
+        next();
+      }
+    }),
+    useColors: false,
+    terminal: true
+  }).inputStream.run('xyz');
+} else {
+  const assert = require('assert');
+  const { spawnSync } = require('child_process');
+
+  const result = spawnSync(
+    process.execPath,
+    ['--use-strict', `${__filename}`, 'child']
+  );
+
+  assert.match(
+    result.stdout.toString(),
+    /\/\/ ReferenceError: xyz is not defined/
+  );
+}