repl: Enable tab completion for global properties

lance · Fishrock123 · commit c39f6c02041c · 2016-07-05T22:15:47.000+02:00
When `useGlobal` is false, tab completion in the repl does not enumerate global properties. Instead of just setting these properties blindly on the global context, e.g. context[prop] = global[prop] Use `Object.defineProperty` and the property descriptor found on `global` for the new property in `context`. Also addresses a previously unnoticed issue where `console` is writable when `useGlobal` is false. If the binary has been built with `./configure --without-intl` then the `Intl` builtin type will not be available in a repl runtime. Check for this in the test. Fixes: #7353 PR-URL: #7369 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/lib/repl.js b/lib/repl.js
@@ -39,6 +39,16 @@ const debug = util.debuglog('repl');
 const parentModule = module;
 const replMap = new WeakMap();
 
+const GLOBAL_OBJECT_PROPERTIES = ['NaN', 'Infinity', 'undefined',
+    'eval', 'parseInt', 'parseFloat', 'isNaN', 'isFinite', 'decodeURI',
+    'decodeURIComponent', 'encodeURI', 'encodeURIComponent',
+    'Object', 'Function', 'Array', 'String', 'Boolean', 'Number',
+    'Date', 'RegExp', 'Error', 'EvalError', 'RangeError',
+    'ReferenceError', 'SyntaxError', 'TypeError', 'URIError',
+    'Math', 'JSON'];
+const GLOBAL_OBJECT_PROPERTY_MAP = {};
+GLOBAL_OBJECT_PROPERTIES.forEach((p) => GLOBAL_OBJECT_PROPERTY_MAP[p] = p);
+
 try {
   // hack for require.resolve("./relative") to work properly.
   module.filename = path.resolve('repl');
@@ -582,10 +592,20 @@ REPLServer.prototype.createContext = function() {
     context = global;
   } else {
     context = vm.createContext();
-    for (var i in global) context[i] = global[i];
-    context.console = new Console(this.outputStream);
     context.global = context;
-    context.global.global = context;
+    const _console = new Console(this.outputStream);
+    Object.defineProperty(context, 'console', {
+      configurable: true,
+      enumerable: true,
+      get: () => _console
+    });
+    Object.getOwnPropertyNames(global).filter((name) => {
+      if (name === 'console' || name === 'global') return false;
+      return GLOBAL_OBJECT_PROPERTY_MAP[name] === undefined;
+    }).forEach((name) => {
+      Object.defineProperty(context, name,
+        Object.getOwnPropertyDescriptor(global, name));
+    });
   }
 
   const module = new Module('<repl>');
@@ -1052,13 +1072,7 @@ REPLServer.prototype.memory = function memory(cmd) {
 function addStandardGlobals(completionGroups, filter) {
   // Global object properties
   // (http://www.ecma-international.org/publications/standards/Ecma-262.htm)
-  completionGroups.push(['NaN', 'Infinity', 'undefined',
-    'eval', 'parseInt', 'parseFloat', 'isNaN', 'isFinite', 'decodeURI',
-    'decodeURIComponent', 'encodeURI', 'encodeURIComponent',
-    'Object', 'Function', 'Array', 'String', 'Boolean', 'Number',
-    'Date', 'RegExp', 'Error', 'EvalError', 'RangeError',
-    'ReferenceError', 'SyntaxError', 'TypeError', 'URIError',
-    'Math', 'JSON']);
+  completionGroups.push(GLOBAL_OBJECT_PROPERTIES);
   // Common keywords. Exclude for completion on the empty string, b/c
   // they just get in the way.
   if (filter) {
diff --git a/test/parallel/test-repl-console.js b/test/parallel/test-repl-console.js
@@ -18,3 +18,6 @@ assert(r.context.console);
 
 // ensure that the repl console instance is not the global one
 assert.notStrictEqual(r.context.console, console);
+
+// ensure that the repl console instance does not have a setter
+assert.throws(() => r.context.console = 'foo', TypeError);
diff --git a/test/parallel/test-repl-context.js b/test/parallel/test-repl-context.js
@@ -0,0 +1,26 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const repl = require('repl');
+
+// Create a dummy stream that does nothing
+const stream = new common.ArrayStream();
+
+// Test when useGlobal is false
+testContext(repl.start({
+  input: stream,
+  output: stream,
+  useGlobal: false
+}));
+
+function testContext(repl) {
+  const context = repl.createContext();
+  // ensure that the repl context gets its own "console" instance
+  assert(context.console instanceof require('console').Console);
+
+  // ensure that the repl's global property is the context
+  assert(context.global === context);
+
+  // ensure that the repl console instance does not have a setter
+  assert.throws(() => context.console = 'foo');
+}
diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
@@ -267,3 +267,19 @@ putIn.run(['.clear']);
 testMe.complete('.b', common.mustCall((error, data) => {
   assert.deepStrictEqual(data, [['break'], 'b']);
 }));
+
+const testNonGlobal = repl.start({
+  input: putIn,
+  output: putIn,
+  useGlobal: false
+});
+
+const builtins = [['Infinity', '', 'Int16Array', 'Int32Array',
+                                 'Int8Array'], 'I'];
+
+if (typeof Intl === 'object') {
+  builtins[0].push('Intl');
+}
+testNonGlobal.complete('I', common.mustCall((error, data) => {
+  assert.deepStrictEqual(data, builtins);
+}));
