Skip to content

Commit

Permalink
chore(cli): model positional arguments in CliArguments (#32718)
Browse files Browse the repository at this point in the history
**This PR does not change CLI functionality (yet)**

This PR models positional arguments in `CliArguments`. Currently they are supposed to be modeled as part of the the default property `_: [Command, ...string]`. This means that in `cli.ts` we mean to use it as so: `args._[1]` instead of `args.ID`. This is a downgrade, so I've updated the following:

Positional arguments are now modeled in `CliArguments` so if the `deploy` command has `STACKS` argument, it shows up as part of its `DeployOptions`. With this model we will replace `args.ID` with` args.acknowledge.ID` which I believe to be acceptable.

Now, the default property is modeled as `_: Command` so that we don't duplicate information.

### Checklist
- [d] 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*
  • Loading branch information
kaizencc authored Jan 5, 2025
1 parent dd6a2f4 commit caf2415
Show file tree
Hide file tree
Showing 7 changed files with 241 additions and 16 deletions.
73 changes: 69 additions & 4 deletions packages/aws-cdk/lib/cli-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { Command } from './settings';
*/
export interface CliArguments {
/**
* The CLI command name followed by any properties of the command
* The CLI command name
*/
readonly _: [Command, ...string[]];
readonly _: Command;

/**
* Global options available to all CLI commands
Expand Down Expand Up @@ -327,6 +327,11 @@ export interface ListOptions {
* @default - false
*/
readonly showDependencies?: boolean;

/**
* Positional argument for list
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -361,6 +366,11 @@ export interface SynthesizeOptions {
* @default - false
*/
readonly quiet?: boolean;

/**
* Positional argument for synthesize
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -504,6 +514,11 @@ export interface BootstrapOptions {
* @default - true
*/
readonly previousParameters?: boolean;

/**
* Positional argument for bootstrap
*/
readonly ENVIRONMENTS?: Array<string>;
}

/**
Expand Down Expand Up @@ -553,6 +568,11 @@ export interface GcOptions {
* @default - undefined
*/
readonly bootstrapStackName?: string;

/**
* Positional argument for gc
*/
readonly ENVIRONMENTS?: Array<string>;
}

/**
Expand Down Expand Up @@ -748,6 +768,11 @@ export interface DeployOptions {
* @default - false
*/
readonly ignoreNoStacks?: boolean;

/**
* Positional argument for deploy
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -792,6 +817,11 @@ export interface RollbackOptions {
* @default - []
*/
readonly orphan?: Array<string>;

/**
* Positional argument for rollback
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -854,6 +884,11 @@ export interface ImportOptions {
* @default - undefined
*/
readonly resourceMapping?: string;

/**
* Positional argument for import
*/
readonly STACK?: string;
}

/**
Expand Down Expand Up @@ -944,6 +979,11 @@ export interface WatchOptions {
* @default - 1
*/
readonly concurrency?: number;

/**
* Positional argument for watch
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -976,6 +1016,11 @@ export interface DestroyOptions {
* @default - undefined
*/
readonly force?: boolean;

/**
* Positional argument for destroy
*/
readonly STACKS?: Array<string>;
}

/**
Expand Down Expand Up @@ -1052,14 +1097,24 @@ export interface DiffOptions {
* @default - true
*/
readonly changeSet?: boolean;

/**
* Positional argument for diff
*/
readonly STACKS?: Array<string>;
}

/**
* Returns all metadata associated with this stack
*
* @struct
*/
export interface MetadataOptions {}
export interface MetadataOptions {
/**
* Positional argument for metadata
*/
readonly STACK?: string;
}

/**
* Acknowledge a notice so that it does not show up anymore
Expand All @@ -1068,7 +1123,12 @@ export interface MetadataOptions {}
*
* @struct
*/
export interface AcknowledgeOptions {}
export interface AcknowledgeOptions {
/**
* Positional argument for acknowledge
*/
readonly ID?: string;
}

/**
* Returns a list of relevant notices
Expand Down Expand Up @@ -1114,6 +1174,11 @@ export interface InitOptions {
* @default - false
*/
readonly generateOnly?: boolean;

/**
* Positional argument for init
*/
readonly TEMPLATE?: string;
}

