From 6b23e4c54f9e6b96333e1dba83864180af826e92 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Tue, 5 Apr 2022 20:22:30 -0600 Subject: [PATCH 1/4] Prevent coerce logic when related arg is not passed --- lib/yargs-factory.ts | 8 ++++++++ test/command.cjs | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 2dff7e395..22f3698fc 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -383,6 +383,14 @@ export class YargsInstance { yargs: YargsInstance ): Partial | Promise> => { let aliases: Dictionary; + + // Skip coerce logic if related arg was not provided + // Addresses: https://github.com/yargs/yargs/issues/2130 + const shouldCoerce = Object.prototype.hasOwnProperty.call(argv, keys); + if (!shouldCoerce) { + return argv; + } + return maybeAsyncResult< Partial | Promise> | any >( diff --git a/test/command.cjs b/test/command.cjs index 189a16596..f37bdda3e 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1647,6 +1647,35 @@ 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(); + }); }); describe('defaults', () => { From 4d86088a672a9933722319859ab6f2288e4d519b Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Wed, 6 Apr 2022 17:02:44 -0600 Subject: [PATCH 2/4] Fix strip-aliased coerce bug --- lib/yargs-factory.ts | 5 ++++- test/command.cjs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 5f248cccb..6472e413f 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -404,7 +404,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 f37bdda3e..fdb792699 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1676,6 +1676,29 @@ describe('Command', () => { .strict() .parse(); }); + + 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', () => { From adf0ccbd0c8b787af4c43e7f22c4c18929a08595 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Wed, 6 Apr 2022 17:12:21 -0600 Subject: [PATCH 3/4] cleanup --- lib/yargs-factory.ts | 1 - test/command.cjs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 6472e413f..47dbb53a5 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -389,7 +389,6 @@ export class YargsInstance { let aliases: Dictionary; // Skip coerce logic if related arg was not provided - // Addresses: https://github.com/yargs/yargs/issues/2130 const shouldCoerce = Object.prototype.hasOwnProperty.call(argv, keys); if (!shouldCoerce) { return argv; diff --git a/test/command.cjs b/test/command.cjs index fdb792699..867f8f9b5 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1677,6 +1677,7 @@ describe('Command', () => { .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}) From 4b2dfc8fca26d489ff36c1b16fadd086f8151a79 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Sat, 9 Apr 2022 14:37:56 -0600 Subject: [PATCH 4/4] nit --- lib/command.ts | 6 +++--- lib/yargs-factory.ts | 2 +- 2 files changed, 4 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 47dbb53a5..279d8bcce 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -389,7 +389,7 @@ export class YargsInstance { let aliases: Dictionary; // Skip coerce logic if related arg was not provided - const shouldCoerce = Object.prototype.hasOwnProperty.call(argv, keys); + const shouldCoerce = Object.hasOwnProperty.call(argv, keys); if (!shouldCoerce) { return argv; }