repl: Assignment of _ allowed with warning

This commit addresses #5431 by
changing the way that the repl handles assignment to the global _
variable.

Prior to this commit, node sets the result of the last expression
evaluated in the repl to `_`. This causes problems for users of
underscore, lodash and other packages where it is common to assign
`_` to the package, e.g. `_ = require('lodash');`.

Changes in this commit now result in the following behavior.

- If unassigned on the repl, `_` continues to refer to the last
  evaluated expression.
- If assigned, the default behavior of assigning `_` to the last
  evaluated expression is disabled, and `_` now references whatever
  value was explicitly set. A warning is issued on the repl -
  'expression assignment to _ now disabled'.
- If `_` is assigned multiple times, the warning is only displayed once.
- When `.clear` is executed in the repl, `_` continues to refer to its
  most recent value, whatever that is (this is per existing behavior).
  If `_` had been explicitly set prior to `.clear` it will not change
  again with the evaluation of the next expression.

PR-URL: #5535
Fixes: #5431
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: lance

doc/api/repl.markdown

@@ -86,6 +86,8 @@ The special variable `_` (underscore) contains the result of the last expression
 4
 ```
 
+Explicitly setting `_` will disable this behavior until the context is reset.
+
 The REPL provides access to any variables in the global scope. You can expose
 a variable to the REPL explicitly by assigning it to the `context` object
 associated with each `REPLServer`.  For example:

lib/repl.js

@@ -205,6 +205,8 @@ function REPLServer(prompt,
   self.useGlobal = !!useGlobal;
   self.ignoreUndefined = !!ignoreUndefined;
   self.replMode = replMode || exports.REPL_MODE_SLOPPY;
+  self.underscoreAssigned = false;
+  self.last = undefined;
 
   self._inTemplateLiteral = false;
 
@@ -471,7 +473,9 @@ function REPLServer(prompt,
           // the second argument to this function is there, print it.
           arguments.length === 2 &&
           (!self.ignoreUndefined || ret !== undefined)) {
-        self.context._ = ret;
+        if (!self.underscoreAssigned) {
+          self.last = ret;
+        }
         self.outputStream.write(self.writer(ret) + '\n');
       }
 
@@ -545,27 +549,43 @@ REPLServer.prototype.createContext = function() {
   context.module = module;
   context.require = require;
 
+
+  this.underscoreAssigned = false;
   this.lines = [];
   this.lines.level = [];
 
   // make built-in modules available directly
   // (loaded lazily)
-  exports._builtinLibs.forEach(function(name) {
+  exports._builtinLibs.forEach((name) => {
     Object.defineProperty(context, name, {
-      get: function() {
+      get: () => {
         var lib = require(name);
-        context._ = context[name] = lib;
+        this.last = context[name] = lib;
         return lib;
       },
       // allow the creation of other globals with this name
-      set: function(val) {
+      set: (val) => {
         delete context[name];
         context[name] = val;
       },
       configurable: true
     });
   });
 
+  Object.defineProperty(context, '_', {
+    configurable: true,
+    get: () => {
+      return this.last;
+    },
+    set: (value) => {
+      this.last = value;
+      if (!this.underscoreAssigned) {
+        this.underscoreAssigned = true;
+        this.outputStream.write('Expression assignment to _ now disabled.\n');
+      }
+    }
+  });
+
   return context;
 };
 

test/parallel/test-repl-underscore.js

@@ -0,0 +1,156 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const repl = require('repl');
+const stream = require('stream');
+
+testSloppyMode();
+testStrictMode();
+testResetContext();
+testMagicMode();
+
+function testSloppyMode() {
+  const r = initRepl(repl.REPL_MODE_SLOPPY);
+
+  // cannot use `let` in sloppy mode
+  r.write(`_;          // initial value undefined
+          var x = 10;  // evaluates to undefined
+          _;           // still undefined
+          y = 10;      // evaluates to 10
+          _;           // 10 from last eval
+          _ = 20;      // explicitly set to 20
+          _;           // 20 from user input
+          _ = 30;      // make sure we can set it twice and no prompt
+          _;           // 30 from user input
+          y = 40;      // make sure eval doesn't change _
+          _;           // remains 30 from user input
+          `);
+
+  assertOutput(r.output, [
+    'undefined',
+    'undefined',
+    'undefined',
+    '10',
+    '10',
+    'Expression assignment to _ now disabled.',
+    '20',
+    '20',
+    '30',
+    '30',
+    '40',
+    '30'
+  ]);
+}
+
+function testStrictMode() {
+  const r = initRepl(repl.REPL_MODE_STRICT);
+
+  r.write(`_;          // initial value undefined
+          var x = 10;  // evaluates to undefined
+          _;           // still undefined
+          let _ = 20;  // use 'let' only in strict mode - evals to undefined
+          _;           // 20 from user input
+          _ = 30;      // make sure we can set it twice and no prompt
+          _;           // 30 from user input
+          var y = 40;  // make sure eval doesn't change _
+          _;           // remains 30 from user input
+          function f() { let _ = 50; } // undefined
+          f();         // undefined
+          _;           // remains 30 from user input
+          `);
+
+  assertOutput(r.output, [
+    'undefined',
+    'undefined',
+    'undefined',
+    'undefined',
+    '20',
+    '30',
+    '30',
+    'undefined',
+    '30',
+    'undefined',
+    'undefined',
+    '30'
+  ]);
+}
+
+function testMagicMode() {
+  const r = initRepl(repl.REPL_MODE_MAGIC);
+
+  r.write(`_;          // initial value undefined
+          x = 10;      //
+          _;           // last eval - 10
+          let _ = 20;  // undefined
+          _;           // 20 from user input
+          _ = 30;      // make sure we can set it twice and no prompt
+          _;           // 30 from user input
+          var y = 40;  // make sure eval doesn't change _
+          _;           // remains 30 from user input
+          function f() { let _ = 50; return _; } // undefined
+          f();         // 50
+          _;           // remains 30 from user input
+          `);
+
+  assertOutput(r.output, [
+    'undefined',
+    '10',
+    '10',
+    'undefined',
+    '20',
+    '30',
+    '30',
+    'undefined',
+    '30',
+    'undefined',
+    '50',
+    '30'
+  ]);
+}
+
+function testResetContext() {
+  const r = initRepl(repl.REPL_MODE_SLOPPY);
+
+  r.write(`_ = 10;     // explicitly set to 10
+          _;           // 10 from user input
+          .clear       // Clearing context...
+          _;           // remains 10
+          x = 20;      // but behavior reverts to last eval
+          _;           // expect 20
+          `);
+
+  assertOutput(r.output, [
+    'Expression assignment to _ now disabled.',
+    '10',
+    '10',
+    'Clearing context...',
+    '10',
+    '20',
+    '20'
+  ]);
+}
+
+function initRepl(mode) {
+  const inputStream = new stream.PassThrough();
+  const outputStream = new stream.PassThrough();
+  outputStream.accum = '';
+
+  outputStream.on('data', (data) => {
+    outputStream.accum += data;
+  });
+
+  return repl.start({
+    input: inputStream,
+    output: outputStream,
+    useColors: false,
+    terminal: false,
+    prompt: '',
+    replMode: mode
+  });
+}
+
+function assertOutput(output, expected) {
+  const lines = output.accum.trim().split('\n');
+  assert.deepStrictEqual(lines, expected);
+}