fix: refactor engines validation to lint syntax

Author: lukekarrys

.eslintrc.local.js

@@ -0,0 +1,30 @@
+const { resolve, relative } = require('path')
+
+// Create an override to lockdown a file to es6 syntax only
+// and only allow it to require an allowlist of files
+const res = (p) => resolve(__dirname, p)
+const rel = (p) => relative(__dirname, res(p))
+const braces = (a) => a.length > 1 ? `{${a.map(rel).join(',')}}` : a[0]
+
+const es6Files = (e) => Object.entries(e).map(([file, allow]) => ({
+  files: `./${rel(file)}`,
+  parserOptions: {
+    ecmaVersion: 6,
+  },
+  rules: {
+    'node/no-restricted-require': ['error', [{
+      name: ['/**', `!${__dirname}/${braces(allow)}`],
+      message: `This file can only require: ${allow.join(',')}`,
+    }]],
+  },
+}))
+
+module.exports = {
+  rules: {
+    'no-console': 'error',
+  },
+  overrides: es6Files({
+    'index.js': ['lib/es6/validate-engines.js'],
+    'lib/es6/validate-engines.js': ['package.json', 'lib/cli.js'],
+  }),
+}

.eslintrc.local.json

@@ -1,5 +1,8 @@
 if (require.main === module) {
-  require('./lib/cli.js')(process)
+  require('./lib/es6/validate-engines.js')(
+    process,
+    () => require(require('path').resolve(__dirname, './lib/cli.js'))
+  )
 } else {
   throw new Error('The programmatic API was removed in npm v8.0.0')
 }

index.js

@@ -1,9 +1,7 @@
 /* eslint-disable max-len */
-// Code in this file should work in all conceivably runnable versions of node.
-// A best effort is made to catch syntax errors to give users a good error message if they are using a node version that doesn't allow syntax we are using in other files such as private properties, etc
 
 // Separated out for easier unit testing
