fix(cli): array arguments in cdk.json are ignored (#33107)

### Issue # (if applicable)

Closes #32814

### Reason for this change

yargs treats arrays weirdly. setting `default: undefined` results in the default array as `['undefined']`. So instead, I set the default to be `default: []`. However, this triggers an issue with the order of combining all arguments, and the CLI default was overriding any values in `cdk.json` for array types. So instead, we must omit the `default` property entirely for arrays, in order to achieve the desired behavior of `undefined` as default.

### Description of changes

Updated code generation to generate NO default property for array types

### Description of how you validated changes

tests in `user-input-gen` and the diff of `parse-command-line-arguments.ts` show that unless already defined, we are omitting defaults for arrays

### 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*

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

@@ -24,15 +24,13 @@ export function parseCommandLineArguments(args: Array<string>): any {
       desc: 'Command-line for a pre-synth build',
     })
     .option('context', {
-      default: [],
       type: 'array',
       alias: 'c',
       desc: 'Add contextual string parameter (KEY=VALUE)',
       nargs: 1,
       requiresArg: true,
     })
     .option('plugin', {
-      default: [],
       type: 'array',
       alias: 'p',
       desc: 'Name or path of a node package that extend the CDK features. Can be specified multiple times',
@@ -151,9 +149,9 @@ export function parseCommandLineArguments(args: Array<string>): any {
       desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr',
     })
     .option('unstable', {
-      default: [],
       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,
     })
@@ -237,10 +235,10 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'Block public access configuration on CDK toolkit bucket (enabled by default) ',
         })
         .option('tags', {
-          default: [],
           type: 'array',
           alias: 't',
           desc: 'Tags to add for the stack (KEY=VALUE)',
+          default: [],
           nargs: 1,
           requiresArg: true,
         })
@@ -250,30 +248,30 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)',
         })
         .option('trust', {
-          default: [],
           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,
         })
         .option('trust-for-lookup', {
-          default: [],
           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,
         })
         .option('untrust', {
-          default: [],
           type: 'array',
           desc: 'The AWS account IDs that should not be trusted by this environment (may be repeated, modern bootstrapping only)',
+          default: [],
           nargs: 1,
           requiresArg: true,
         })
         .option('cloudformation-execution-policies', {
-          default: [],
           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,
         })
@@ -356,10 +354,10 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'Deploy all available stacks',
         })
         .option('build-exclude', {
-          default: [],
           type: 'array',
           alias: 'E',
           desc: 'Do not rebuild asset with the given ID. Can be specified multiple times',
+          default: [],
           nargs: 1,
           requiresArg: true,
         })
@@ -382,7 +380,6 @@ export function parseCommandLineArguments(args: Array<string>): any {
           requiresArg: true,
         })
         .option('tags', {
-          default: [],
           type: 'array',
           alias: 't',
           desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)',
@@ -420,9 +417,9 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'Always deploy stack even if templates are identical',
         })
         .option('parameters', {
-          default: {},
           type: 'array',
           desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)',
+          default: {},
           nargs: 1,
           requiresArg: true,
         })
@@ -524,9 +521,9 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: "Whether to validate the bootstrap stack version. Defaults to 'true', disable with --no-validate-bootstrap-version.",
         })
         .option('orphan', {
-          default: [],
           type: 'array',
           desc: 'Orphan the given resources, identified by their logical ID (can be specified multiple times)',
+          default: [],
           nargs: 1,
           requiresArg: true,
         }),
