fix(cli): cdk deploy -R does not disable rollback (#32514)

### Issue 

`cdk deploy -R` should be the same as `cdk deploy --no-rollback` or `cdk deploy --rollback=false`, but has no effect

### Reason for this change

PR #31850 introduced this bug by accidentally flipping the order of arguments passed to the `yargsNegativeAlias` helper. 
This caused the helper to have no effect.

### Description of changes

- Changed the codegen to fully generate negative aliases for the user
- Fixed the generate code to use the correct argument order again for `yargsNegativeAlias`
- Renamed the parameters to make it easier to understand.
- Made `nargs: 1` and `requiresArg: true` implied for all array options. Some options did miss one or both of these. This was an oversight.


### Description of how you validated changes

Added an explicit test case for `-R`.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: mrgrain

aws-cdk.code-workspace

@@ -30,7 +30,8 @@
       {
         "name": "aws-custom-resource-sdk-adapter",
         "rootPath": "packages/@aws-cdk/aws-custom-resource-sdk-adapter"
-      }
+      },
+      { "name": "yargs-gen", "rootPath": "tools/@aws-cdk/yargs-gen" }
     ]
   },
   "extensions": {

packages/aws-cdk/bin/cdk

@@ -1,6 +1,6 @@
 #!/usr/bin/env node
 // source maps must be enabled before importing files
 process.setSourceMapsEnabled(true);
-const { cli } = require("../lib/cli");
+const { cli } = require("../lib");
 
 cli();

packages/aws-cdk/lib/cli.ts

@@ -27,6 +27,7 @@ import { Notices } from '../lib/notices';
 import { Command, Configuration, Settings } from '../lib/settings';
 import * as version from '../lib/version';
 import { SdkToCliLogger } from './api/aws-auth/sdk-logger';
+import { yargsNegativeAlias } from './util/yargs-helpers';
 
 /* eslint-disable max-len */
 /* eslint-disable @typescript-eslint/no-shadow */ // yargs
@@ -527,18 +528,6 @@ function arrayFromYargs(xs: string[]): string[] | undefined {
   return xs.filter((x) => x !== '');
 }
 
-function yargsNegativeAlias<T extends { [x in S | L]: boolean | undefined }, S extends string, L extends string>(
-  shortName: S,
-  longName: L,
-): (argv: T) => T {
-  return (argv: T) => {
-    if (shortName in argv && argv[shortName]) {
-      (argv as any)[longName] = false;
-    }
-    return argv;
-  };
-}
-
 function determineHotswapMode(hotswap?: boolean, hotswapFallback?: boolean, watch?: boolean): HotswapMode {
   if (hotswap && hotswapFallback) {
     throw new Error('Can not supply both --hotswap and --hotswap-fallback at the same time');

packages/aws-cdk/lib/config.ts

@@ -13,8 +13,8 @@ export function makeConfig(): CliConfig {
     globalOptions: {
       'app': { type: 'string', alias: 'a', desc: 'REQUIRED WHEN RUNNING APP: command-line for executing your app or a cloud assembly directory (e.g. "node bin/my-app.js"). Can also be specified in cdk.json or ~/.cdk.json', requiresArg: true },
       'build': { type: 'string', desc: 'Command-line for a pre-synth build' },
-      'context': { type: 'array', alias: 'c', desc: 'Add contextual string parameter (KEY=VALUE)', nargs: 1, requiresArg: true },
-      'plugin': { type: 'array', alias: 'p', desc: 'Name or path of a node package that extend the CDK features. Can be specified multiple times', nargs: 1 },
+      'context': { type: 'array', alias: 'c', desc: 'Add contextual string parameter (KEY=VALUE)' },
+      'plugin': { type: 'array', alias: 'p', desc: 'Name or path of a node package that extend the CDK features. Can be specified multiple times' },
       'trace': { type: 'boolean', desc: 'Print trace for stack warnings' },
       'strict': { type: 'boolean', desc: 'Do not construct stacks with warnings' },
       'lookups': { type: 'boolean', desc: 'Perform context lookups (synthesis fails if this is disabled and context lookups need to be performed)', default: true },
@@ -77,11 +77,11 @@ export function makeConfig(): CliConfig {
           'bootstrap-customer-key': { type: 'boolean', desc: 'Create a Customer Master Key (CMK) for the bootstrap bucket (you will be charged but can customize permissions, modern bootstrapping only)', default: undefined, conflicts: 'bootstrap-kms-key-id' },
           'qualifier': { type: 'string', desc: 'String which must be unique for each bootstrap stack. You must configure it on your CDK app if you change this from the default.', default: undefined },
           'public-access-block-configuration': { type: 'boolean', desc: 'Block public access configuration on CDK toolkit bucket (enabled by default) ', default: undefined },
-          'tags': { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', nargs: 1, requiresArg: true, default: [] },
+          'tags': { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', default: [] },
           'execute': { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true },
-          'trust': { type: 'array', desc: 'The AWS account IDs that should be trusted to perform deployments into this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true },
-          'trust-for-lookup': { type: 'array', desc: 'The AWS account IDs that should be trusted to look up values in this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true },
-          'cloudformation-execution-policies': { type: 'array', desc: 'The Managed Policy ARNs that should be attached to the role performing deployments into this environment (may be repeated, modern bootstrapping only)', default: [], nargs: 1, requiresArg: true },
+          'trust': { type: 'array', desc: 'The AWS account IDs that should be trusted to perform deployments into this environment (may be repeated, modern bootstrapping only)', default: [] },
+          'trust-for-lookup': { type: 'array', desc: 'The AWS account IDs that should be trusted to look up values in this environment (may be repeated, modern bootstrapping only)', default: [] },
+          'cloudformation-execution-policies': { type: 'array', desc: 'The Managed Policy ARNs that should be attached to the role performing deployments into this environment (may be repeated, modern bootstrapping only)', default: [] },
           'force': { alias: 'f', type: 'boolean', desc: 'Always bootstrap even if it would downgrade template version', default: false },
           'termination-protection': { type: 'boolean', default: undefined, desc: 'Toggle CloudFormation termination protection on the bootstrap stacks' },
           'show-template': { type: 'boolean', desc: 'Instead of actual bootstrapping, print the current CLI\'s bootstrapping template to stdout for customization', default: false },
@@ -109,12 +109,12 @@ export function makeConfig(): CliConfig {
         description: 'Deploys the stack(s) named STACKS into your AWS account',
         options: {
           'all': { type: 'boolean', desc: 'Deploy all available stacks', default: false },
-          'build-exclude': { type: 'array', alias: 'E', nargs: 1, desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
+          'build-exclude': { type: 'array', alias: 'E', desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
           'exclusively': { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' },
           'require-approval': { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' },
-          'notification-arns': { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events. These will be added to ARNs specified with the \'notificationArns\' stack property.', nargs: 1, requiresArg: true },
+          'notification-arns': { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events. These will be added to ARNs specified with the \'notificationArns\' stack property.' },
           // @deprecated(v2) -- tags are part of the Cloud Assembly and tags specified here will be overwritten on the next deployment
-          'tags': { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)', nargs: 1, requiresArg: true },
+          'tags': { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)' },
           'execute': { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet) (deprecated)', deprecated: true },
           'change-set-name': { type: 'string', desc: 'Name of the CloudFormation change set to create (only if method is not direct)' },
           'method': {
@@ -125,7 +125,7 @@ export function makeConfig(): CliConfig {
             desc: 'How to perform the deployment. Direct is a bit faster but lacks progress information',
           },
           'force': { alias: 'f', type: 'boolean', desc: 'Always deploy stack even if templates are identical', default: false },
-          'parameters': { type: 'array', desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)', nargs: 1, requiresArg: true, default: {} },
+          'parameters': { type: 'array', desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)', default: {} },
           'outputs-file': { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true },
           'previous-parameters': { type: 'boolean', default: true, desc: 'Use previous values for existing parameters (you must specify all parameters on every deployment if this is disabled)' },
           'toolkit-stack-name': { type: 'string', desc: 'The name of the existing CDK toolkit stack (only used for app using legacy synthesis)', requiresArg: true },
@@ -136,11 +136,6 @@ export function makeConfig(): CliConfig {
               'Note: do **not** disable this flag for deployments with resource replacements, as that will always fail',
             negativeAlias: 'R',
           },
-          'R': {
-            type: 'boolean',
-            hidden: true,
-            // Hack to get '-R' as an alias for '--no-rollback', suggested by: https://github.com/yargs/yargs/issues/1729
-          },
           'hotswap': {
             type: 'boolean',
             desc: "Attempts to perform a 'hotswap' deployment, " +
@@ -199,8 +194,6 @@ export function makeConfig(): CliConfig {
           'orphan': {
             // alias: 'o' conflicts with --output
             type: 'array',
-            nargs: 1,
-            requiresArg: true,
             desc: 'Orphan the given resources, identified by their logical ID (can be specified multiple times)',
             default: [],
           },
@@ -261,7 +254,7 @@ export function makeConfig(): CliConfig {
           // .option('previous-parameters', { type: 'boolean', default: true, desc: 'Use previous values for existing parameters (you must specify all parameters on every deployment if this is disabled)' })
           // .option('outputs-file', { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true })
           // .option('notification-arns', { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true })
-          'build-exclude': { type: 'array', alias: 'E', nargs: 1, desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
+          'build-exclude': { type: 'array', alias: 'E', desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
           'exclusively': { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' },
           'change-set-name': { type: 'string', desc: 'Name of the CloudFormation change set to create' },
           'force': { alias: 'f', type: 'boolean', desc: 'Always deploy stack even if templates are identical', default: false },
@@ -271,12 +264,7 @@ export function makeConfig(): CliConfig {
             type: 'boolean',
             desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. " +
               'Note: do **not** disable this flag for deployments with resource replacements, as that will always fail',
-            negativeAlias: '-R',
-          },
-          // same hack for -R as above in 'deploy'
-          'R': {
-            type: 'boolean',
-            hidden: true,
+            negativeAlias: 'R',
           },
           'hotswap': {
             type: 'boolean',
@@ -436,7 +424,7 @@ export class DynamicValue {
   public static fromInline(f: () => any): DynamicResult {
     return {
       dynamicType: 'function',
-      dynamicValue: f,
+      dynamicValue: f.toString(),
     };
   }
 }

packages/aws-cdk/lib/parse-command-line-arguments.ts

@@ -39,6 +39,7 @@ export function parseCommandLineArguments(
       alias: 'p',
       desc: 'Name or path of a node package that extend the CDK features. Can be specified multiple times',
       nargs: 1,
+      requiresArg: true,
     })
     .option('trace', {
       type: 'boolean',
@@ -148,6 +149,8 @@ export function parseCommandLineArguments(
       type: 'array',
       desc: 'Opt in to unstable features. The flag indicates that the scope and API of a feature might still change. Otherwise the feature is generally production ready and fully supported. Can be specified multiple times.',
       default: [],
+      nargs: 1,
+      requiresArg: true,
     })
     .command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', (yargs: Argv) =>
       yargs
@@ -231,9 +234,9 @@ export function parseCommandLineArguments(
           type: 'array',
           alias: 't',
           desc: 'Tags to add for the stack (KEY=VALUE)',
+          default: [],
           nargs: 1,
           requiresArg: true,
-          default: [],
         })
         .option('execute', {
           type: 'boolean',
@@ -339,9 +342,10 @@ export function parseCommandLineArguments(
         .option('build-exclude', {
           type: 'array',
           alias: 'E',
-          nargs: 1,
           desc: 'Do not rebuild asset with the given ID. Can be specified multiple times',
           default: [],
+          nargs: 1,
+          requiresArg: true,
         })
         .option('exclusively', {
           type: 'boolean',
@@ -391,9 +395,9 @@ export function parseCommandLineArguments(
         .option('parameters', {
           type: 'array',
           desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)',
+          default: {},
           nargs: 1,
           requiresArg: true,
-          default: {},
         })
         .option('outputs-file', {
           type: 'string',
@@ -420,11 +424,8 @@ export function parseCommandLineArguments(
           type: 'boolean',
           desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. Note: do **not** disable this flag for deployments with resource replacements, as that will always fail",
         })
-        .middleware(yargsNegativeAlias('rollback', 'R'), true)
-        .option('R', {
-          type: 'boolean',
-          hidden: true,
-        })
+        .middleware(yargsNegativeAlias('R', 'rollback'), true)
+        .option('R', { type: 'boolean', hidden: true })
         .option('hotswap', {
           type: 'boolean',
           desc: "Attempts to perform a 'hotswap' deployment, but does not fall back to a full deployment if that is not possible. Instead, changes to any non-hotswappable properties are ignored.Do not use this in production environments",
@@ -486,10 +487,10 @@ export function parseCommandLineArguments(
         })
         .option('orphan', {
           type: 'array',
-          nargs: 1,
-          requiresArg: true,
           desc: 'Orphan the given resources, identified by their logical ID (can be specified multiple times)',
           default: [],
+          nargs: 1,
+          requiresArg: true,
         })
     )
     .command('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) =>
@@ -535,9 +536,10 @@ export function parseCommandLineArguments(
         .option('build-exclude', {
           type: 'array',
           alias: 'E',
-          nargs: 1,
           desc: 'Do not rebuild asset with the given ID. Can be specified multiple times',
           default: [],
+          nargs: 1,
+          requiresArg: true,
         })
         .option('exclusively', {
           type: 'boolean',
@@ -568,11 +570,8 @@ export function parseCommandLineArguments(
           type: 'boolean',
           desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. Note: do **not** disable this flag for deployments with resource replacements, as that will always fail",
         })
-        .middleware(yargsNegativeAlias('rollback', '-R'), true)
-        .option('R', {
-          type: 'boolean',
-          hidden: true,
-        })
+        .middleware(yargsNegativeAlias('R', 'rollback'), true)
+        .option('R', { type: 'boolean', hidden: true })
         .option('hotswap', {
           type: 'boolean',
           desc: "Attempts to perform a 'hotswap' deployment, but does not fall back to a full deployment if that is not possible. Instead, changes to any non-hotswappable properties are ignored.'true' by default, use --no-hotswap to turn off",
@@ -734,6 +733,8 @@ export function parseCommandLineArguments(
         .option('filter', {
           type: 'array',
           desc: 'Filters the resource scan based on the provided criteria in the following format: "key1=value1,key2=value2"\n This field can be passed multiple times for OR style filtering: \n filtering options: \n resource-identifier: A key-value pair that identifies the target resource. i.e. {"ClusterName", "myCluster"}\n resource-type-prefix: A string that represents a type-name prefix. i.e. "AWS::DynamoDB::"\n tag-key: a string that matches resources with at least one tag with the provided key. i.e. "myTagKey"\n tag-value: a string that matches resources with at least one tag with the provided value. i.e. "myTagValue"',
+          nargs: 1,
+          requiresArg: true,
         })
         .option('compress', {
           type: 'boolean',

packages/aws-cdk/lib/util/yargs-helpers.ts

@@ -0,0 +1,21 @@
+/**
+ * yargs middleware to negate an option if a negative alias is provided
+ * E.g. `-R` will imply `--rollback=false`
+ *
+ * @param optionToNegate The name of the option to negate, e.g. `rollback`
+ * @param negativeAlias The alias that should negate the option, e.g. `R`
+ * @returns
+ */
+export function yargsNegativeAlias<T extends { [x in S | L]: boolean | undefined }, S extends string, L extends string>(
+  negativeAlias: S,
+  optionToNegate: L,
+): (argv: T) => T {
+  return (argv: T) => {
+    // if R in argv && argv[R]
+    // then argv[rollback] = false
+    if (negativeAlias in argv && argv[negativeAlias]) {
+      (argv as any)[optionToNegate] = false;
+    }
+    return argv;
+  };
+}

packages/aws-cdk/test/parse-command-line-arguments.test.ts

@@ -0,0 +1,8 @@
+import { parseCommandLineArguments } from '../lib/parse-command-line-arguments';
+import { yargsNegativeAlias } from '../lib/util/yargs-helpers';
+
+test('cdk deploy -R sets rollback to false', async () => {
+  const argv = await parseCommandLineArguments(['deploy', '-R'], 'open', ['typescript'], ['typescript'], 'test', yargsNegativeAlias);
+
+  expect(argv.rollback).toBe(false);
+});