-module.exports = async process => {
+module.exports = async (process, validateEngines) => {
   // set it here so that regardless of what happens later, we don't
   // leak any private CLI configs to other programs
   process.title = 'npm'
@@ -13,31 +11,6 @@ module.exports = async process => {
     process.argv.splice(1, 1, 'npm', '-g')
   }
 
-  const nodeVersion = process.version.replace(/-.*$/, '')
-  const pkg = require('../package.json')
-  const engines = pkg.engines.node
-  const npmVersion = `v${pkg.version}`
-
-  const unsupportedMessage = `npm ${npmVersion} does not support Node.js ${nodeVersion}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`
-
-  const brokenMessage = `ERROR: npm ${npmVersion} is known not to run on Node.js ${nodeVersion}.  This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`
-
-  // Coverage ignored because this is only hit in very unsupported node versions and it's a best effort attempt to show something nice in those cases
-  /* istanbul ignore next */
-  const syntaxErrorHandler = (err) => {
-    if (err instanceof SyntaxError) {
-      // eslint-disable-next-line no-console
-      console.error(`${brokenMessage}\n\nERROR:`)
-      // eslint-disable-next-line no-console
-      console.error(err)
-      return process.exit(1)
-    }
-    throw err
-  }
-
-  process.on('uncaughtException', syntaxErrorHandler)
-  process.on('unhandledRejection', syntaxErrorHandler)
-
   const satisfies = require('semver/functions/satisfies')
   const exitHandler = require('./utils/exit-handler.js')
   const Npm = require('./npm.js')
@@ -51,14 +24,13 @@ module.exports = async process => {
   log.info('using', 'node@%s', process.version)
 
   // At this point we've required a few files and can be pretty sure we dont contain invalid syntax for this version of node. It's possible a lazy require would, but that's unlikely enough that it's not worth catching anymore and we attach the more important exit handlers.
-  process.off('uncaughtException', syntaxErrorHandler)
-  process.off('unhandledRejection', syntaxErrorHandler)
+  validateEngines.off()
   process.on('uncaughtException', exitHandler)
   process.on('unhandledRejection', exitHandler)
 
   // It is now safe to log a warning if they are using a version of node that is not going to fail on syntax errors but is still unsupported and untested and might not work reliably. This is safe to use the logger now which we want since this will show up in the error log too.
-  if (!satisfies(nodeVersion, engines)) {
-    log.warn('cli', unsupportedMessage)
+  if (!satisfies(validateEngines.node, validateEngines.engines)) {
+    log.warn('cli', validateEngines.unsupportedMessage)
   }
 
   let cmd

lib/cli.js

@@ -0,0 +1,49 @@
+// This is separate to indicate that it should contain code we expect to work in
+// all versions of node >= 6.  This is a best effort to catch syntax errors to
+// give users a good error message if they are using a node version that doesn't
+// allow syntax we are using such as private properties, etc. This file is
+// linted with ecmaVersion=6 so we don't use invalid syntax, which is set in the
+// .eslintrc.local.json file
+
+const { engines: { node: engines }, version } = require('../../package.json')
+const npm = `v${version}`
+
+module.exports = (process, getCli) => {
+  const node = process.version.replace(/-.*$/, '')
+
+  /* eslint-disable-next-line max-len */
+  const unsupportedMessage = `npm ${npm} does not support Node.js ${node}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`
+
+  /* eslint-disable-next-line max-len */
+  const brokenMessage = `ERROR: npm ${npm} is known not to run on Node.js ${node}.  This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`
+
+  // coverage ignored because this is only hit in very unsupported node versions
+  // and it's a best effort attempt to show something nice in those cases
+  /* istanbul ignore next */
+  const syntaxErrorHandler = (err) => {
+    if (err instanceof SyntaxError) {
+      // eslint-disable-next-line no-console
+      console.error(`${brokenMessage}\n\nERROR:`)
+      // eslint-disable-next-line no-console
+      console.error(err)
+      return process.exit(1)
+    }
+    throw err
+  }
+
+  process.on('uncaughtException', syntaxErrorHandler)
+  process.on('unhandledRejection', syntaxErrorHandler)
+
+  // require this only after setting up the error handlers
+  const cli = getCli()
+  return cli(process, {
+    node,
+    npm,
+    engines,
+    unsupportedMessage,
+    off: () => {
+      process.off('uncaughtException', syntaxErrorHandler)
+      process.off('unhandledRejection', syntaxErrorHandler)
+    },
+  })
+}

lib/es6/validate-engines.js

@@ -1,6 +1,7 @@
 const t = require('tap')
 const { load: loadMockNpm } = require('../fixtures/mock-npm.js')
 const tmock = require('../fixtures/tmock')
+const validateEngines = require('../../lib/es6/validate-engines.js')
 
 const cliMock = async (t, opts) => {
   let exitHandlerArgs = null
@@ -20,7 +21,7 @@ const cliMock = async (t, opts) => {
 
   return {
     Npm,
-    cli,
+    cli: (p) => validateEngines(p, () => cli),
     outputs,
     exitHandlerCalled: () => exitHandlerArgs,
     exitHandlerNpm: () => npm,

test/lib/cli.js

@@ -0,0 +1,34 @@
+const t = require('tap')
+const mockGlobals = require('@npmcli/mock-globals')
+const tmock = require('../../fixtures/tmock')
+
+const mockValidateEngines = (t) => {
+  const validateEngines = tmock(t, '{LIB}/es6/validate-engines.js', {
+    '{ROOT}/package.json': { version: '1.2.3', engines: { node: '>=0' } },
+  })
+  mockGlobals(t, { 'process.version': 'v4.5.6' })
+  return validateEngines(process, () => (_, r) => r)
+}
+
+t.test('validate engines', async t => {
+  t.equal(process.listenerCount('uncaughtException'), 0)
+  t.equal(process.listenerCount('unhandledRejection'), 0)
+
+  const result = mockValidateEngines(t)
+
+  t.equal(process.listenerCount('uncaughtException'), 1)
+  t.equal(process.listenerCount('unhandledRejection'), 1)
+
+  t.match(result, {
+    node: 'v4.5.6',
+    npm: 'v1.2.3',
+    engines: '>=0',
+    /* eslint-disable-next-line max-len */
+    unsupportedMessage: 'npm v1.2.3 does not support Node.js v4.5.6. This version of npm supports the following node versions: `>=0`. You can find the latest version at https://nodejs.org/.',
+  })
+
+  result.off()
+
+  t.equal(process.listenerCount('uncaughtException'), 0)
+  t.equal(process.listenerCount('unhandledRejection'), 0)
+})