fix: coerce pollutes argv (#2161)

yargs · Apr 9, 2022 · 2d1136d · 2d1136d
1 parent ad9fcac
commit 2d1136d
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 4 deletions.
diff --git 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
@@ -387,6 +387,13 @@ export class YargsInstance {
         yargs: YargsInstance
       ): Partial<Arguments> | Promise<Partial<Arguments>> => {
         let aliases: Dictionary<string[]>;
+
+        // Skip coerce logic if related arg was not provided
+        const shouldCoerce = Object.hasOwnProperty.call(argv, keys);
+        if (!shouldCoerce) {
+          return argv;
+        }
+
         return maybeAsyncResult<
           Partial<Arguments> | Promise<Partial<Arguments>> | any
         >(
@@ -396,7 +403,10 @@ export class YargsInstance {
           },
           (result: any): Partial<Arguments> => {
             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
@@ -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', () => {