|
1 | 1 | const { log } = require('proc-log') |
2 | | -const { definitions, shorthands } = require('@npmcli/config/lib/definitions') |
| 2 | +const { definitions } = require('@npmcli/config/lib/definitions') |
3 | 3 | const nopt = require('nopt') |
4 | 4 |
|
5 | 5 | class BaseCommand { |
@@ -323,10 +323,9 @@ class BaseCommand { |
323 | 323 | delete parsed.argv |
324 | 324 | } |
325 | 325 |
|
326 | | - // Validate flags - only if command has definitions (new system) |
327 | | - if (this.constructor.definitions && this.constructor.definitions.length > 0) { |
328 | | - this.#validateFlags(parsed, commandDefinitions, remains) |
329 | | - } |
| 326 | + // Validate unknown CLI flags/configs and unexpected positionals. |
| 327 | + // Runs for every command; command-specific flags are allow-listed here so they don't trip the global unknown-config collection from Config.loadCLI(). |
| 328 | + this.validateCli(commandDefinitions, remains) |
330 | 329 |
|
331 | 330 | // Check for conflicts between main flags and their aliases |
332 | 331 | // Also map aliases back to their main keys |
@@ -363,64 +362,67 @@ class BaseCommand { |
363 | 362 | return [{ ...defaults, ...filtered }, remains] |
364 | 363 | } |
365 | 364 |
|
366 | | - // Validate flags and throw errors for unknown flags or unexpected positionals |
367 | | - #validateFlags (parsed, commandDefinitions, remains) { |
368 | | - // Build a set of all valid flag names (global + command-specific + shorthands) |
369 | | - const validFlags = new Set([ |
370 | | - ...Object.keys(definitions), |
| 365 | + // Unified CLI validation — runs for every command (definitions-based and legacy). |
| 366 | + // Reads collected unknown configs from Config (they were collected, not thrown, during Config.load()), subtracts any command-specific definitions, and throws a single aggregated error. |
| 367 | + // Also enforces extra-positional errors for commands that set a finite `static positionals`. |
| 368 | + // Shellout commands (run/exec/lifecycle) leave `static positionals = null` and are unaffected. |
| 369 | + // Commands that set `static skipConfigValidation = true` (config, help, doctor, completion, version) bypass both unknown-config checks so they can operate against a broken .npmrc. |
| 370 | + validateCli (commandDefinitions = this.constructor.definitions || [], remains = null) { |
| 371 | + const allowlist = new Set([ |
371 | 372 | ...commandDefinitions.map(d => d.key), |
372 | | - ...Object.keys(shorthands), // Add global shorthands like 'verbose', 'dd', etc. |
| 373 | + ...commandDefinitions.flatMap(d => Array.isArray(d.alias) ? d.alias : []), |
373 | 374 | ]) |
374 | 375 |
|
375 | | - // Add aliases to valid flags |
376 | | - for (const def of commandDefinitions) { |
377 | | - if (def.alias && Array.isArray(def.alias)) { |
378 | | - for (const alias of def.alias) { |
379 | | - validFlags.add(alias) |
380 | | - } |
| 376 | + if (!this.constructor.skipConfigValidation) { |
| 377 | + const cliUnknowns = this.npm.config.getUnknownConfigs('cli') |
| 378 | + .filter(u => !allowlist.has(u.key) && !allowlist.has(u.baseKey)) |
| 379 | + if (cliUnknowns.length > 0) { |
| 380 | + const flagList = cliUnknowns |
| 381 | + .map(u => (u.baseKey ? `--${u.baseKey} (${u.key})` : `--${u.key}`)) |
| 382 | + .join(', ') |
| 383 | + throw this.usageError( |
| 384 | + `Unknown cli config${cliUnknowns.length > 1 ? 's' : ''}: ${flagList}. ` + |
| 385 | + `Run \`npm help config\` for supported options.` |
| 386 | + ) |
381 | 387 | } |
382 | | - } |
383 | 388 |
|
384 | | - // Check parsed flags against valid flags |
385 | | - const unknownFlags = [] |
386 | | - for (const key of Object.keys(parsed)) { |
387 | | - if (!validFlags.has(key)) { |
388 | | - unknownFlags.push(key) |
| 389 | + const fileUnknowns = [] |
| 390 | + for (const where of ['builtin', 'project', 'user', 'global']) { |
| 391 | + fileUnknowns.push(...this.npm.config.getUnknownConfigs(where)) |
389 | 392 | } |
390 | | - } |
391 | | - |
392 | | - // Throw error if unknown flags were found |
393 | | - if (unknownFlags.length > 0) { |
394 | | - const flagList = unknownFlags.map(f => `--${f}`).join(', ') |
395 | | - throw this.usageError(`Unknown flag${unknownFlags.length > 1 ? 's' : ''}: ${flagList}`) |
396 | | - } |
397 | | - |
398 | | - // Remove warnings for command-specific definitions that npm's global config doesn't know about (these were queued as "unknown" during config.load()) |
399 | | - for (const def of commandDefinitions) { |
400 | | - this.npm.config.removeWarning(def.key) |
401 | | - if (def.alias && Array.isArray(def.alias)) { |
402 | | - for (const alias of def.alias) { |
403 | | - this.npm.config.removeWarning(alias) |
404 | | - } |
| 393 | + if (fileUnknowns.length > 0) { |
| 394 | + const lines = fileUnknowns.map(u => { |
| 395 | + const display = u.baseKey ? `"${u.baseKey}" (${u.key})` : `"${u.key}"` |
| 396 | + return ` - ${u.where} config ${display} from ${u.source}` |
| 397 | + }) |
| 398 | + const msg = [ |
| 399 | + `Unknown npm configuration key${fileUnknowns.length > 1 ? 's' : ''}:`, |
| 400 | + ...lines, |
| 401 | + 'See `npm help npmrc` for supported config options.', |
| 402 | + ].join('\n') |
| 403 | + throw Object.assign(new Error(msg), { |
| 404 | + code: 'EUNKNOWNCONFIG', |
| 405 | + unknownConfigs: fileUnknowns, |
| 406 | + }) |
405 | 407 | } |
406 | 408 | } |
407 | 409 |
|
408 | | - // Remove warnings for unknown positionals that were actually consumed as flag values by command-specific definitions (e.g., --id <value> where --id is command-specific) |
409 | | - const remainsSet = new Set(remains) |
410 | | - for (const unknownPos of this.npm.config.getUnknownPositionals()) { |
411 | | - if (!remainsSet.has(unknownPos)) { |
412 | | - // This value was consumed as a flag value, not truly a positional |
413 | | - this.npm.config.removeUnknownPositional(unknownPos) |
| 410 | + // Positionals consumed as flag values by command-specific definitions were queued as "unknown positional" warnings by Config.unknownHandler; drop those since they're actually flag arguments. |
| 411 | + if (Array.isArray(remains)) { |
| 412 | + const remainsSet = new Set(remains) |
| 413 | + for (const unknownPos of this.npm.config.getUnknownPositionals()) { |
| 414 | + if (!remainsSet.has(unknownPos)) { |
| 415 | + this.npm.config.removeUnknownPositional(unknownPos) |
| 416 | + } |
414 | 417 | } |
415 | 418 | } |
416 | 419 |
|
417 | | - // Warn about extra positional arguments beyond what the command expects |
418 | | - const expectedPositionals = this.constructor.positionals |
419 | | - if (expectedPositionals !== null && remains.length > expectedPositionals) { |
420 | | - const extraPositionals = remains.slice(expectedPositionals) |
421 | | - for (const extra of extraPositionals) { |
422 | | - throw new Error(`Unknown positional argument: ${extra}`) |
423 | | - } |
| 420 | + const expected = this.constructor.positionals |
| 421 | + if (expected !== null && remains !== null && remains.length > expected) { |
| 422 | + const extra = remains.slice(expected) |
| 423 | + throw this.usageError( |
| 424 | + `Unknown positional argument${extra.length > 1 ? 's' : ''}: ${extra.join(', ')}` |
| 425 | + ) |
424 | 426 | } |
425 | 427 |
|
426 | 428 | this.npm.config.logWarnings() |
|
0 commit comments