From ad8257fa5b9795d3d79fa4a91d0f18c43f024ab3 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Wed, 2 Mar 2016 16:45:16 -0500 Subject: [PATCH] repl: Assignment of _ allowed with warning This commit addresses https://github.com/nodejs/node/issues/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: https://github.com/nodejs/node/pull/5535 Fixes: https://github.com/nodejs/node/issues/5431 Reviewed-By: Roman Reiss Reviewed-By: James M Snell --- doc/api/repl.markdown | 2 + lib/repl.js | 30 ++++- test/parallel/test-repl-underscore.js | 156 ++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-repl-underscore.js diff --git a/doc/api/repl.markdown b/doc/api/repl.markdown index e9bf877e8a..f77833ea47 100644 --- a/doc/api/repl.markdown +++ b/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: diff --git a/lib/repl.js b/lib/repl.js index ab9f1bf56d..e95e9b636d 100644 --- a/lib/repl.js +++ b/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,20 +549,22 @@ 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; }, @@ -566,6 +572,20 @@ REPLServer.prototype.createContext = function() { }); }); + 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; }; diff --git a/test/parallel/test-repl-underscore.js b/test/parallel/test-repl-underscore.js new file mode 100644 index 0000000000..97fc508ad9 --- /dev/null +++ b/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); +}