From 1ca3b7be1289637ce0986dcd54d994a6f5680ece Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Thu, 15 Jul 2021 19:54:17 -0600 Subject: [PATCH 1/9] fix: prevent version name collision --- lib/yargs-factory.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 53da93e5e..8846338bf 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -904,6 +904,13 @@ export class YargsInstance { opt = {}; } + // Prevent version name collision + if (key === 'version' || opt?.alias === 'version') { + throw new YError( + '"version" is a reserved word.\nPlease use a different option key or use the built-in version method (if applicable).\nhttps://yargs.js.org/docs/#api-reference-version' + ); + } + this.#options.key[key] = true; // track manually set keys. if (opt.alias) this.alias(key, opt.alias); From f2c8b1a1353c350a35b5e4490f23707554db169f Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Fri, 16 Jul 2021 00:23:07 -0600 Subject: [PATCH 2/9] fixes --- lib/yargs-factory.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 8846338bf..613195306 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -905,9 +905,17 @@ export class YargsInstance { } // Prevent version name collision - if (key === 'version' || opt?.alias === 'version') { + // Addresses: https://github.com/yargs/yargs/issues/1979 + if (this.#versionOpt && (key === 'version' || opt?.alias === 'version')) { throw new YError( - '"version" is a reserved word.\nPlease use a different option key or use the built-in version method (if applicable).\nhttps://yargs.js.org/docs/#api-reference-version' + [ + '"version" is a reserved word.', + 'Please do one of the following:', + '- Disable version with `yargs.version(false)` if using "version" as an option', + '- Use the built-in `yargs.version` method instead (if applicable)', + '- Use a different option key', + 'https://yargs.js.org/docs/#api-reference-version', + ].join('\n') ); } From e04e882bcb4de117206e1d8c6f30e580141c9721 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Fri, 16 Jul 2021 13:59:53 -0600 Subject: [PATCH 3/9] tests --- test/usage.cjs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/usage.cjs b/test/usage.cjs index 22ab8782a..ad5920d52 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -5,6 +5,7 @@ const checkUsage = require('./helpers/utils.cjs').checkOutput; const chalk = require('chalk'); const yargs = require('../index.cjs'); +const expect = require('chai').expect; const {YError} = require('../build/index.cjs'); const should = require('chai').should(); @@ -1570,6 +1571,46 @@ describe('usage tests', () => { r.logs[0].should.eql('1.0.2'); }); + describe('when an option or alias "version" is set', () => { + it('fails unless false is first argument', done => { + yargs + .command('cmd1', 'cmd desc', yargs => + yargs.option('v', { + alias: 'version', + describe: 'version desc', + type: 'string', + }) + ) + .fail(msg => { + msg.should.match(/reserved word/); + return done(); + }) + .wrap(null) + .parse('cmd1 --version 0.25.10'); + }); + + it('allows if false is first argument', () => { + yargs + .command( + 'cmd1', + 'cmd1 desc', + yargs => + yargs.version(false).option('version', { + alias: 'v', + describe: 'version desc', + type: 'string', + }), + argv => { + argv.version.should.equal('0.25.10'); + } + ) + .fail(() => { + expect.fail(); + }) + .parse('cmd1 --version 0.25.10'); + }); + }); + describe('when exitProcess is false', () => { it('should not validate arguments (required argument)', () => { const r = checkUsage(() => From f165b0dbf58763cdee6fcac9df79d2fc262e15a3 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Fri, 16 Jul 2021 14:05:54 -0600 Subject: [PATCH 4/9] fixes --- test/usage.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/usage.cjs b/test/usage.cjs index ad5920d52..7061104f0 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1572,7 +1572,7 @@ describe('usage tests', () => { }); describe('when an option or alias "version" is set', () => { - it('fails unless false is first argument', done => { + it('fails unless version is disabled', done => { yargs .command('cmd1', 'cmd desc', yargs => yargs.option('v', { @@ -1589,7 +1589,7 @@ describe('usage tests', () => { .parse('cmd1 --version 0.25.10'); }); - it('allows if false is first argument', () => { + it('works if version is disabled', () => { yargs .command( 'cmd1', From 9f552a623f83be006bb7aced8281cf55e4361d62 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Fri, 16 Jul 2021 14:11:15 -0600 Subject: [PATCH 5/9] fixes --- test/usage.cjs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/usage.cjs b/test/usage.cjs index 7061104f0..8e03a9cbe 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1571,6 +1571,7 @@ describe('usage tests', () => { r.logs[0].should.eql('1.0.2'); }); + // Addresses: https://github.com/yargs/yargs/issues/1979 describe('when an option or alias "version" is set', () => { it('fails unless version is disabled', done => { yargs From c2ae4c1ace0ec6fd24f0abe0b51f7b81cc47ad4f Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Thu, 5 Aug 2021 20:05:16 -0600 Subject: [PATCH 6/9] fixes --- lib/platform-shims/browser.mjs | 2 ++ lib/platform-shims/cjs.ts | 1 + lib/platform-shims/deno.ts | 6 ++++++ lib/platform-shims/esm.mjs | 1 + lib/typings/common-types.ts | 1 + lib/yargs-factory.ts | 4 ++-- test/usage.cjs | 9 ++++----- 7 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/platform-shims/browser.mjs b/lib/platform-shims/browser.mjs index 5bba14653..a08d7845c 100644 --- a/lib/platform-shims/browser.mjs +++ b/lib/platform-shims/browser.mjs @@ -41,6 +41,8 @@ export default { process: { argv: () => [], cwd: () => '', + // eslint-disable-next-line no-unused-vars + emitWarning: (warning, name, code, ctor) => {}, execPath: () => '', // exit is noop browser: exit: () => {}, diff --git a/lib/platform-shims/cjs.ts b/lib/platform-shims/cjs.ts index 03f36f797..ac4b59db2 100644 --- a/lib/platform-shims/cjs.ts +++ b/lib/platform-shims/cjs.ts @@ -26,6 +26,7 @@ export default { process: { argv: () => process.argv, cwd: process.cwd, + emitWarning: process.emitWarning, execPath: () => process.execPath, exit: (code: number) => { // eslint-disable-next-line no-process-exit diff --git a/lib/platform-shims/deno.ts b/lib/platform-shims/deno.ts index 0c4436bfe..b24521ee7 100644 --- a/lib/platform-shims/deno.ts +++ b/lib/platform-shims/deno.ts @@ -82,6 +82,12 @@ export default { process: { argv: () => argv, cwd: () => cwd, + emitWarning: ( + warning: string | Error, + name?: string, + cod?: string, + ctor?: Function + ) => {}, execPath: () => { try { return Deno.execPath(); diff --git a/lib/platform-shims/esm.mjs b/lib/platform-shims/esm.mjs index bc0479162..5254adee7 100644 --- a/lib/platform-shims/esm.mjs +++ b/lib/platform-shims/esm.mjs @@ -45,6 +45,7 @@ export default { process: { argv: () => process.argv, cwd: process.cwd, + emitWarning: process.emitWarning, execPath: () => process.execPath, exit: process.exit, nextTick: process.nextTick, diff --git a/lib/typings/common-types.ts b/lib/typings/common-types.ts index 9b9bcfcda..9e9ae241a 100644 --- a/lib/typings/common-types.ts +++ b/lib/typings/common-types.ts @@ -103,6 +103,7 @@ export interface PlatformShim { process: { argv: () => string[]; cwd: () => string; + emitWarning: typeof process.emitWarning; execPath: () => string; exit: (code: number) => void; nextTick: (cb: Function) => void; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index f0921282e..d48484d07 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -904,10 +904,10 @@ export class YargsInstance { opt = {}; } - // Prevent version name collision + // Warn about version name collision // Addresses: https://github.com/yargs/yargs/issues/1979 if (this.#versionOpt && (key === 'version' || opt?.alias === 'version')) { - throw new YError( + this.#shim.process.emitWarning( [ '"version" is a reserved word.', 'Please do one of the following:', diff --git a/test/usage.cjs b/test/usage.cjs index 512134a15..8ba3c7043 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1573,7 +1573,7 @@ describe('usage tests', () => { // Addresses: https://github.com/yargs/yargs/issues/1979 describe('when an option or alias "version" is set', () => { - it('fails unless version is disabled', done => { + it('emits warning unless version is disabled', () => { yargs .command('cmd1', 'cmd desc', yargs => yargs.option('v', { @@ -1582,15 +1582,14 @@ describe('usage tests', () => { type: 'string', }) ) - .fail(msg => { - msg.should.match(/reserved word/); - return done(); + .fail(() => { + expect.fail(); }) .wrap(null) .parse('cmd1 --version 0.25.10'); }); - it('works if version is disabled', () => { + it('does not emit warning if version is disabled', () => { yargs .command( 'cmd1', From 2f8c1ecc5bbd8adc36abd8e12042d700e7889ce3 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Thu, 5 Aug 2021 23:04:19 -0600 Subject: [PATCH 7/9] fixes --- lib/platform-shims/browser.mjs | 2 +- lib/platform-shims/cjs.ts | 3 +- lib/platform-shims/deno.ts | 7 +--- lib/platform-shims/esm.mjs | 4 +- lib/typings/common-types.ts | 2 +- lib/yargs-factory.ts | 19 ++++++++- test/helpers/utils.cjs | 7 ++++ test/usage.cjs | 70 +++++++++++++++++++--------------- 8 files changed, 70 insertions(+), 44 deletions(-) diff --git a/lib/platform-shims/browser.mjs b/lib/platform-shims/browser.mjs index a08d7845c..6f9911e82 100644 --- a/lib/platform-shims/browser.mjs +++ b/lib/platform-shims/browser.mjs @@ -42,7 +42,7 @@ export default { argv: () => [], cwd: () => '', // eslint-disable-next-line no-unused-vars - emitWarning: (warning, name, code, ctor) => {}, + emitWarning: (warning, name) => {}, execPath: () => '', // exit is noop browser: exit: () => {}, diff --git a/lib/platform-shims/cjs.ts b/lib/platform-shims/cjs.ts index ac4b59db2..6a18223c2 100644 --- a/lib/platform-shims/cjs.ts +++ b/lib/platform-shims/cjs.ts @@ -26,7 +26,8 @@ export default { process: { argv: () => process.argv, cwd: process.cwd, - emitWarning: process.emitWarning, + emitWarning: (warning: string | Error, type?: string) => + process.emitWarning(warning, type), execPath: () => process.execPath, exit: (code: number) => { // eslint-disable-next-line no-process-exit diff --git a/lib/platform-shims/deno.ts b/lib/platform-shims/deno.ts index b24521ee7..6bea0422b 100644 --- a/lib/platform-shims/deno.ts +++ b/lib/platform-shims/deno.ts @@ -82,12 +82,7 @@ export default { process: { argv: () => argv, cwd: () => cwd, - emitWarning: ( - warning: string | Error, - name?: string, - cod?: string, - ctor?: Function - ) => {}, + emitWarning: (warning: string | Error, type?: string) => {}, execPath: () => { try { return Deno.execPath(); diff --git a/lib/platform-shims/esm.mjs b/lib/platform-shims/esm.mjs index 5254adee7..f81f7ea4e 100644 --- a/lib/platform-shims/esm.mjs +++ b/lib/platform-shims/esm.mjs @@ -3,7 +3,7 @@ import { notStrictEqual, strictEqual } from 'assert' import cliui from 'cliui' import escalade from 'escalade/sync' -import { format, inspect } from 'util' +import { inspect } from 'util' import { readFileSync } from 'fs' import { fileURLToPath } from 'url'; import Parser from 'yargs-parser' @@ -45,7 +45,7 @@ export default { process: { argv: () => process.argv, cwd: process.cwd, - emitWarning: process.emitWarning, + emitWarning: (warning, type) => process.emitWarning(warning, type), execPath: () => process.execPath, exit: process.exit, nextTick: process.nextTick, diff --git a/lib/typings/common-types.ts b/lib/typings/common-types.ts index 9e9ae241a..10e44d5ec 100644 --- a/lib/typings/common-types.ts +++ b/lib/typings/common-types.ts @@ -103,7 +103,7 @@ export interface PlatformShim { process: { argv: () => string[]; cwd: () => string; - emitWarning: typeof process.emitWarning; + emitWarning: (warning: string | Error, type?: string) => void; execPath: () => string; exit: (code: number) => void; nextTick: (cb: Function) => void; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index d48484d07..34275480f 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -88,6 +88,7 @@ export function YargsFactory(_shim: PlatformShim) { const kCopyDoubleDash = Symbol('copyDoubleDash'); const kCreateLogger = Symbol('copyDoubleDash'); const kDeleteFromParserHintObject = Symbol('deleteFromParserHintObject'); +const kEmitWarning = Symbol('emitWarning'); const kFreeze = Symbol('freeze'); const kGetDollarZero = Symbol('getDollarZero'); const kGetParserConfiguration = Symbol('getParserConfiguration'); @@ -172,6 +173,7 @@ export class YargsInstance { #defaultShowHiddenOpt = 'show-hidden'; #exitError: YError | string | undefined | null = null; #detectLocale = true; + #emittedWarnings: Dictionary = {}; #exitProcess = true; #frozens: FrozenYargsInstance[] = []; #globalMiddleware: GlobalMiddleware; @@ -907,7 +909,7 @@ export class YargsInstance { // Warn about version name collision // Addresses: https://github.com/yargs/yargs/issues/1979 if (this.#versionOpt && (key === 'version' || opt?.alias === 'version')) { - this.#shim.process.emitWarning( + this[kEmitWarning]( [ '"version" is a reserved word.', 'Please do one of the following:', @@ -915,7 +917,9 @@ export class YargsInstance { '- Use the built-in `yargs.version` method instead (if applicable)', '- Use a different option key', 'https://yargs.js.org/docs/#api-reference-version', - ].join('\n') + ].join('\n'), + undefined, + 'versionWarning' // TODO: better dedupeId ); } @@ -1464,6 +1468,17 @@ export class YargsInstance { // now delete the description from usage.js. delete this.#usage.getDescriptions()[optionKey]; } + [kEmitWarning]( + warning: string, + name: string | undefined, + deduplicationID: string + ) { + // prevent duplicate warning emissions + if (!this.#emittedWarnings[deduplicationID]) { + this.#shim.process.emitWarning(warning, name); + this.#emittedWarnings[deduplicationID] = true; + } + } [kFreeze]() { this.#frozens.push({ options: this.#options, diff --git a/test/helpers/utils.cjs b/test/helpers/utils.cjs index 8d8eef723..d4e6ca30e 100644 --- a/test/helpers/utils.cjs +++ b/test/helpers/utils.cjs @@ -9,6 +9,7 @@ exports.checkOutput = function checkOutput(f, argv, cb) { let exitCode = 0; const _exit = process.exit; const _emit = process.emit; + const _emitWarning = process.emitWarning; const _env = process.env; const _argv = process.argv; const _error = console.error; @@ -25,6 +26,7 @@ exports.checkOutput = function checkOutput(f, argv, cb) { const errors = []; const logs = []; const warnings = []; + const emittedWarnings = []; console.error = (...msg) => { errors.push(format(...msg)); @@ -35,6 +37,9 @@ exports.checkOutput = function checkOutput(f, argv, cb) { console.warn = (...msg) => { warnings.push(format(...msg)); }; + process.emitWarning = warning => { + emittedWarnings.push(warning); + }; let result; @@ -80,6 +85,7 @@ exports.checkOutput = function checkOutput(f, argv, cb) { process.emit = _emit; process.env = _env; process.argv = _argv; + process.emitWarning = _emitWarning; console.error = _error; console.log = _log; @@ -93,6 +99,7 @@ exports.checkOutput = function checkOutput(f, argv, cb) { errors, logs, warnings, + emittedWarnings, exit, exitCode, result, diff --git a/test/usage.cjs b/test/usage.cjs index 8ba3c7043..eef18facc 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1573,41 +1573,49 @@ describe('usage tests', () => { // Addresses: https://github.com/yargs/yargs/issues/1979 describe('when an option or alias "version" is set', () => { - it('emits warning unless version is disabled', () => { - yargs - .command('cmd1', 'cmd desc', yargs => - yargs.option('v', { - alias: 'version', - describe: 'version desc', - type: 'string', + it('emits warning if version is not disabled', () => { + const r = checkUsage(() => + yargs + .command('cmd1', 'cmd1 desc', yargs => + yargs.option('v', { + alias: 'version', + describe: 'version desc', + type: 'string', + }) + ) + .fail(() => { + expect.fail(); }) - ) - .fail(() => { - expect.fail(); - }) - .wrap(null) - .parse('cmd1 --version 0.25.10'); + .wrap(null) + .parse('cmd1 --version 0.25.10') + ); + r.should.have.property('emittedWarnings').with.length(1); + r.emittedWarnings[0].should.match(/reserved word/); }); it('does not emit warning if version is disabled', () => { - yargs - .command( - 'cmd1', - 'cmd1 desc', - yargs => - yargs.version(false).option('version', { - alias: 'v', - describe: 'version desc', - type: 'string', - }), - argv => { - argv.version.should.equal('0.25.10'); - } - ) - .fail(() => { - expect.fail(); - }) - .parse('cmd1 --version 0.25.10'); + const r = checkUsage(() => + yargs + .command( + 'cmd1', + 'cmd1 desc', + yargs => + yargs.version(false).option('version', { + alias: 'v', + describe: 'version desc', + type: 'string', + }), + argv => { + argv.version.should.equal('0.25.10'); + } + ) + .fail(() => { + expect.fail(); + }) + .parse('cmd1 --version 0.25.10') + ); + + r.should.have.property('emittedWarnings').with.length(0); }); }); From 84b57fff250cb542d788a9fc868cb0f8f1881c8c Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Thu, 5 Aug 2021 23:05:44 -0600 Subject: [PATCH 8/9] fixes --- lib/yargs-factory.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 34275480f..4ddf32eb2 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1470,12 +1470,12 @@ export class YargsInstance { } [kEmitWarning]( warning: string, - name: string | undefined, + type: string | undefined, deduplicationID: string ) { // prevent duplicate warning emissions if (!this.#emittedWarnings[deduplicationID]) { - this.#shim.process.emitWarning(warning, name); + this.#shim.process.emitWarning(warning, type); this.#emittedWarnings[deduplicationID] = true; } } From 5f73c587f08222daa189e9032ac87d4b8b3ce084 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Fri, 6 Aug 2021 07:05:28 -0600 Subject: [PATCH 9/9] oops, this isn't golang --- lib/platform-shims/browser.mjs | 1 - lib/yargs-factory.ts | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/platform-shims/browser.mjs b/lib/platform-shims/browser.mjs index 6f9911e82..5f8ec61f4 100644 --- a/lib/platform-shims/browser.mjs +++ b/lib/platform-shims/browser.mjs @@ -41,7 +41,6 @@ export default { process: { argv: () => [], cwd: () => '', - // eslint-disable-next-line no-unused-vars emitWarning: (warning, name) => {}, execPath: () => '', // exit is noop browser: diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 4ddf32eb2..7c39e2883 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1471,12 +1471,12 @@ export class YargsInstance { [kEmitWarning]( warning: string, type: string | undefined, - deduplicationID: string + deduplicationId: string ) { // prevent duplicate warning emissions - if (!this.#emittedWarnings[deduplicationID]) { + if (!this.#emittedWarnings[deduplicationId]) { this.#shim.process.emitWarning(warning, type); - this.#emittedWarnings[deduplicationID] = true; + this.#emittedWarnings[deduplicationId] = true; } } [kFreeze]() {