fix: evaluate configs in command class

 ### Refactor Config Reading During `exec`

This removes the previous workarounds that were required due to tests
setting configs that were not available in the constructor of the
commands.

This also made the fix for nodejs/node#45881
easier since the config checks for workspaces can now all be made in the
command. The fix was to move the package.json detection to
`@npmcli/config` and use that boolean instead of setting
`location=project` which affected all commands such as `config set`
saving to the wrong location.

Some notable changes from this refactor include:
-  `execWorkspaces` no longer being passed the raw workspace filters and
   instead requiring `this.setWorkspaces()` to be called which was
   already being done in most commands
- `static workspaces = [true|false]` on all commands to indicate
  workspaces support. This is used in docs generation and whether to
  throw when `execWorkspaces` is called or not.

 ### Drop `fakeMockNpm` and refactor `mockNpm`

This refactor also drops `fakeMockNpm` in favor of `realMockNpm` (now
just `mockNpm`), and adds new features to `mockNpm`.

Some new features of `mockNpm`:

- sets all configs via argv so they are parsed before `npm.load()`. This
  is the most important change as it more closely resembles real usage.
- automatically resets `process.exitCode`
- automatically puts global `node_modules` in correct testdir based on platform
- more helpful error messages when used in unsupported ways
- ability to preload a command for execution
- sets `cwd` automatically to `prefix` and sets `globalPrefix` to the
  specified testdir

Note that this commit does not include the actual test changes, which
are included in the following commits for readability reasons.

 ### Linting

This also removes some of the one off linting rules that were set in the
past to reduce churn and fixes all remaining errors.

Author: lukekarrys

.eslintrc.local.json

@@ -1,14 +1,5 @@
 {
   "rules": {
-    "no-shadow": "off",
     "no-console": "error"
-  },
-  "overrides": [{
-    "files": [
-      "test/**"
-    ],
-    "rules": {
-      "no-console": "off"
-    }
-  }]
+  }
 }

lib/arborist-cmd.js

@@ -17,22 +17,35 @@ class ArboristCmd extends BaseCommand {
     'install-links',
   ]
 
