From 2d1136d303ea805685a973ded62f52efd49b78b9 Mon Sep 17 00:00:00 2001 From: Landon Yarrington <33426811+jly36963@users.noreply.github.com> Date: Sat, 9 Apr 2022 16:52:12 -0600 Subject: [PATCH] fix: coerce pollutes argv (#2161) --- lib/command.ts | 6 ++--- lib/yargs-factory.ts | 12 +++++++++- test/command.cjs | 53 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index b588b7e40..ff9da5b1b 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -603,9 +603,9 @@ export class CommandInstance { // If both positionals/options provided, no default was set, // and if at least one is an array: don't overwrite, combine. if ( - !Object.prototype.hasOwnProperty.call(defaults, key) && - Object.prototype.hasOwnProperty.call(argv, key) && - Object.prototype.hasOwnProperty.call(parsed.argv, key) && + !Object.hasOwnProperty.call(defaults, key) && + Object.hasOwnProperty.call(argv, key) && + Object.hasOwnProperty.call(parsed.argv, key) && (Array.isArray(argv[key]) || Array.isArray(parsed.argv[key])) ) { argv[key] = ([] as string[]).concat(argv[key], parsed.argv[key]); diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 69d882072..279d8bcce 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -387,6 +387,13 @@ export class YargsInstance { yargs: YargsInstance ): Partial | Promise> => { let aliases: Dictionary; + + // Skip coerce logic if related arg was not provided + const shouldCoerce = Object.hasOwnProperty.call(argv, keys); + if (!shouldCoerce) { + return argv; + } + return maybeAsyncResult< Partial | Promise> | any >( @@ -396,7 +403,10 @@ export class YargsInstance { }, (result: any): Partial => { argv[keys] = result; - if (aliases[keys]) { + const stripAliased = yargs + .getInternalMethods() + .getParserConfiguration()['strip-aliased']; + if (aliases[keys] && stripAliased !== true) { for (const alias of aliases[keys]) { argv[alias] = result; } diff --git a/test/command.cjs b/test/command.cjs index 189a16596..867f8f9b5 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1647,6 +1647,59 @@ describe('Command', () => { argv.rest.should.equal('bar baz'); coerceExecutionCount.should.equal(1); }); + + // Addresses: https://github.com/yargs/yargs/issues/2130 + it('should not run or set new properties on argv when related argument is not passed', () => { + yargs('cmd1') + .command( + 'cmd1', + 'cmd1 desc', + yargs => + yargs + .option('foo', {alias: 'f', type: 'string'}) + .option('bar', { + alias: 'b', + type: 'string', + implies: 'f', + coerce: () => expect.fail(), // Should not be called + }) + .fail(() => { + expect.fail(); // Should not fail because of implies + }), + argv => { + // eslint-disable-next-line no-prototype-builtins + if (Object.prototype.hasOwnProperty(argv, 'b')) { + expect.fail(); // 'b' was not provided, coerce should not set it + } + } + ) + .strict() + .parse(); + }); + + // Addresses: https://github.com/yargs/yargs/issues/2159 + it('should not add aliases to argv if strip-aliased config is true', () => { + yargs('cmd1 -f hello -b world') + .parserConfiguration({'strip-aliased': true}) + .command( + 'cmd1', + 'cmd1 desc', + yargs => + yargs.option('foo', {alias: 'f', type: 'string'}).option('bar', { + type: 'string', + alias: 'b', + coerce: val => val, + }), + argv => { + // eslint-disable-next-line no-prototype-builtins + if (Object.prototype.hasOwnProperty(argv, 'b')) { + expect.fail(); // 'b' is an alias, it should not be in argv + } + } + ) + .strict() + .parse(); + }); }); describe('defaults', () => {