From 506d2101a5ae7e501539eb39fa911c0d63db6fd5 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Tue, 17 Dec 2024 12:10:01 +0000 Subject: [PATCH] chore(cli): simplify config with helper functions (#32547) ### Reason for this change Simplify the codegen for CLI config. Previously the generated function expected a number of arguments at call time. Instead we now require a single helpers object at build time. This holds typewriter references to any function called during config parsing. Additionally, some the helper functions for init and migrate are now just called at build time. This is fine because these values don't change dynamically unless a code change is made to the CLI. ### Description of changes - Moved all helper functions into a single file - Created a typewriter proxy class for that file - Use the module proxy instead passing arguments at runtime - This allows us to get rid of the `DynamicValue` stuff all-together and instead we can just check if a value is a typewriter expression. ### Description of how you validated changes Existing tests cover this. Run additional manual verification as well. ### 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.ts | 16 +--- packages/aws-cdk/lib/config.ts | 88 ++++++------------- .../lib/parse-command-line-arguments.ts | 24 ++--- packages/aws-cdk/lib/util/yargs-helpers.ts | 36 +++++++- packages/aws-cdk/scripts/yargs-gen.ts | 4 +- .../test/parse-command-line-arguments.test.ts | 38 +++++++- tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts | 48 +++++----- tools/@aws-cdk/yargs-gen/test/cli.test.ts | 58 +++++++----- 8 files changed, 173 insertions(+), 139 deletions(-) diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index b2fa6e4040e80..f40a71945485b 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -20,14 +20,13 @@ import { CdkToolkit, AssetBuildTime } from '../lib/cdk-toolkit'; import { realHandler as context } from '../lib/commands/context'; import { realHandler as docs } from '../lib/commands/docs'; import { realHandler as doctor } from '../lib/commands/doctor'; -import { MIGRATE_SUPPORTED_LANGUAGES, getMigrateScanType } from '../lib/commands/migrate'; -import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; +import { getMigrateScanType } from '../lib/commands/migrate'; +import { cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/logging'; 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 @@ -38,17 +37,8 @@ if (!process.stdout.isTTY) { } export async function exec(args: string[], synthesizer?: Synthesizer): Promise { - function makeBrowserDefault(): string { - const defaultBrowserCommand: { [key in NodeJS.Platform]?: string } = { - darwin: 'open %u', - win32: 'start %u', - }; - - const cmd = defaultBrowserCommand[process.platform]; - return cmd ?? 'xdg-open %u'; - } - const argv = await parseCommandLineArguments(args, makeBrowserDefault(), await availableInitLanguages(), MIGRATE_SUPPORTED_LANGUAGES as string[], version.DISPLAY_VERSION, yargsNegativeAlias); + const argv = await parseCommandLineArguments(args); // if one -v, log at a DEBUG level // if 2 -v, log at a TRACE level diff --git a/packages/aws-cdk/lib/config.ts b/packages/aws-cdk/lib/config.ts index 62714e334f66f..f7aaf8d6bc013 100644 --- a/packages/aws-cdk/lib/config.ts +++ b/packages/aws-cdk/lib/config.ts @@ -1,14 +1,17 @@ -import type { CliConfig, DynamicResult } from '@aws-cdk/yargs-gen'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { CliHelpers, type CliConfig } from '@aws-cdk/yargs-gen'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; +import { MIGRATE_SUPPORTED_LANGUAGES } from './commands/migrate'; import { RequireApproval } from './diff'; +import { availableInitLanguages } from './init'; -/* eslint-disable quote-props */ +export const YARGS_HELPERS = new CliHelpers('./util/yargs-helpers'); /** * Source of truth for all CDK CLI commands. `yargs-gen` translates this into the `yargs` definition * in `lib/parse-command-line-arguments.ts`. */ -export function makeConfig(): CliConfig { +export async function makeConfig(): Promise { return { 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 }, @@ -34,11 +37,11 @@ export function makeConfig(): CliConfig { 'output': { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true }, 'notices': { type: 'boolean', desc: 'Show relevant notices' }, 'no-color': { type: 'boolean', desc: 'Removes colors and other style from console output', default: false }, - 'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: DynamicValue.fromInline(() => process.env.CI !== undefined) }, + 'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: YARGS_HELPERS.isCI() }, 'unstable': { 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: [] }, }, commands: { - 'list': { + list: { arg: { name: 'STACKS', variadic: true, @@ -50,7 +53,7 @@ export function makeConfig(): CliConfig { 'show-dependencies': { type: 'boolean', default: false, alias: 'd', desc: 'Display stack dependency information for each stack' }, }, }, - 'synthesize': { + synthesize: { arg: { name: 'STACKS', variadic: true, @@ -58,9 +61,9 @@ export function makeConfig(): CliConfig { aliases: ['synth'], description: 'Synthesizes and prints the CloudFormation template for this stack', options: { - 'exclusively': { type: 'boolean', alias: 'e', desc: 'Only synthesize requested stacks, don\'t include dependencies' }, - 'validation': { type: 'boolean', desc: 'After synthesis, validate stacks with the "validateOnSynth" attribute set (can also be controlled with CDK_VALIDATION)', default: true }, - 'quiet': { type: 'boolean', alias: 'q', desc: 'Do not output CloudFormation Template to stdout', default: false }, + exclusively: { type: 'boolean', alias: 'e', desc: 'Only synthesize requested stacks, don\'t include dependencies' }, + validation: { type: 'boolean', desc: 'After synthesis, validate stacks with the "validateOnSynth" attribute set (can also be controlled with CDK_VALIDATION)', default: true }, + quiet: { type: 'boolean', alias: 'q', desc: 'Do not output CloudFormation Template to stdout', default: false }, }, }, bootstrap: { @@ -242,18 +245,6 @@ export function makeConfig(): CliConfig { variadic: true, }, options: { - // I'm fairly certain none of these options, present for 'deploy', make sense for 'watch': - // .option('all', { type: 'boolean', default: false, desc: 'Deploy all available stacks' }) - // .option('ci', { type: 'boolean', desc: 'Force CI detection', default: process.env.CI !== undefined }) - // @deprecated(v2) -- tags are part of the Cloud Assembly and tags specified here will be overwritten on the next deployment - // .option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)', nargs: 1, requiresArg: true }) - // .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true }) - // These options, however, are more subtle - I could be convinced some of these should also be available for 'watch': - // .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' }) - // .option('parameters', { type: 'array', desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)', nargs: 1, requiresArg: true, default: {} }) - // .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', 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' }, @@ -295,9 +286,9 @@ export function makeConfig(): CliConfig { variadic: true, }, options: { - 'all': { type: 'boolean', default: false, desc: 'Destroy all available stacks' }, - 'exclusively': { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' }, - 'force': { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' }, + all: { type: 'boolean', default: false, desc: 'Destroy all available stacks' }, + exclusively: { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' }, + force: { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' }, }, }, diff: { @@ -336,7 +327,7 @@ export function makeConfig(): CliConfig { notices: { description: 'Returns a list of relevant notices', options: { - 'unacknowledged': { type: 'boolean', alias: 'u', default: false, desc: 'Returns a list of unacknowledged notices' }, + unacknowledged: { type: 'boolean', alias: 'u', default: false, desc: 'Returns a list of unacknowledged notices' }, }, }, init: { @@ -346,16 +337,16 @@ export function makeConfig(): CliConfig { variadic: false, }, options: { - 'language': { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: DynamicValue.fromParameter('availableInitLanguages') } as any, // TODO: preamble, this initTemplateLanguages variable needs to go as a statement there. + 'language': { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: await availableInitLanguages() }, 'list': { type: 'boolean', desc: 'List the available templates' }, 'generate-only': { type: 'boolean', default: false, desc: 'If true, only generates project files, without executing additional operations such as setting up a git repo, installing dependencies or compiling the project' }, }, }, - 'migrate': { + migrate: { description: false as any, options: { 'stack-name': { type: 'string', alias: 'n', desc: 'The name assigned to the stack created in the new project. The name of the app will be based off this name as well.', requiresArg: true }, - 'language': { type: 'string', default: 'typescript', alias: 'l', desc: 'The language to be used for the new project', choices: DynamicValue.fromParameter('migrateSupportedLanguages') as any }, + 'language': { type: 'string', default: 'typescript', alias: 'l', desc: 'The language to be used for the new project', choices: MIGRATE_SUPPORTED_LANGUAGES }, 'account': { type: 'string', desc: 'The account to retrieve the CloudFormation stack template from' }, 'region': { type: 'string', desc: 'The region to retrieve the CloudFormation stack template from' }, 'from-path': { type: 'string', desc: 'The path to the CloudFormation template to migrate. Use this for locally stored templates' }, @@ -379,54 +370,29 @@ export function makeConfig(): CliConfig { 'compress': { type: 'boolean', desc: 'Use this flag to zip the generated CDK app' }, }, }, - 'context': { + context: { description: 'Manage cached context values', options: { - 'reset': { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true }, - 'force': { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false }, - 'clear': { desc: 'Clear all context', type: 'boolean' }, + reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true }, + force: { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false }, + clear: { desc: 'Clear all context', type: 'boolean' }, }, }, - 'docs': { + docs: { aliases: ['doc'], description: 'Opens the reference documentation in a browser', options: { - 'browser': { + browser: { alias: 'b', desc: 'the command to use to open the browser, using %u as a placeholder for the path of the file to open', type: 'string', - default: DynamicValue.fromParameter('browserDefault'), + default: YARGS_HELPERS.browserForPlatform(), }, }, }, - 'doctor': { + doctor: { description: 'Check your set-up for potential problems', }, }, }; } - -/** - * Informs the code library, `@aws-cdk/yargs-gen`, that - * this value references an entity not defined in this configuration file. - */ -export class DynamicValue { - /** - * Instructs `yargs-gen` to retrieve this value from the parameter with passed name. - */ - public static fromParameter(parameterName: string): DynamicResult { - return { - dynamicType: 'parameter', - dynamicValue: parameterName, - }; - } - - public static fromInline(f: () => any): DynamicResult { - const ARROW = '=>'; - const body = f.toString(); - return { - dynamicType: 'function', - dynamicValue: body.substring(body.indexOf(ARROW) + ARROW.length).trim(), - }; - } -} diff --git a/packages/aws-cdk/lib/parse-command-line-arguments.ts b/packages/aws-cdk/lib/parse-command-line-arguments.ts index 4f85e19394827..c7e4050d9f1e6 100644 --- a/packages/aws-cdk/lib/parse-command-line-arguments.ts +++ b/packages/aws-cdk/lib/parse-command-line-arguments.ts @@ -4,16 +4,10 @@ // ------------------------------------------------------------------------------------------- /* eslint-disable @typescript-eslint/comma-dangle, comma-spacing, max-len, quotes, quote-props */ import { Argv } from 'yargs'; +import * as helpers from './util/yargs-helpers'; // @ts-ignore TS6133 -export function parseCommandLineArguments( - args: Array, - browserDefault: string, - availableInitLanguages: Array, - migrateSupportedLanguages: Array, - version: string, - yargsNegativeAlias: any -): any { +export function parseCommandLineArguments(args: Array): any { return yargs .env('CDK') .usage('Usage: cdk -a COMMAND') @@ -143,7 +137,7 @@ export function parseCommandLineArguments( .option('ci', { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', - default: process.env.CI !== undefined, + default: helpers.isCI(), }) .option('unstable', { type: 'array', @@ -424,8 +418,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('R', 'rollback'), true) .option('R', { type: 'boolean', hidden: true }) + .middleware(helpers.yargsNegativeAlias('R', 'rollback'), 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", @@ -570,8 +564,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('R', 'rollback'), true) .option('R', { type: 'boolean', hidden: true }) + .middleware(helpers.yargsNegativeAlias('R', 'rollback'), 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", @@ -679,7 +673,7 @@ export function parseCommandLineArguments( type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', - choices: availableInitLanguages, + choices: ['csharp', 'fsharp', 'go', 'java', 'javascript', 'python', 'typescript'], }) .option('list', { type: 'boolean', @@ -704,7 +698,7 @@ export function parseCommandLineArguments( default: 'typescript', alias: 'l', desc: 'The language to be used for the new project', - choices: migrateSupportedLanguages, + choices: ['typescript', 'go', 'java', 'python', 'csharp'], }) .option('account', { type: 'string', @@ -765,11 +759,11 @@ export function parseCommandLineArguments( alias: 'b', desc: 'the command to use to open the browser, using %u as a placeholder for the path of the file to open', type: 'string', - default: browserDefault, + default: helpers.browserForPlatform(), }) ) .command('doctor', 'Check your set-up for potential problems') - .version(version) + .version(helpers.cliVersion()) .demandCommand(1, '') .recommendCommands() .help() diff --git a/packages/aws-cdk/lib/util/yargs-helpers.ts b/packages/aws-cdk/lib/util/yargs-helpers.ts index 719b531b0d24c..92c982ab7bffd 100644 --- a/packages/aws-cdk/lib/util/yargs-helpers.ts +++ b/packages/aws-cdk/lib/util/yargs-helpers.ts @@ -1,10 +1,12 @@ +import * as version from '../../lib/version'; + /** * 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 + * @returns a middleware function that can be passed to yargs */ export function yargsNegativeAlias( negativeAlias: S, @@ -19,3 +21,35 @@ export function yargsNegativeAlias { diff --git a/packages/aws-cdk/test/parse-command-line-arguments.test.ts b/packages/aws-cdk/test/parse-command-line-arguments.test.ts index 0571fb608b7c9..485536a7c28df 100644 --- a/packages/aws-cdk/test/parse-command-line-arguments.test.ts +++ b/packages/aws-cdk/test/parse-command-line-arguments.test.ts @@ -1,8 +1,40 @@ 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); - + const argv = await parseCommandLineArguments(['deploy', '-R']); expect(argv.rollback).toBe(false); }); + +describe('cdk docs', () => { + const originalPlatform = process.platform; + // Helper to mock process.platform + const mockPlatform = (platform: string) => { + Object.defineProperty(process, 'platform', { + value: platform, + writable: false, + enumerable: true, + configurable: true, + }); + }; + + // Restore original platform after each test + afterEach(() => { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + writable: false, + enumerable: true, + configurable: true, + }); + }); + + test.each([ + ['darwin', 'open %u'], + ['win32', 'start %u'], + ['linux', 'xdg-open %u'], + ['freebsd', 'xdg-open %u'], + ])('for %s should return "%s"', async (platform, browser) => { + mockPlatform(platform); + const argv = await parseCommandLineArguments(['docs']); + expect(argv.browser).toBe(browser); + }); +}); diff --git a/tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts b/tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts index 0bfc5af85412e..341315ac4a8b9 100644 --- a/tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts +++ b/tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts @@ -1,4 +1,4 @@ -import { Expression, FreeFunction, Module, SelectiveModuleImport, Statement, Type, TypeScriptRenderer, code } from '@cdklabs/typewriter'; +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 { CliConfig, CliOption, YargsOption } from './yargs-types'; @@ -8,7 +8,18 @@ import { CliConfig, CliOption, YargsOption } from './yargs-types'; // eslint-disable-next-line @typescript-eslint/no-require-imports const cloneDeep = require('lodash.clonedeep'); -export async function renderYargs(config: CliConfig): Promise { +export class CliHelpers extends ExternalModule { + public readonly browserForPlatform = makeCallableExpr(this, 'browserForPlatform'); + public readonly cliVersion = makeCallableExpr(this, 'cliVersion'); + public readonly isCI = makeCallableExpr(this, 'isCI'); + public readonly yargsNegativeAlias = makeCallableExpr(this, 'yargsNegativeAlias'); +} + +function makeCallableExpr(scope: IScope, name: string) { + return $E(expr.sym(new ThingSymbol(name, scope))); +} + +export async function renderYargs(config: CliConfig, helpers: CliHelpers): Promise { const scope = new Module('aws-cdk'); scope.documentation.push( '-------------------------------------------------------------------------------------------'); @@ -17,6 +28,7 @@ export async function renderYargs(config: CliConfig): Promise { scope.documentation.push('-------------------------------------------------------------------------------------------'); scope.addImport(new SelectiveModuleImport(scope, 'yargs', ['Argv'])); + helpers.import(scope, 'helpers'); // 'https://github.com/yargs/yargs/issues/1929', // 'https://github.com/evanw/esbuild/issues/1492', @@ -29,14 +41,9 @@ export async function renderYargs(config: CliConfig): Promise { returnType: Type.ANY, parameters: [ { name: 'args', type: Type.arrayOf(Type.STRING) }, - { name: 'browserDefault', type: Type.STRING }, - { name: 'availableInitLanguages', type: Type.arrayOf(Type.STRING) }, - { name: 'migrateSupportedLanguages', type: Type.arrayOf(Type.STRING) }, - { name: 'version', type: Type.STRING }, - { name: 'yargsNegativeAlias', type: Type.ANY }, ], }); - parseCommandLineArguments.addBody(makeYargs(config)); + parseCommandLineArguments.addBody(makeYargs(config, helpers)); const ts = new TypeScriptRenderer({ disabledEsLintRules: [ @@ -67,14 +74,14 @@ export async function renderYargs(config: CliConfig): Promise { // By using the config above, every --arg will only consume one argument, so you can do the following: // // ./prog --arg one --arg two position => will parse to { arg: ['one', 'two'], _: ['positional'] }. -function makeYargs(config: CliConfig): Statement { +function makeYargs(config: CliConfig, helpers: CliHelpers): Statement { let yargsExpr: Expression = code.expr.ident('yargs'); yargsExpr = yargsExpr .callMethod('env', lit('CDK')) .callMethod('usage', lit('Usage: cdk -a COMMAND')); // we must compute global options first, as they are not part of an argument to a command call - yargsExpr = makeOptions(yargsExpr, config.globalOptions); + yargsExpr = makeOptions(yargsExpr, config.globalOptions, helpers); for (const command of Object.keys(config.commands)) { const commandFacts = config.commands[command]; @@ -87,7 +94,7 @@ function makeYargs(config: CliConfig): Statement { // must compute options before we compute the full command, because in yargs, the options are an argument to the command call. let optionsExpr: Expression = code.expr.directCode('(yargs: Argv) => yargs'); - optionsExpr = makeOptions(optionsExpr, commandFacts.options ?? {}); + optionsExpr = makeOptions(optionsExpr, commandFacts.options ?? {}, helpers); const commandCallArgs: Array = []; if (aliases) { @@ -104,10 +111,10 @@ function makeYargs(config: CliConfig): Statement { yargsExpr = yargsExpr.callMethod('command', ...commandCallArgs); } - return code.stmt.ret(makeEpilogue(yargsExpr)); + return code.stmt.ret(makeEpilogue(yargsExpr, helpers)); } -function makeOptions(prefix: Expression, options: { [optionName: string]: CliOption }) { +function makeOptions(prefix: Expression, options: { [optionName: string]: CliOption }, helpers: CliHelpers) { let optionsExpr = prefix; for (const option of Object.keys(options)) { const theOption: CliOption = options[option]; @@ -122,11 +129,8 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: CliOpt for (const optionProp of Object.keys(optionProps).filter(opt => !['negativeAlias'].includes(opt))) { const optionValue = (optionProps as any)[optionProp]; - if (optionValue && optionValue.dynamicType === 'parameter') { - optionArgs[optionProp] = code.expr.ident(optionValue.dynamicValue); - } else if (optionValue && optionValue.dynamicType === 'function') { - const inlineFunction: string = optionValue.dynamicValue; - optionArgs[optionProp] = code.expr.directCode(inlineFunction); + if (optionValue instanceof Expression) { + optionArgs[optionProp] = optionValue; } else { optionArgs[optionProp] = lit(optionValue); } @@ -139,20 +143,20 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: CliOpt // We need an additional option and a middleware: // .option('R', { type: 'boolean', hidden: true }).middleware(yargsNegativeAlias('R', 'rollback'), true) if (theOption.negativeAlias) { - const middleware = code.expr.builtInFn('yargsNegativeAlias', lit(theOption.negativeAlias), lit(option)); - optionsExpr = optionsExpr.callMethod('middleware', middleware, lit(true)); + const middleware = helpers.yargsNegativeAlias.call(lit(theOption.negativeAlias), lit(option)); optionsExpr = optionsExpr.callMethod('option', lit(theOption.negativeAlias), code.expr.lit({ type: 'boolean', hidden: true, })); + optionsExpr = optionsExpr.callMethod('middleware', middleware, lit(true)); } } return optionsExpr; } -function makeEpilogue(prefix: Expression) { - let completeDefinition = prefix.callMethod('version', code.expr.ident('version')); +function makeEpilogue(prefix: Expression, helpers: CliHelpers) { + let completeDefinition = prefix.callMethod('version', helpers.cliVersion()); completeDefinition = completeDefinition.callMethod('demandCommand', lit(1), lit('')); // just print help completeDefinition = completeDefinition.callMethod('recommendCommands'); completeDefinition = completeDefinition.callMethod('help'); diff --git a/tools/@aws-cdk/yargs-gen/test/cli.test.ts b/tools/@aws-cdk/yargs-gen/test/cli.test.ts index 7c903e34fdfbf..52988720cf870 100644 --- a/tools/@aws-cdk/yargs-gen/test/cli.test.ts +++ b/tools/@aws-cdk/yargs-gen/test/cli.test.ts @@ -1,4 +1,7 @@ -import { CliConfig, renderYargs } from '../lib'; +import { $E, expr, ThingSymbol } from '@cdklabs/typewriter'; +import { CliConfig, CliHelpers, renderYargs } from '../lib'; + +const YARGS_HELPERS = new CliHelpers('./util/yargs-helpers'); describe('render', () => { test('can generate global options', async () => { @@ -20,23 +23,17 @@ describe('render', () => { commands: {}, }; - expect(await renderYargs(config)).toMatchInlineSnapshot(` + expect(await renderYargs(config, YARGS_HELPERS)).toMatchInlineSnapshot(` "// ------------------------------------------------------------------------------------------- // GENERATED FROM packages/aws-cdk/lib/config.ts. // Do not edit by hand; all changes will be overwritten at build time from the config file. // ------------------------------------------------------------------------------------------- /* eslint-disable @typescript-eslint/comma-dangle, comma-spacing, max-len, quotes, quote-props */ import { Argv } from 'yargs'; + import * as helpers from './util/yargs-helpers'; // @ts-ignore TS6133 - export function parseCommandLineArguments( - args: Array, - browserDefault: string, - availableInitLanguages: Array, - migrateSupportedLanguages: Array, - version: string, - yargsNegativeAlias: any - ): any { + export function parseCommandLineArguments(args: Array): any { return yargs .env('CDK') .usage('Usage: cdk -a COMMAND') @@ -57,7 +54,7 @@ describe('render', () => { nargs: 1, requiresArg: true, }) - .version(version) + .version(helpers.cliVersion()) .demandCommand(1, '') .recommendCommands() .help() @@ -90,23 +87,17 @@ describe('render', () => { }, }; - expect(await renderYargs(config)).toMatchInlineSnapshot(` + expect(await renderYargs(config, YARGS_HELPERS)).toMatchInlineSnapshot(` "// ------------------------------------------------------------------------------------------- // GENERATED FROM packages/aws-cdk/lib/config.ts. // Do not edit by hand; all changes will be overwritten at build time from the config file. // ------------------------------------------------------------------------------------------- /* eslint-disable @typescript-eslint/comma-dangle, comma-spacing, max-len, quotes, quote-props */ import { Argv } from 'yargs'; + import * as helpers from './util/yargs-helpers'; // @ts-ignore TS6133 - export function parseCommandLineArguments( - args: Array, - browserDefault: string, - availableInitLanguages: Array, - migrateSupportedLanguages: Array, - version: string, - yargsNegativeAlias: any - ): any { + export function parseCommandLineArguments(args: Array): any { return yargs .env('CDK') .usage('Usage: cdk -a COMMAND') @@ -117,10 +108,10 @@ describe('render', () => { alias: 'o', desc: 'text for one', }) - .middleware(yargsNegativeAlias('O', 'one'), true) .option('O', { type: 'boolean', hidden: true }) + .middleware(helpers.yargsNegativeAlias('O', 'one'), true) ) - .version(version) + .version(helpers.cliVersion()) .demandCommand(1, '') .recommendCommands() .help() @@ -134,4 +125,27 @@ describe('render', () => { " `); }); + + test('can pass-through expression unchanged', async () => { + const config: CliConfig = { + globalOptions: {}, + commands: { + test: { + description: 'the action under test', + options: { + one: { + type: 'boolean', + default: $E( + expr + .sym(new ThingSymbol('banana', YARGS_HELPERS)) + .call(expr.lit(1), expr.lit(2), expr.lit(3)), + ), + }, + }, + }, + }, + }; + + expect(await renderYargs(config, YARGS_HELPERS)).toContain('default: helpers.banana(1, 2, 3)'); + }); });