test: remove common.allowGlobals

lib/.eslintrc.yaml

@@ -31,6 +31,7 @@ rules:
   # Custom rules in tools/eslint-rules
   node-core/lowercase-name-for-primitive: error
   node-core/non-ascii-character: error
+  node-core/no-global-pollute: error
 globals:
   Intl: false
   # Assertions

lib/internal/main/eval_string.js

@@ -12,6 +12,7 @@ const { addBuiltinLibsToObject } = require('internal/modules/cjs/helpers');
 const { getOptionValue } = require('internal/options');
 const source = getOptionValue('--eval');
 prepareMainThreadExecution();
+// eslint-disable-next-line node-core/no-global-pollute
 addBuiltinLibsToObject(global);
 markBootstrapComplete();
 if (getOptionValue('--entry-type') === 'module')

lib/internal/per_context/setup.js

@@ -1,15 +1,14 @@
 // This file is compiled as if it's wrapped in a function with arguments
 // passed by node::NewContext()
-/* global global */
 
 'use strict';
 
 // https://github.com/nodejs/node/issues/14909
-if (global.Intl) {
-  delete global.Intl.v8BreakIterator;
+if (typeof Intl !== 'undefined') {
+  delete Intl.v8BreakIterator;
 }
 
 // https://github.com/nodejs/node/issues/21219
-if (global.Atomics) {
-  delete global.Atomics.wake;
+if (typeof Atomics !== 'undefined') {
+  delete Atomics.wake;
 }

lib/internal/process/execution.js

@@ -62,6 +62,7 @@ function evalScript(name, body, breakFirstLine) {
   const module = new CJSModule(name);
   module.filename = path.join(cwd, name);
   module.paths = CJSModule._nodeModulePaths(cwd);
+  // eslint-disable-next-line node-core/no-global-pollute
   global.kVmBreakFirstLineSymbol = kVmBreakFirstLineSymbol;
   const script = `
     global.__filename = ${JSON.stringify(name)};

test/common/README.md

@@ -33,12 +33,6 @@ The `benchmark` module is used by tests to run benchmarks.
 The `common` module is used by tests for consistency across repeated
 tasks.
 
-### allowGlobals(...whitelist)
-* `whitelist` [<Array>] Array of Globals
-* return [<Array>]
-
-Takes `whitelist` and concats that with predefined `knownGlobals`.
-
 ### busyLoop(time)
 * `time` [<number>]
 

test/common/index.js

@@ -262,49 +262,6 @@ function platformTimeout(ms) {
   return ms; // ARMv8+
 }
 
-let knownGlobals = [
-  clearImmediate,
-  clearInterval,
-  clearTimeout,
-  global,
-  setImmediate,
-  setInterval,
-  setTimeout,
-  queueMicrotask,
-];
-
-if (global.gc) {
-  knownGlobals.push(global.gc);
-}
-
-if (process.env.NODE_TEST_KNOWN_GLOBALS) {
-  const knownFromEnv = process.env.NODE_TEST_KNOWN_GLOBALS.split(',');
-  allowGlobals(...knownFromEnv);
-}
-
-function allowGlobals(...whitelist) {
-  knownGlobals = knownGlobals.concat(whitelist);
-}
-
-function leakedGlobals() {
-  const leaked = [];
-
-  for (const val in global) {
-    if (!knownGlobals.includes(global[val])) {
-      leaked.push(val);
-    }
-  }
-
-  return leaked;
-}
-
-process.on('exit', function() {
-  const leaked = leakedGlobals();
-  if (leaked.length > 0) {
-    assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
-  }
-});
-
 const mustCallChecks = [];
 
 function runCallChecks(exitCode) {
@@ -715,7 +672,6 @@ function runWithInvalidFD(func) {
 }
 
 module.exports = {
-  allowGlobals,
   buildType,
   busyLoop,
   canCreateSymLink,

test/common/index.mjs

@@ -32,7 +32,6 @@ const {
   childShouldThrowAndAbort,
   createZeroFilledFile,
   platformTimeout,
-  allowGlobals,
   mustCall,
   mustCallAtLeast,
   hasMultiLocalhost,
@@ -78,7 +77,6 @@ export {
   childShouldThrowAndAbort,
   createZeroFilledFile,
   platformTimeout,
-  allowGlobals,
   mustCall,
   mustCallAtLeast,
   hasMultiLocalhost,

test/parallel/test-common.js

@@ -26,15 +26,6 @@ const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const { execFile } = require('child_process');
 
-// Test for leaked global detection
-{
-  const p = fixtures.path('leakedGlobal.js');
-  execFile(process.execPath, [p], common.mustCall((err, stdout, stderr) => {
-    assert.notStrictEqual(err.code, 0);
-    assert.ok(/\bAssertionError\b.*\bUnexpected global\b.*\bgc\b/.test(stderr));
-  }));
-}
-
 // common.mustCall() tests
 assert.throws(function() {
   common.mustCall(function() {}, 'foo');

test/parallel/test-domain-crypto.js

@@ -29,7 +29,6 @@ if (!common.hasCrypto)
 const crypto = require('crypto');
 
 // Pollution of global is intentional as part of test.
-common.allowGlobals(require('domain'));
 // See https://github.com/nodejs/node/commit/d1eff9ab
 global.domain = require('domain');
 

test/parallel/test-fs-write.js

@@ -36,8 +36,6 @@ const expected = 'ümlaut.';
 const constants = fs.constants;
 
 /* eslint-disable no-undef */
-common.allowGlobals(externalizeString, isOneByteString, x);
-
 {
   const expected = 'ümlaut eins';  // Must be a unique string.
   externalizeString(expected);

test/parallel/test-global.js

@@ -23,7 +23,7 @@
 // treated as a global without being declared with `var`/`let`/`const`.
 
 /* eslint-disable strict */
-const common = require('../common');
+require('../common');
 const fixtures = require('../common/fixtures');
 
 const assert = require('assert');
@@ -54,8 +54,6 @@ builtinModules.forEach((moduleName) => {
   assert.deepStrictEqual(new Set(Object.keys(global)), new Set(expected));
 }
 
-common.allowGlobals('bar', 'foo');
-
 baseFoo = 'foo'; // eslint-disable-line no-undef
 global.baseBar = 'bar';
 

test/parallel/test-repl-autolibs.js

@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+require('../common');
 const ArrayStream = require('../common/arraystream');
 const assert = require('assert');
 const util = require('util');
@@ -63,7 +63,6 @@ function test2() {
   };
   const val = {};
   global.url = val;
-  common.allowGlobals(val);
   assert(!gotWrite);
   putIn.run(['url']);
   assert(gotWrite);

test/parallel/test-repl-reset-event.js

@@ -26,8 +26,6 @@ const assert = require('assert');
 const repl = require('repl');
 const util = require('util');
 
-common.allowGlobals(42);
-
 // Create a dummy stream that does nothing
 const dummy = new ArrayStream();
 

test/parallel/test-repl.js

@@ -770,7 +770,6 @@ const tcpTests = [
 
     socket.end();
   }
-  common.allowGlobals(...Object.values(global));
 })();
 
 function startTCPRepl() {

test/parallel/test-timer-immediate.js

@@ -1,5 +1,4 @@
 'use strict';
 const common = require('../common');
 global.process = {};  // Boom!
-common.allowGlobals(global.process);
 setImmediate(common.mustCall());

test/parallel/test-vm-new-script-this-context.js

@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const Script = require('vm').Script;
 
@@ -58,11 +58,3 @@ global.f = function() { global.foo = 100; };
 script = new Script('f()');
 script.runInThisContext(script);
 assert.strictEqual(global.foo, 100);
-
-common.allowGlobals(
-  global.hello,
-  global.code,
-  global.foo,
-  global.obj,
-  global.f
-);

test/parallel/test-vm-run-in-new-context.js

@@ -22,7 +22,7 @@
 'use strict';
 // Flags: --expose-gc
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const vm = require('vm');
 
@@ -91,10 +91,3 @@ for (const arg of [filename, { filename }]) {
     return true;
   });
 }
-
-common.allowGlobals(
-  global.hello,
-  global.code,
-  global.foo,
-  global.obj
-);

test/parallel/test-vm-static-this.js

@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* eslint-disable strict */
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const vm = require('vm');
 
@@ -56,10 +56,3 @@ assert.strictEqual(global.foo, 1);
 global.f = function() { global.foo = 100; };
 vm.runInThisContext('f()');
 assert.strictEqual(global.foo, 100);
-
-common.allowGlobals(
-  global.hello,
-  global.foo,
-  global.obj,
-  global.f
-);

tools/eslint-rules/no-global-pollute.js

@@ -0,0 +1,33 @@
+/**
+ * @fileOverview This rule makes sure that the globals don't get polluted by
+ * adding new entries to `global`.
+ * @author Ruben Bridgewater <ruben@bridgewater.de>
+ */
+
+'use strict';
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function(context) {
+  const filename = context.getFilename();
+
+  return {
+    'Program:exit': function() {
+      const globalScope = context.getScope();
+      const variable = globalScope.set.get('global');
+      if (variable &&
+          !filename.includes('bootstrap') &&
+          !filename.includes('repl.js')) {
+        const msg = 'Please do not pollute the global scope';
+        variable.references.forEach((reference) => {
+          context.report({
+            node: reference.identifier,
+            message: msg
+          });
+        });
+      }
+    }
+  };
+};