@@ -578,10 +575,10 @@ export function parseCommandLineArguments(args: Array<string>): any {
     .command('watch [STACKS..]', "Shortcut for 'deploy --watch'", (yargs: Argv) =>
       yargs
         .option('build-exclude', {
-          default: [],
           type: 'array',
           alias: 'E',
           desc: 'Do not rebuild asset with the given ID. Can be specified multiple times',
+          default: [],
           nargs: 1,
           requiresArg: true,
         })
@@ -796,7 +793,6 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'Determines if a new scan should be created, or the last successful existing scan should be used \n options are "new" or "most-recent"',
         })
         .option('filter', {
-          default: [],
           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,

packages/aws-cdk/lib/cli/user-input.ts

@@ -143,14 +143,14 @@ export interface GlobalOptions {
   /**
    * Add contextual string parameter (KEY=VALUE)
    *
-   * @default - []
+   * @default - undefined
    */
   readonly context?: Array<string>;
 
   /**
    * Name or path of a node package that extend the CDK features. Can be specified multiple times
    *
-   * @default - []
+   * @default - undefined
    */
   readonly plugin?: Array<string>;
 
@@ -632,7 +632,7 @@ export interface DeployOptions {
    *
    * aliases: t
    *
-   * @default - []
+   * @default - undefined
    */
   readonly tags?: Array<string>;
 
@@ -1264,7 +1264,7 @@ export interface MigrateOptions {
    *  tag-key: a string that matches resources with at least one tag with the provided key. i.e. "myTagKey"
    *  tag-value: a string that matches resources with at least one tag with the provided value. i.e. "myTagValue"
    *
-   * @default - []
+   * @default - undefined
    */
   readonly filter?: Array<string>;
 

packages/aws-cdk/test/cli/cli-arguments.test.ts

@@ -14,11 +14,11 @@ describe('yargs', () => {
         assetMetadata: undefined,
         build: undefined,
         caBundlePath: undefined,
-        context: [],
+        context: undefined,
         ignoreErrors: false,
         noColor: false,
         pathMetadata: undefined,
-        plugin: [],
+        plugin: undefined,
         profile: undefined,
         proxy: undefined,
         roleArn: undefined,
@@ -60,7 +60,7 @@ describe('yargs', () => {
         progress: undefined,
         requireApproval: undefined,
         rollback: false,
-        tags: [],
+        tags: undefined,
         toolkitStackName: undefined,
         watch: undefined,
       },

packages/aws-cdk/test/cli/user-config.test.ts

@@ -3,6 +3,7 @@ import * as os from 'os';
 import * as fs_path from 'path';
 import * as fs from 'fs-extra';
 import { Configuration, PROJECT_CONFIG, PROJECT_CONTEXT } from '../../lib/cli/user-configuration';
+import { parseCommandLineArguments } from '../../lib/cli/parse-command-line-arguments';
 
 // mock fs deeply
 jest.mock('fs-extra');
@@ -112,3 +113,33 @@ test('Can specify the `quiet` key in the user config', async () => {
 
   expect(config.settings.get(['quiet'])).toBe(true);
 });
+
+test('array settings are not overridden by yarg defaults', async () => {
+  // GIVEN
+  const GIVEN_CONFIG: Map<string, any> = new Map([
+    [PROJECT_CONFIG, {
+      plugin: ['dummy'],
+    }],
+  ]);
+  const argsWithPlugin = await parseCommandLineArguments(['ls', '--plugin', '[]']);
+  const argsWithoutPlugin = await parseCommandLineArguments(['ls']);
+
+  // WHEN
+  mockedFs.pathExists.mockImplementation(path => {
+    return GIVEN_CONFIG.has(path);
+  });
+  mockedFs.readJSON.mockImplementation(path => {
+    return GIVEN_CONFIG.get(path);
+  });
+
+  const configWithPlugin = await new Configuration({
+    commandLineArguments: argsWithPlugin,
+  }).load();
+  const configWithoutPlugin = await new Configuration({
+    commandLineArguments: argsWithoutPlugin,
+  }).load();
+
+  // THEN
+  expect(configWithPlugin.settings.get(['plugin'])).toEqual(['[]']);
+  expect(configWithoutPlugin.settings.get(['plugin'])).toEqual(['dummy']);
+});

tools/@aws-cdk/user-input-gen/lib/user-input-gen.ts

@@ -1,7 +1,7 @@
 import { Module, SelectiveModuleImport, StructType, Type, TypeScriptRenderer } from '@cdklabs/typewriter';
 import { EsLintRules } from '@cdklabs/typewriter/lib/eslint-rules';
 import * as prettier from 'prettier';
-import { generateDefault, kebabToCamelCase, kebabToPascal } from './util';
+import { kebabToCamelCase, kebabToPascal } from './util';
 import { CliConfig } from './yargs-types';
 
 export async function renderUserInputType(config: CliConfig): Promise<string> {
@@ -46,7 +46,7 @@ export async function renderUserInputType(config: CliConfig): Promise<string> {
       name: kebabToCamelCase(optionName),
       type: convertType(option.type, option.count),
       docs: {
-        default: normalizeDefault(option.default, option.type),
+        default: normalizeDefault(option.default),
         summary: option.desc,
         deprecated: option.deprecated ? String(option.deprecated) : undefined,
       },
@@ -81,7 +81,7 @@ export async function renderUserInputType(config: CliConfig): Promise<string> {
         type: convertType(option.type, option.count),
         docs: {
           // Notification Arns is a special property where undefined and [] mean different things
-          default: optionName === 'notification-arns' ? 'undefined' : normalizeDefault(option.default, option.type),
+          default: optionName === 'notification-arns' ? 'undefined' : normalizeDefault(option.default),
           summary: option.desc,
           deprecated: option.deprecated ? String(option.deprecated) : undefined,
           remarks: option.alias ? `aliases: ${Array.isArray(option.alias) ? option.alias.join(' ') : option.alias}` : undefined,
@@ -140,7 +140,7 @@ function convertType(type: 'string' | 'array' | 'number' | 'boolean' | 'count',
   }
 }
 
-function normalizeDefault(defaultValue: any, type: string): string {
+function normalizeDefault(defaultValue: any): string {
   switch (typeof defaultValue) {
     case 'boolean':
     case 'string':
@@ -153,7 +153,6 @@ function normalizeDefault(defaultValue: any, type: string): string {
     case 'undefined':
     case 'function':
     default:
-      const generatedDefault = generateDefault(type);
-      return generatedDefault ? JSON.stringify(generatedDefault) : 'undefined';
+      return 'undefined';
   }
 }

tools/@aws-cdk/user-input-gen/lib/util.ts

@@ -1,9 +1,5 @@
 import { code, Expression } from '@cdklabs/typewriter';
 
-export function generateDefault(type: string) {
-  return type === 'array' ? [] : undefined;
-}
-
 export function lit(value: any): Expression {
   switch (value) {
     case undefined:

tools/@aws-cdk/user-input-gen/lib/yargs-gen.ts

@@ -1,7 +1,7 @@
 import { $E, Expression, ExternalModule, FreeFunction, IScope, Module, SelectiveModuleImport, Statement, ThingSymbol, Type, TypeScriptRenderer, code, expr } from '@cdklabs/typewriter';
 import { EsLintRules } from '@cdklabs/typewriter/lib/eslint-rules';
 import * as prettier from 'prettier';
-import { generateDefault, lit } from './util';
+import { lit } from './util';
 import { CliConfig, CliOption, YargsOption } from './yargs-types';
 
 // to import lodash.clonedeep properly, we would need to set esModuleInterop: true
@@ -114,10 +114,10 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: CliOpt
   let optionsExpr = prefix;
   for (const option of Object.keys(options)) {
     const theOption: CliOption = {
-      // Make the default explicit (overridden if the option includes an actual default)
-      // 'notification-arns' is a special snowflake that should be defaulted to 'undefined', but https://github.com/yargs/yargs/issues/2443
-      // prevents us from doing so. This should be changed if the issue is resolved.
-      ...(option === 'notification-arns' ? {} : { default: generateDefault(options[option].type) }),
+      // https://github.com/yargs/yargs/issues/2443 prevents us from supplying 'undefined' as the default
+      // for array types, because this turns into ['undefined']. The only way to achieve yargs' default is
+      // to provide no default.
+      ...(options[option].type == 'array' ? {} : { default: undefined }),
       ...options[option],
     };
     const optionProps: YargsOption = cloneDeep(theOption);

tools/@aws-cdk/user-input-gen/test/user-input-gen.test.ts

@@ -110,7 +110,7 @@ describe('render', () => {
         /**
          * plugins to load
          *
-         * @default - []
+         * @default - undefined
          */
         readonly plugin?: Array<string>;
       }
@@ -205,7 +205,7 @@ describe('render', () => {
         /**
          * Other array
          *
-         * @default - []
+         * @default - undefined
          */
         readonly otherArray?: Array<string>;
       }

tools/@aws-cdk/user-input-gen/test/yargs-gen.test.ts

@@ -50,7 +50,6 @@ describe('render', () => {
             desc: 'text for two',
           })
           .option('three', {
-            default: [],
             type: 'array',
             alias: 't',
             desc: 'text for three',
@@ -198,7 +197,6 @@ describe('render', () => {
                 requiresArg: true,
               })
               .option('other-array', {
-                default: [],
                 type: 'array',
                 desc: 'Other array',
                 nargs: 1,