From d6e4f2d050708ec5d19ea7dd2f62133e7fd90ba4 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 16 Feb 2022 12:25:48 +0100 Subject: [PATCH] test: compute list of expected globals from ESLint config file We don't want to rely on mutable globals for core modules. Instead of maintaining a separate list of known globals in our test files, parse the ESLint config to ensure all globals are restricted in the `lib/` directory. --- test/common/index.js | 75 +++------------------- test/common/parseEslintConfigForGlobals.js | 39 +++++++++++ 2 files changed, 47 insertions(+), 67 deletions(-) create mode 100644 test/common/parseEslintConfigForGlobals.js diff --git a/test/common/index.js b/test/common/index.js index d550d6b8fd76bd..207e1c07d84798 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -37,10 +37,7 @@ const bits = ['arm64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x', 'x64'] .includes(process.arch) ? 64 : 32; const hasIntl = !!process.config.variables.v8_enable_i18n_support; -const { - atob, - btoa -} = require('buffer'); +const parseEslintConfigForGlobals = require('./parseEslintConfigForGlobals'); // Some tests assume a umask of 0o022 so set that up front. Tests that need a // different umask will set it themselves. @@ -257,67 +254,10 @@ function platformTimeout(ms) { return ms; // ARMv8+ } -let knownGlobals = [ - atob, - btoa, - clearImmediate, - clearInterval, - clearTimeout, - global, - setImmediate, - setInterval, - setTimeout, - queueMicrotask, -]; - -// TODO(@jasnell): This check can be temporary. AbortController is -// not currently supported in either Node.js 12 or 10, making it -// difficult to run tests comparatively on those versions. Once -// all supported versions have AbortController as a global, this -// check can be removed and AbortController can be added to the -// knownGlobals list above. -if (global.AbortController) - knownGlobals.push(global.AbortController); - -if (global.gc) { - knownGlobals.push(global.gc); -} - -if (global.performance) { - knownGlobals.push(global.performance); -} -if (global.PerformanceMark) { - knownGlobals.push(global.PerformanceMark); -} -if (global.PerformanceMeasure) { - knownGlobals.push(global.PerformanceMeasure); -} - -// TODO(@ethan-arrowood): Similar to previous checks, this can be temporary -// until v16.x is EOL. Once all supported versions have structuredClone we -// can add this to the list above instead. -if (global.structuredClone) { - knownGlobals.push(global.structuredClone); -} - -if (global.fetch) { - knownGlobals.push( - global.fetch, - global.FormData, - global.Request, - global.Response, - global.Headers, - ); -} -if (hasCrypto && global.crypto) { - knownGlobals.push(global.crypto); - knownGlobals.push(global.Crypto); - knownGlobals.push(global.CryptoKey); - knownGlobals.push(global.SubtleCrypto); -} +const knownGlobals = parseEslintConfigForGlobals(); function allowGlobals(...allowlist) { - knownGlobals = knownGlobals.concat(allowlist); + knownGlobals.push(...allowlist); } if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { @@ -329,9 +269,10 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { function leakedGlobals() { const leaked = []; - for (const val in global) { - if (!knownGlobals.includes(global[val])) { - leaked.push(val); + const globals = Object.getOwnPropertyDescriptors(global); + for (const val in globals) { + if (!knownGlobals.includes(global[val]) && globals[val].configurable) { + leaked.push(val.toString()); } } @@ -341,7 +282,7 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { process.on('exit', function() { const leaked = leakedGlobals(); if (leaked.length > 0) { - assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`); + assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}. Add it to lib/.eslint.yaml or call common.allowGlobals().`); } }); } diff --git a/test/common/parseEslintConfigForGlobals.js b/test/common/parseEslintConfigForGlobals.js new file mode 100644 index 00000000000000..ef0c28f8c9cbb8 --- /dev/null +++ b/test/common/parseEslintConfigForGlobals.js @@ -0,0 +1,39 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const readline = require('readline'); + +const searchLines = [ + ' no-restricted-globals:', + ' node-core/prefer-primordials:', +]; + +const restrictedGlobalLine = /^\s{4}- name:\s?([^#\s]+)/; +const closingLine = /^\s{0,3}[^#\s]/; + +module.exports = function parseEslintConfigForGlobals() { + const eslintConfig = readline.createInterface({ + input: fs.createReadStream(path.join(__dirname, '..', '..', 'lib', '.eslintrc.yaml')), + crlfDelay: Infinity, + }); + + const globals = [process]; + + let isReadingGlobals = false; + eslintConfig.on('line', (line) => { + if (isReadingGlobals) { + const match = restrictedGlobalLine.exec(line); + if (match && match[1] in global) { + globals.push(global[match[1]]); + } else if (closingLine.test(line)) { + isReadingGlobals = false; + } + } else if (line === searchLines[0]) { + searchLines.shift(); + isReadingGlobals = true; + } + }); + + return globals; +};