Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repl: Assignment of _ allowed with warning #5535

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/api/repl.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
30 changes: 25 additions & 5 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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;
};

Expand Down
156 changes: 156 additions & 0 deletions test/parallel/test-repl-underscore.js
Original file line number Diff line number Diff line change
@@ -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'
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Care to explain why expression assignment to _ now disabled is not printed in strict/magic modes? I think I'm missing something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's complicated in this case. The behavior of let is such that the setter on an object is not called in a scenario like this. Because of the way runInContext and runInThisContext work, the behavior is going to be something similar to this: https://gist.github.com/lance/2b2a6a8286b4bb52ddcf. If you run this script, the output is:

$ node test-repl-use-global.js
some value
some value
Setting 'val' a third value
a third value
Setting 'val' a fourth value
a fourth value

Notice that a "Setting 'val'..." message is not printed for when a let statement is used.

** EDIT **
I have updated the gist with a bunch of comments and examples showing what happens with let, this, and var. The behavior in the REPL is, I believe, consistent with what you should expect.

}

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);
}