refactor(cli): docs, doctor, context commands do not use configuration directly anymore (#32578)

This is a refactor that follows #32558. The effort is to ensure that the `configuration` object is not directly used by commands and instead, the relevant information is passed directly to the function.

- doctor: no options needed
- docs: 1 option needed
- context: in addition to a few options, the context command also saves *some* values to `cdk.context.json`. this requires some modifications to the `Context` object to support this. 

We move away from using the archaic API in `command-api.ts`. This seems to be an old effort to standardize CLI commands that was only used for doctor/docs/context. We have since evolved to use command-specific property bags.

Since this is a refactor PR. I aim to change no functionality of the CLI.

### 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: kaizencc

codecov.yml

@@ -34,3 +34,4 @@ component_management:
 # https://docs.codecov.com/docs/ignoring-paths
 ignore:
   - packages/aws-cdk/lib/parse-command-line-arguments.ts # this file is generated and some lines cannot be tested
+  - packages/aws-cdk/lib/cli.ts # we integ test this file

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

@@ -1224,7 +1224,7 @@ export interface ContextOptions {
   /**
    * Clear all context
    *
-   * @default - undefined
+   * @default - false
    */
   readonly clear?: boolean;
 }

packages/aws-cdk/lib/cli.ts

@@ -17,9 +17,9 @@ import { Deployments } from '../lib/api/deployments';
 import { PluginHost } from '../lib/api/plugin';
 import { ToolkitInfo } from '../lib/api/toolkit-info';
 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 { contextHandler as context } from '../lib/commands/context';
+import { docs } from '../lib/commands/docs';
+import { doctor } from '../lib/commands/doctor';
 import { getMigrateScanType } from '../lib/commands/migrate';
 import { cliInit, printAvailableTemplates } from '../lib/init';
 import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/logging';
@@ -37,7 +37,6 @@ if (!process.stdout.isTTY) {
 }
 
 export async function exec(args: string[], synthesizer?: Synthesizer): Promise<number | void> {
-
   const argv = await parseCommandLineArguments(args);
 
   // if one -v, log at a DEBUG level
@@ -154,9 +153,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
     throw new Error(`First argument should be a string. Got: ${cmd} (${typeof(cmd)})`);
   }
 
-  // Bundle up global objects so the commands have access to them
-  const commandOptions = { args: argv, configuration, aws: sdkProvider };
-
   try {
     return await main(cmd, argv);
   } finally {
@@ -174,7 +170,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
       await notices.refresh();
       notices.display();
     }
-
   }
 
   async function main(command: string, args: any): Promise<number | void> {
@@ -207,13 +202,19 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
 
     switch (command) {
       case 'context':
-        return context(commandOptions);
+        return context({
+          context: configuration.context,
+          clear: argv.clear,
+          json: argv.json,
+          force: argv.force,
+          reset: argv.reset,
+        });
 
       case 'docs':
-        return docs(commandOptions);
+        return docs({ browser: configuration.settings.get(['browser']) });
 
       case 'doctor':
-        return doctor(commandOptions);
+        return doctor();
 
       case 'ls':
       case 'list':

packages/aws-cdk/lib/command-api.ts

@@ -1,27 +1,64 @@
 import * as chalk from 'chalk';
 import { minimatch } from 'minimatch';
 import * as version from '../../lib/version';
-import { CommandOptions } from '../command-api';
 import { print, error, warning } from '../logging';
 import { Context, PROJECT_CONFIG, PROJECT_CONTEXT, USER_DEFAULTS } from '../settings';
 import { renderTable } from '../util';
 
-export async function realHandler(options: CommandOptions): Promise<number> {
-  const { configuration, args } = options;
-  if (args.clear) {
-    configuration.context.clear();
-    await configuration.saveContext();
+/**
+ * Options for the context command
+ */
+export interface ContextOptions {
+  /**
+   * The context object sourced from all context locations
+   */
+  context: Context;
+
+  /**
+   * The context key (or its index) to reset
+   *
+   * @default undefined
+   */
+  reset?: string;
+
+  /**
+   * Ignore missing key error
+   *
+   * @default false
+   */
+  force?: boolean;
+
+  /**
+   * Clear all context
+   *
+   * @default false
+   */
+  clear?: boolean;
+
+  /**
+   * Use JSON output instead of YAML when templates are printed to STDOUT
+   *
+   * @default false
+   */
+  json?: boolean;
+}
+
+export async function contextHandler(options: ContextOptions): Promise<number> {
+  if (options.clear) {
+    options.context.clear();
+    await options.context.save(PROJECT_CONTEXT);
     print('All context values cleared.');
-  } else if (args.reset) {
-    invalidateContext(configuration.context, args.reset as string, args.force as boolean);
-    await configuration.saveContext();
+  } else if (options.reset) {
+    invalidateContext(options.context, options.reset, options.force ?? false);
+    await options.context.save(PROJECT_CONTEXT);
   } else {
     // List -- support '--json' flag
-    if (args.json) {
-      const contextValues = configuration.context.all;
+    if (options.json) {
+      /* istanbul ignore next */
+      const contextValues = options.context.all;
       process.stdout.write(JSON.stringify(contextValues, undefined, 2));
     } else {
-      listContext(configuration.context);
+      listContext(options.context);
     }
   }
   await version.displayVersionMessage();
@@ -105,13 +142,15 @@ function invalidateContext(context: Context, key: string, force: boolean) {
     throw new Error(`No context value matching key: ${key}`);
   }
 }
+
 function printUnset(unset: string[]) {
   if (unset.length === 0) return;
   print('The following matched context values reset. They will be refreshed on next synthesis');
   unset.forEach((match) => {
     print('  %s', match);
   });
 }
+
 function printReadonly(readonly: string[]) {
   if (readonly.length === 0) return;
   warning('The following matched context values could not be reset through the CLI');
@@ -121,6 +160,7 @@ function printReadonly(readonly: string[]) {
   print('');
   print('This usually means they are configured in %s or %s', chalk.blue(PROJECT_CONFIG), chalk.blue(USER_DEFAULTS));
 }
+
 function keysByExpression(context: Context, expression: string) {
   return context.keys.filter(minimatch.filter(expression));
 }

packages/aws-cdk/lib/commands/context.ts

@@ -1,16 +1,25 @@
 import * as childProcess from 'child_process';
 import * as chalk from 'chalk';
 import { debug, print, warning } from '../../lib/logging';
-import { CommandOptions } from '../command-api';
 
 export const command = 'docs';
 export const describe = 'Opens the reference documentation in a browser';
 export const aliases = ['doc'];
 
-export async function realHandler(options: CommandOptions): Promise<number> {
+/**
+ * Options for the docs command
+ */
+export interface DocsOptions {
+  /**
+   * The command to use to open the browser
+   */
+  browser: string;
+}
+
+export async function docs(options: DocsOptions): Promise<number> {
   const url = 'https://docs.aws.amazon.com/cdk/api/v2/';
   print(chalk.green(url));
-  const browserCommand = (options.args.browser as string).replace(/%u/g, url);
+  const browserCommand = (options.browser).replace(/%u/g, url);
   debug(`Opening documentation ${chalk.green(browserCommand)}`);
   return new Promise<number>((resolve, _reject) => {
     childProcess.exec(browserCommand, (err, stdout, stderr) => {

packages/aws-cdk/lib/commands/docs.ts

@@ -3,9 +3,8 @@ import * as cxapi from '@aws-cdk/cx-api';
 import * as chalk from 'chalk';
 import { print } from '../../lib/logging';
 import * as version from '../../lib/version';
-import { CommandOptions } from '../command-api';
 
-export async function realHandler(_options: CommandOptions): Promise<number> {
+export async function doctor(): Promise<number> {
   let exitStatus: number = 0;
   for (const verification of verifications) {
     if (!await verification()) {

packages/aws-cdk/lib/commands/doctor.ts

@@ -373,9 +373,9 @@ export async function makeConfig(): Promise<CliConfig> {
       context: {
         description: 'Manage cached context values',
         options: {
-          reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true },
+          reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true, default: undefined },
           force: { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false },
-          clear: { desc: 'Clear all context', type: 'boolean' },
+          clear: { desc: 'Clear all context', type: 'boolean', default: false },
         },
       },
       docs: {

packages/aws-cdk/lib/config.ts

@@ -742,6 +742,7 @@ export function parseCommandLineArguments(args: Array<string>): any {
           desc: 'The context key (or its index) to reset',
           type: 'string',
           requiresArg: true,
+          default: undefined,
         })
         .option('force', {
           alias: 'f',
@@ -752,6 +753,7 @@ export function parseCommandLineArguments(args: Array<string>): any {
         .option('clear', {
           desc: 'Clear all context',
           type: 'boolean',
+          default: false,
         })
     )
     .command(['docs', 'doc'], 'Opens the reference documentation in a browser', (yargs: Argv) =>

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

@@ -99,7 +99,7 @@ export class Configuration {
     return this._projectConfig;
   }
 
-  private get projectContext() {
+  public get projectContext() {
     if (!this._projectContext) {
       throw new Error('#load has not been called yet!');
     }
@@ -121,12 +121,12 @@ export class Configuration {
     }
 
     const contextSources = [
-      this.commandLineContext,
-      this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(),
-      this.projectContext,
+      { bag: this.commandLineContext },
+      { fileName: PROJECT_CONFIG, bag: this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly() },
+      { fileName: PROJECT_CONTEXT, bag: this.projectContext },
     ];
     if (readUserContext) {
-      contextSources.push(userConfig.subSettings([CONTEXT_KEY]).makeReadOnly());
+      contextSources.push({ fileName: USER_DEFAULTS, bag: userConfig.subSettings([CONTEXT_KEY]).makeReadOnly() });
     }
 
     this.context = new Context(...contextSources);
@@ -166,6 +166,19 @@ async function loadAndLog(fileName: string): Promise<Settings> {
   return ret;
 }
 
+interface ContextBag {
+  /**
+   * The file name of the context. Will be used to potentially
+   * save new context back to the original file.
+   */
+  fileName?: string;
+
+  /**
+   * The context values.
+   */
+  bag: Settings;
+}
+
 /**
  * Class that supports overlaying property bags
  *
@@ -176,9 +189,11 @@ async function loadAndLog(fileName: string): Promise<Settings> {
  */
 export class Context {
   private readonly bags: Settings[];
+  private readonly fileNames: (string|undefined)[];
 
-  constructor(...bags: Settings[]) {
-    this.bags = bags.length > 0 ? bags : [new Settings()];
+  constructor(...bags: ContextBag[]) {
+    this.bags = bags.length > 0 ? bags.map(b => b.bag) : [new Settings()];
+    this.fileNames = bags.length > 0 ? bags.map(b => b.fileName) : ['default'];
   }
 
   public get keys(): string[] {
@@ -227,6 +242,26 @@ export class Context {
       this.unset(key);
     }
   }
+
+  /**
+   * Save a specific context file
+   */
+  public async save(fileName: string): Promise<this> {
+    const index = this.fileNames.indexOf(fileName);
+
+    // File not found, don't do anything in this scenario
+    if (index === -1) {
+      return this;
+    }
+
+    const bag = this.bags[index];
+    if (bag.readOnly) {
+      throw new Error(`Context file ${fileName} is read only!`);
+    }
+
+    await bag.save(fileName);
+    return this;
+  }
 }
 
 /**

packages/aws-cdk/lib/settings.ts

@@ -1,31 +1,25 @@
 import * as child_process from 'child_process';
 import { mocked } from 'jest-mock';
-import { CommandHandler } from '../lib/command-api';
-import { realHandler } from '../lib/commands/docs';
-const argv = {
-  browser: 'echo %u',
-  commandHandler: undefined as (CommandHandler | undefined),
-};
+import { docs } from '../lib/commands/docs';
 
 // eslint-disable-next-line no-console
 console.log = jest.fn();
 jest.mock('child_process');
 
 describe('`cdk docs`', () => {
-
   test('exits with 0 when everything is OK', async () => {
     const mockChildProcessExec: any = (_: string, cb: (err?: Error, stdout?: string, stderr?: string) => void) => cb();
     mocked(child_process.exec).mockImplementation(mockChildProcessExec);
 
-    const result = await realHandler({ args: argv } as any);
+    const result = await docs({ browser: 'echo %u' });
     expect(result).toBe(0);
   });
 
   test('exits with 0 when opening the browser fails', async () => {
     const mockChildProcessExec: any = (_: string, cb: (err: Error, stdout?: string, stderr?: string) => void) => cb(new Error('TEST'));
     mocked(child_process.exec).mockImplementation(mockChildProcessExec);
 
-    const result = await realHandler({ args: argv } as any);
+    const result = await docs({ browser: 'echo %u' });
     expect(result).toBe(0);
   });
 });