+  static workspaces = true
   static ignoreImplicitWorkspace = false
 
   constructor (npm) {
     super(npm)
-    if (this.npm.config.isDefault('audit')
-      && (this.npm.global || this.npm.config.get('location') !== 'project')
-    ) {
-      this.npm.config.set('audit', false)
-    } else if (this.npm.global && this.npm.config.get('audit')) {
-      log.warn('config',
-        'includes both --global and --audit, which is currently unsupported.')
+
+    const { config } = this.npm
+
+    // when location isn't set and global isn't true check for a package.json at
+    // the localPrefix and set the location to project if found
+    const locationProject = config.get('location') === 'project' || (
+      config.isDefault('location')
+      // this is different then `npm.global` which falls back to checking
+      // location which we do not want to use here
+      && !config.get('global')
+      && npm.localPackage
+    )
+
+    // if audit is not set and we are in global mode and location is not project
+    // and we assume its not a project related context, then we set audit=false
+    if (config.isDefault('audit') && (this.npm.global || !locationProject)) {
+      config.set('audit', false)
+    } else if (this.npm.global && config.get('audit')) {
+      log.warn('config', 'includes both --global and --audit, which is currently unsupported.')
     }
   }
 
-  async execWorkspaces (args, filters) {
-    await this.setWorkspaces(filters)
+  async execWorkspaces (args) {
+    await this.setWorkspaces()
     return this.exec(args)
   }
 }

lib/base-command.js

@@ -8,12 +8,21 @@ const getWorkspaces = require('./workspaces/get-workspaces.js')
 const cmdAliases = require('./utils/cmd-list').aliases
 
 class BaseCommand {
+  static workspaces = false
+  static ignoreImplicitWorkspace = true
+
   constructor (npm) {
     this.wrapWidth = 80
     this.npm = npm
 
-    if (!this.skipConfigValidation) {
-      this.npm.config.validate()
+    const { config } = this.npm
+
+    if (!this.constructor.skipConfigValidation) {
+      config.validate()
+    }
+
+    if (config.get('workspaces') === false && config.get('workspace').length) {
+      throw new Error('Can not use --no-workspaces and --workspace at the same time')
     }
   }
 
@@ -25,35 +34,31 @@ class BaseCommand {
     return this.constructor.description
   }
 
-  get ignoreImplicitWorkspace () {
-    return this.constructor.ignoreImplicitWorkspace
-  }
-
-  get skipConfigValidation () {
-    return this.constructor.skipConfigValidation
+  get params () {
+    return this.constructor.params
   }
 
   get usage () {
     const usage = [
-      `${this.constructor.description}`,
+      `${this.description}`,
       '',
       'Usage:',
     ]
 
     if (!this.constructor.usage) {
-      usage.push(`npm ${this.constructor.name}`)
+      usage.push(`npm ${this.name}`)
     } else {
-      usage.push(...this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`))
+      usage.push(...this.constructor.usage.map(u => `npm ${this.name} ${u}`))
     }
 
-    if (this.constructor.params) {
+    if (this.params) {
       usage.push('')
       usage.push('Options:')
       usage.push(this.wrappedParams)
     }
 
     const aliases = Object.keys(cmdAliases).reduce((p, c) => {
-      if (cmdAliases[c] === this.constructor.name) {
+      if (cmdAliases[c] === this.name) {
         p.push(c)
       }
       return p
@@ -68,7 +73,7 @@ class BaseCommand {
     }
 
     usage.push('')
-    usage.push(`Run "npm help ${this.constructor.name}" for more info`)
+    usage.push(`Run "npm help ${this.name}" for more info`)
 
     return usage.join('\n')
   }
@@ -77,7 +82,7 @@ class BaseCommand {
     let results = ''
     let line = ''
 
-    for (const param of this.constructor.params) {
+    for (const param of this.params) {
       const usage = `[${ConfigDefinitions[param].usage}]`
       if (line.length && line.length + usage.length > this.wrapWidth) {
         results = [results, line].filter(Boolean).join('\n')
@@ -98,26 +103,48 @@ class BaseCommand {
     })
   }
 
-  async execWorkspaces (args, filters) {
-    throw Object.assign(new Error('This command does not support workspaces.'), {
-      code: 'ENOWORKSPACES',
-    })
-  }
+  async cmdExec (args) {
+    const { config } = this.npm
 
-  async setWorkspaces (filters) {
-    if (this.isArboristCmd) {
-      this.includeWorkspaceRoot = false
+    if (config.get('usage')) {
+      return this.npm.output(this.usage)
     }
 
-    const relativeFrom = relative(this.npm.localPrefix, process.cwd()).startsWith('..')
-      ? this.npm.localPrefix
-      : process.cwd()
+    const hasWsConfig = config.get('workspaces') || config.get('workspace').length
+    // if cwd is a workspace, the default is set to [that workspace]
+    const implicitWs = config.get('workspace', 'default').length
 
+    // (-ws || -w foo) && (cwd is not a workspace || command is not ignoring implicit workspaces)
+    if (hasWsConfig && (!implicitWs || !this.constructor.ignoreImplicitWorkspace)) {
+      if (this.npm.global) {
+        throw new Error('Workspaces not supported for global packages')
+      }
+      if (!this.constructor.workspaces) {
+        throw Object.assign(new Error('This command does not support workspaces.'), {
+          code: 'ENOWORKSPACES',
+        })
+      }
+      return this.execWorkspaces(args)
+    }
+
+    return this.exec(args)
+  }
+
+  async setWorkspaces () {
+    const includeWorkspaceRoot = this.isArboristCmd
+      ? false
+      : this.npm.config.get('include-workspace-root')
+
+    const prefixInsideCwd = relative(this.npm.localPrefix, process.cwd()).startsWith('..')
+    const relativeFrom = prefixInsideCwd ? this.npm.localPrefix : process.cwd()
+
+    const filters = this.npm.config.get('workspace')
     const ws = await getWorkspaces(filters, {
       path: this.npm.localPrefix,
-      includeWorkspaceRoot: this.includeWorkspaceRoot,
+      includeWorkspaceRoot,
       relativeFrom,
     })
+
     this.workspaces = ws
     this.workspaceNames = [...ws.keys()]
     this.workspacePaths = [...ws.values()]

lib/cli.js

@@ -68,6 +68,11 @@ module.exports = async process => {
   // leak any private CLI configs to other programs
   process.title = 'npm'
 
+  // if npm is called as "npmg" or "npm_g", then run in global mode.
+  if (process.argv[1][process.argv[1].length - 1] === 'g') {
+    process.argv.splice(1, 1, 'npm', '-g')
+  }
+
   // Nothing should happen before this line if we can't guarantee it will
   // not have syntax errors in some version of node
   const validateEngines = createEnginesValidation()
@@ -78,11 +83,6 @@ module.exports = async process => {
   const npm = new Npm()
   exitHandler.setNpm(npm)
 
-  // if npm is called as "npmg" or "npm_g", then run in global mode.
-  if (process.argv[1][process.argv[1].length - 1] === 'g') {
-    process.argv.splice(1, 1, 'npm', '-g')
-  }
-
   // only log node and npm paths in argv initially since argv can contain
   // sensitive info. a cleaned version will be logged later
   const log = require('./utils/log-shim.js')
@@ -112,6 +112,7 @@ module.exports = async process => {
   // this is how to use npm programmatically:
   try {
     await npm.load()
+
     if (npm.config.get('version', 'cli')) {
       npm.output(npm.version)
       return exitHandler()
@@ -130,7 +131,7 @@ module.exports = async process => {
       return exitHandler()
     }
 
-    await npm.exec(cmd, npm.argv)
+    await npm.exec(cmd)
     return exitHandler()
   } catch (err) {
     if (err.code === 'EUNKNOWNCOMMAND') {

lib/commands/access.js

@@ -37,8 +37,6 @@ class Access extends BaseCommand {
     'registry',
   ]
 
-  static ignoreImplicitWorkspace = true
-
   static usage = [
     'list packages [<user>|<scope>|<scope:team> [<package>]',
     'list collaborators [<package> [<user>]]',

lib/commands/adduser.js

@@ -13,8 +13,6 @@ class AddUser extends BaseCommand {
     'auth-type',
   ]
 
-  static ignoreImplicitWorkspace = true
-
   async exec (args) {
     const scope = this.npm.config.get('scope')
     let registry = this.npm.config.get('registry')

lib/commands/audit.js

@@ -152,7 +152,7 @@ class VerifySignatures {
     const keys = await fetch.json('/-/npm/v1/keys', {
       ...this.npm.flatOptions,
       registry,
-    }).then(({ keys }) => keys.map((key) => ({
+    }).then(({ keys: ks }) => ks.map((key) => ({
       ...key,
       pemkey: `-----BEGIN PUBLIC KEY-----\n${key.key}\n-----END PUBLIC KEY-----`,
     }))).catch(err => {

lib/commands/cache.js

@@ -2,7 +2,7 @@ const cacache = require('cacache')
 const Arborist = require('@npmcli/arborist')
 const pacote = require('pacote')
 const fs = require('fs/promises')
-const path = require('path')
+const { join } = require('path')
 const semver = require('semver')
 const BaseCommand = require('../base-command.js')
 const npa = require('npm-package-arg')
@@ -74,8 +74,6 @@ class Cache extends BaseCommand {
     'verify',
   ]
 
-  static ignoreImplicitWorkspace = true
-
   async completion (opts) {
     const argv = opts.conf.argv.remain
     if (argv.length === 2) {
@@ -111,7 +109,7 @@ class Cache extends BaseCommand {
 
   // npm cache clean [pkg]*
   async clean (args) {
-    const cachePath = path.join(this.npm.cache, '_cacache')
+    const cachePath = join(this.npm.cache, '_cacache')
     if (args.length === 0) {
       if (!this.npm.config.get('force')) {
         throw new Error(`As of npm@5, the npm cache self-heals from corruption issues
@@ -169,7 +167,7 @@ class Cache extends BaseCommand {
   }
 
   async verify () {
-    const cache = path.join(this.npm.cache, '_cacache')
+    const cache = join(this.npm.cache, '_cacache')
     const prefix = cache.indexOf(process.env.HOME) === 0
       ? `~${cache.slice(process.env.HOME.length)}`
       : cache
@@ -192,7 +190,7 @@ class Cache extends BaseCommand {
 
   // npm cache ls [--package <spec> ...]
   async ls (specs) {
-    const cachePath = path.join(this.npm.cache, '_cacache')
+    const cachePath = join(this.npm.cache, '_cacache')
     const cacheKeys = Object.keys(await cacache.ls(cachePath))
     if (specs.length > 0) {
       // get results for each package spec specified

lib/commands/completion.js

@@ -31,6 +31,7 @@
 
 const fs = require('fs/promises')
 const nopt = require('nopt')
+const { resolve } = require('path')
 
 const { definitions, shorthands } = require('../utils/config/index.js')
 const { aliases, commands, plumbing } = require('../utils/cmd-list.js')
@@ -40,29 +41,20 @@ const configNames = Object.keys(definitions)
 const shorthandNames = Object.keys(shorthands)
 const allConfs = configNames.concat(shorthandNames)
 const { isWindowsShell } = require('../utils/is-windows.js')
-const fileExists = async (file) => {
-  try {
-    const stat = await fs.stat(file)
-    return stat.isFile()
-  } catch {
-    return false
-  }
-}
+const fileExists = (file) => fs.stat(file).then(s => s.isFile()).catch(() => false)
 
 const BaseCommand = require('../base-command.js')
 
 class Completion extends BaseCommand {
   static description = 'Tab Completion for npm'
   static name = 'completion'
-  static ignoreImplicitWorkspace = true
 
   // completion for the completion command
   async completion (opts) {
     if (opts.w > 2) {
       return
     }
 
-    const { resolve } = require('path')
     const [bashExists, zshExists] = await Promise.all([
       fileExists(resolve(process.env.HOME, '.bashrc')),
       fileExists(resolve(process.env.HOME, '.zshrc')),
@@ -93,7 +85,7 @@ class Completion extends BaseCommand {
     if (COMP_CWORD === undefined ||
       COMP_LINE === undefined ||
       COMP_POINT === undefined) {
-      return dumpScript()
+      return dumpScript(resolve(this.npm.npmRoot, 'lib', 'utils', 'completion.sh'))
     }
 
     // ok we're actually looking at the envs and outputting the suggestions
@@ -150,9 +142,9 @@ class Completion extends BaseCommand {
     // take a little shortcut and use npm's arg parsing logic.
     // don't have to worry about the last arg being implicitly
     // boolean'ed, since the last block will catch that.
-    const types = Object.entries(definitions).reduce((types, [key, def]) => {
-      types[key] = def.type
-      return types
+    const types = Object.entries(definitions).reduce((acc, [key, def]) => {
+      acc[key] = def.type
+      return acc
     }, {})
     const parsed = opts.conf =
       nopt(types, shorthands, partialWords.slice(0, -1), 0)
@@ -196,10 +188,7 @@ class Completion extends BaseCommand {
   }
 }
 
-const dumpScript = async () => {
-  const { resolve } = require('path')
-  const p = resolve(__dirname, '..', 'utils', 'completion.sh')
-
+const dumpScript = async (p) => {
   const d = (await fs.readFile(p, 'utf8')).replace(/^#!.*?\n/, '')
   await new Promise((res, rej) => {
     let done = false