/**
Expand Down
21 changes: 18 additions & 3 deletions packages/aws-cdk/lib/convert-to-cli-args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function convertToCliArgs(args: any): CliArguments {
commandOptions = {
long: args.long,
showDependencies: args.showDependencies,
STACKS: args.STACKS,
};
break;

Expand All @@ -49,6 +50,7 @@ export function convertToCliArgs(args: any): CliArguments {
exclusively: args.exclusively,
validation: args.validation,
quiet: args.quiet,
STACKS: args.STACKS,
};
break;

Expand All @@ -72,6 +74,7 @@ export function convertToCliArgs(args: any): CliArguments {
toolkitStackName: args.toolkitStackName,
template: args.template,
previousParameters: args.previousParameters,
ENVIRONMENTS: args.ENVIRONMENTS,
};
break;

Expand All @@ -83,6 +86,7 @@ export function convertToCliArgs(args: any): CliArguments {
createdBufferDays: args.createdBufferDays,
confirm: args.confirm,
bootstrapStackName: args.bootstrapStackName,
ENVIRONMENTS: args.ENVIRONMENTS,
};
break;

Expand Down Expand Up @@ -113,6 +117,7 @@ export function convertToCliArgs(args: any): CliArguments {
assetParallelism: args.assetParallelism,
assetPrebuild: args.assetPrebuild,
ignoreNoStacks: args.ignoreNoStacks,
STACKS: args.STACKS,
};
break;

Expand All @@ -123,6 +128,7 @@ export function convertToCliArgs(args: any): CliArguments {
force: args.force,
validateBootstrapVersion: args.validateBootstrapVersion,
orphan: args.orphan,
STACKS: args.STACKS,
};
break;

Expand All @@ -135,6 +141,7 @@ export function convertToCliArgs(args: any): CliArguments {
force: args.force,
recordResourceMapping: args.recordResourceMapping,
resourceMapping: args.resourceMapping,
STACK: args.STACK,
};
break;

Expand All @@ -151,6 +158,7 @@ export function convertToCliArgs(args: any): CliArguments {
hotswapFallback: args.hotswapFallback,
logs: args.logs,
concurrency: args.concurrency,
STACKS: args.STACKS,
};
break;

Expand All @@ -159,6 +167,7 @@ export function convertToCliArgs(args: any): CliArguments {
all: args.all,
exclusively: args.exclusively,
force: args.force,
STACKS: args.STACKS,
};
break;

Expand All @@ -173,15 +182,20 @@ export function convertToCliArgs(args: any): CliArguments {
processed: args.processed,
quiet: args.quiet,
changeSet: args.changeSet,
STACKS: args.STACKS,
};
break;

case 'metadata':
commandOptions = {};
commandOptions = {
STACK: args.STACK,
};
break;

case 'acknowledge':
commandOptions = {};
commandOptions = {
ID: args.ID,
};
break;

case 'notices':
Expand All @@ -195,6 +209,7 @@ export function convertToCliArgs(args: any): CliArguments {
language: args.language,
list: args.list,
generateOnly: args.generateOnly,
TEMPLATE: args.TEMPLATE,
};
break;

Expand Down Expand Up @@ -232,7 +247,7 @@ export function convertToCliArgs(args: any): CliArguments {
break;
}
const cliArguments: CliArguments = {
_: args._,
_: args._[0],
globalOptions,
[args._[0]]: commandOptions,
};
Expand Down
31 changes: 30 additions & 1 deletion packages/aws-cdk/test/cli-arguments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test('yargs object can be converted to cli arguments', async () => {
const result = convertToCliArgs(input);

expect(result).toEqual({
_: ['deploy'],
_: 'deploy',
globalOptions: {
app: undefined,
assetMetadata: undefined,
Expand Down Expand Up @@ -36,6 +36,7 @@ test('yargs object can be converted to cli arguments', async () => {
output: undefined,
},
deploy: {
STACKS: undefined,
all: false,
assetParallelism: undefined,
assetPrebuild: true,
Expand Down Expand Up @@ -64,3 +65,31 @@ test('yargs object can be converted to cli arguments', async () => {
},
});
});

test('positional argument is correctly passed through -- variadic', async () => {
const input = await parseCommandLineArguments(['deploy', 'stack1', 'stack2', '-R', '-v', '--ci']);

const result = convertToCliArgs(input);

expect(result).toEqual({
_: 'deploy',
deploy: expect.objectContaining({
STACKS: ['stack1', 'stack2'],
}),
globalOptions: expect.anything(),
});
});

test('positional argument is correctly passed through -- single', async () => {
const input = await parseCommandLineArguments(['acknowledge', 'id1', '-v', '--ci']);

const result = convertToCliArgs(input);

expect(result).toEqual({
_: 'acknowledge',
acknowledge: expect.objectContaining({
ID: 'id1',
}),
globalOptions: expect.anything(),
});
});
10 changes: 9 additions & 1 deletion tools/@aws-cdk/cli-args-gen/lib/cli-args-function-gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function buildCommandSwitch(config: CliConfig): string {
`case '${commandName}':`,
'commandOptions = {',
...buildCommandOptions(config.commands[commandName]),
...(config.commands[commandName].arg ? [buildPositionalArguments(config.commands[commandName].arg)] : []),
'};',
`break;
`);
Expand All @@ -84,10 +85,17 @@ function buildCommandOptions(options: CliAction): string[] {
return commandOptions;
}

function buildPositionalArguments(arg: { name: string; variadic: boolean }): string {
if (arg.variadic) {
return `${arg.name}: args.${arg.name}`;
}
return `${arg.name}: args.${arg.name}`;
}

function buildCliArgs(): string {
return [
'const cliArguments: CliArguments = {',
'_: args._,',
'_: args._[0],',
'globalOptions,',
'[args._[0]]: commandOptions',
'}',
Expand Down
17 changes: 15 additions & 2 deletions tools/@aws-cdk/cli-args-gen/lib/cli-args-gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export async function renderCliArgsType(config: CliConfig): Promise<string> {

cliArgType.addProperty({
name: '_',
type: Type.ambient(`[${commandEnum}, ...string[]]`),
type: commandEnum,
docs: {
summary: 'The CLI command name followed by any properties of the command',
summary: 'The CLI command name',
},
});

Expand Down Expand Up @@ -73,6 +73,7 @@ export async function renderCliArgsType(config: CliConfig): Promise<string> {
},
});

// add command level options
for (const [optionName, option] of Object.entries(command.options ?? {})) {
commandType.addProperty({
name: kebabToCamelCase(optionName),
Expand All @@ -88,6 +89,18 @@ export async function renderCliArgsType(config: CliConfig): Promise<string> {
});
}

// add positional argument associated with the command
if (command.arg) {
commandType.addProperty({
name: command.arg.name,
type: command.arg.variadic ? Type.arrayOf(Type.STRING) : Type.STRING,
docs: {
summary: `Positional argument for ${commandName}`,
},
optional: true,
});
}

cliArgType.addProperty({
name: kebabToCamelCase(commandName),
type: Type.fromName(scope, commandType.name),
Expand Down
Loading

0 comments on commit caf2415

Please sign in to comment.