refactor(toolkit-lib,cli): access default messages through IoHelper (#513)

This change is in preparation of work to have all default message go
through `IoHelper` instead of using `IO.DEFAULT_XYZ`. It contains a
number of clean-up and alignment changes:

- `IoDefaultMessages` is now a local-only class, that is only accessed
through the new `IoHelper.defaults` getter.
- Removed unused usage of `IoHelper` and `IoDefaultMessages` in
`DiffFormatter`.
- Changed every use of `IoDefaultMessages` to `IoHelper.defaults`
- Made `IoDefaultMessages` methods async to be consistent with the rest,
we can `void` at the place of use if we really need to
- Also fixed most of the remaining `void`'ed call-sites for the above.
Only the legacy logging is now left.
- Removed unused `messages.ts` file
- Removed unused `trace` from `logging.ts` (and respective test) to not
encourage further use

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

Author: mrgrain

packages/@aws-cdk/cloudformation-diff/lib/format.ts

@@ -61,7 +61,8 @@ export function formatSecurityChanges(
   stream: NodeJS.WritableStream,
   templateDiff: TemplateDiff,
   logicalToPathMap: { [logicalId: string]: string } = {},
-  context?: number) {
+  context?: number,
+) {
   const formatter = new Formatter(stream, logicalToPathMap, templateDiff, context);
 
   formatSecurityChangesWithBanner(formatter, templateDiff);

packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts

@@ -11,8 +11,6 @@ import type * as cxapi from '@aws-cdk/cx-api';
 import * as chalk from 'chalk';
 import { PermissionChangeType } from '../../payloads';
 import type { NestedStackTemplates } from '../cloudformation';
-import type { IoHelper } from '../io/private';
-import { IoDefaultMessages } from '../io/private';
 import { StringWriteStream } from '../streams';
 
 /**
@@ -50,11 +48,6 @@ interface FormatStackDiffOutput {
  * Props for the Diff Formatter
  */
 interface DiffFormatterProps {
-  /**
-   * Helper for the IoHost class
-   */
-  readonly ioHelper: IoHelper;
-
   /**
    * The relevant information for the Template that is being diffed.
    * Includes the old/current state of the stack as well as the new state.
@@ -89,7 +82,6 @@ interface FormatStackDiffOptions {
 }
 
 interface ReusableStackDiffOptions extends FormatStackDiffOptions {
-  readonly ioDefaultHelper: IoDefaultMessages;
 }
 
 /**
@@ -136,7 +128,6 @@ export interface TemplateInfo {
  * Class for formatting the diff output
  */
 export class DiffFormatter {
-  private readonly ioHelper: IoHelper;
   private readonly oldTemplate: any;
   private readonly newTemplate: cxapi.CloudFormationStackArtifact;
   private readonly stackName: string;
@@ -151,7 +142,6 @@ export class DiffFormatter {
   private _diffs: { [name: string]: TemplateDiff } = {};
 
   constructor(props: DiffFormatterProps) {
-    this.ioHelper = props.ioHelper;
     this.oldTemplate = props.templateInfo.oldTemplate;
     this.newTemplate = props.templateInfo.newTemplate;
     this.stackName = props.templateInfo.newTemplate.displayName ?? props.templateInfo.newTemplate.stackName;
@@ -204,15 +194,11 @@ export class DiffFormatter {
    * Format the stack diff
    */
   public formatStackDiff(options: FormatStackDiffOptions = {}): FormatStackDiffOutput {
-    const ioDefaultHelper = new IoDefaultMessages(this.ioHelper);
     return this.formatStackDiffHelper(
       this.oldTemplate,
       this.stackName,
       this.nestedStacks,
-      {
-        ...options,
-        ioDefaultHelper,
-      },
+      options,
     );
   }
 

packages/@aws-cdk/toolkit-lib/lib/api/io/private/index.ts

@@ -5,4 +5,3 @@ export * from './span';
 export * from './message-maker';
 export * from './messages';
 export * from './types';
-export * from './io-default-messages';

packages/@aws-cdk/toolkit-lib/lib/api/io/private/io-default-messages.ts

@@ -10,45 +10,48 @@ import { IO } from './messages';
  * for the various log levels.
  */
 export class IoDefaultMessages {
-  constructor(private readonly ioHelper: IoHelper) {
+  private readonly ioHelper: IoHelper;
+
+  constructor(ioHelper: IoHelper) {
+    this.ioHelper = ioHelper;
   }
 
-  public notify(msg: ActionLessMessage<unknown>): Promise<void> {
+  public async notify(msg: ActionLessMessage<unknown>): Promise<void> {
     return this.ioHelper.notify(msg);
   }
 
-  public requestResponse<T, U>(msg: ActionLessRequest<T, U>): Promise<U> {
+  public async requestResponse<T, U>(msg: ActionLessRequest<T, U>): Promise<U> {
     return this.ioHelper.requestResponse(msg);
   }
 
-  public error(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_ERROR, input, ...args);
+  public async error(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_ERROR, input, ...args);
   }
 
-  public warn(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_WARN, input, ...args);
+  public async warn(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_WARN, input, ...args);
   }
 
-  public warning(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_WARN, input, ...args);
+  public async warning(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_WARN, input, ...args);
   }
 
-  public info(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_INFO, input, ...args);
+  public async info(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_INFO, input, ...args);
   }
 
-  public debug(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_DEBUG, input, ...args);
+  public async debug(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_DEBUG, input, ...args);
   }
 
-  public trace(input: string, ...args: unknown[]) {
-    this.emitMessage(IO.DEFAULT_TOOLKIT_TRACE, input, ...args);
+  public async trace(input: string, ...args: unknown[]): Promise<void> {
+    return this.emitMessage(IO.DEFAULT_TOOLKIT_TRACE, input, ...args);
   }
 
-  public result(input: string, ...args: unknown[]) {
+  public async result(input: string, ...args: unknown[]): Promise<void> {
     const message = args.length > 0 ? util.format(input, ...args) : input;
     // This is just the default "info" message but with a level of "result"
-    void this.ioHelper.notify({
+    return this.ioHelper.notify({
       time: new Date(),
       code: IO.DEFAULT_TOOLKIT_INFO.code,
       level: 'result',
@@ -57,9 +60,9 @@ export class IoDefaultMessages {
     });
   }
 
-  private emitMessage(maker: IoMessageMaker<void>, input: string, ...args: unknown[]) {
+  private async emitMessage(maker: IoMessageMaker<void>, input: string, ...args: unknown[]): Promise<void> {
     // Format message if args are provided
     const message = args.length > 0 ? util.format(input, ...args) : input;
-    void this.ioHelper.notify(maker.msg(message));
+    return this.ioHelper.notify(maker.msg(message));
   }
 }

packages/@aws-cdk/toolkit-lib/lib/api/io/private/io-helper.ts

@@ -1,6 +1,7 @@
 import type { IIoHost } from '../io-host';
 import type { IoMessage, IoRequest } from '../io-message';
 import type { ToolkitAction } from '../toolkit-action';
+import { IoDefaultMessages } from './io-default-messages';
 import type { SpanEnd, SpanDefinition } from './span';
 import { SpanMaker } from './span';
 
@@ -15,12 +16,18 @@ export class IoHelper implements IIoHost {
     return new IoHelper(ioHost, action);
   }
 
+  /**
+   * Simplified access to emit default messages.
+   */
+  public readonly defaults: IoDefaultMessages;
+
   private readonly ioHost: IIoHost;
   private readonly action: ToolkitAction;
 
   private constructor(ioHost: IIoHost, action: ToolkitAction) {
     this.ioHost = ioHost;
     this.action = action;
+    this.defaults = new IoDefaultMessages(this);
   }
 
   /**

packages/@aws-cdk/toolkit-lib/lib/api/notices/cached-data-source.ts

@@ -1,7 +1,7 @@
 import * as fs from 'fs-extra';
 import type { Notice, NoticeDataSource } from './types';
 import { ToolkitError } from '../../toolkit/toolkit-error';
-import type { IoDefaultMessages } from '../io/private';
+import type { IoHelper } from '../io/private';
 
 interface CachedNotices {
   expiration: number;
@@ -13,7 +13,7 @@ const TIME_TO_LIVE_ERROR = 1 * 60 * 1000; // 1 minute
 
 export class CachedDataSource implements NoticeDataSource {
   constructor(
-    private readonly ioMessages: IoDefaultMessages,
+    private readonly ioHelper: IoHelper,
     private readonly fileName: string,
     private readonly dataSource: NoticeDataSource,
     private readonly skipCache?: boolean,
@@ -31,7 +31,7 @@ export class CachedDataSource implements NoticeDataSource {
       try {
         updatedData = await this.fetchInner();
       } catch (e) {
-        this.ioMessages.debug(`Could not refresh notices: ${e}`);
+        await this.ioHelper.defaults.debug(`Could not refresh notices: ${e}`);
         updatedData = {
           expiration: Date.now() + TIME_TO_LIVE_ERROR,
           notices: [],
@@ -42,7 +42,7 @@ export class CachedDataSource implements NoticeDataSource {
       }
       return updatedData.notices;
     } else {
-      this.ioMessages.debug(`Reading cached notices from ${this.fileName}`);
+      await this.ioHelper.defaults.debug(`Reading cached notices from ${this.fileName}`);
       return data;
     }
   }
@@ -65,7 +65,7 @@ export class CachedDataSource implements NoticeDataSource {
         ? await fs.readJSON(this.fileName) as CachedNotices
         : defaultValue;
     } catch (e) {
-      this.ioMessages.debug(`Failed to load notices from cache: ${e}`);
+      await this.ioHelper.defaults.debug(`Failed to load notices from cache: ${e}`);
       return defaultValue;
     }
   }
@@ -74,7 +74,7 @@ export class CachedDataSource implements NoticeDataSource {
     try {
       await fs.writeJSON(this.fileName, cached);
     } catch (e) {
-      this.ioMessages.debug(`Failed to store notices in the cache: ${e}`);
+      await this.ioHelper.defaults.debug(`Failed to store notices in the cache: ${e}`);
     }
   }
 }

packages/@aws-cdk/toolkit-lib/lib/api/notices/filter.ts

@@ -1,5 +1,5 @@
 import * as semver from 'semver';
-import { IO, type IoDefaultMessages } from '../io/private';
+import { IO, type IoHelper } from '../io/private';
 import type { ConstructTreeNode } from '../tree';
 import { loadTreeFromDir } from '../tree';
 import type { BootstrappedEnvironment, Component, Notice } from './types';
@@ -58,13 +58,16 @@ export interface NoticesFilterFilterOptions {
 }
 
 export class NoticesFilter {
-  constructor(private readonly ioMessages: IoDefaultMessages) {
+  private readonly ioHelper: IoHelper;
+
+  constructor(ioHelper: IoHelper) {
+    this.ioHelper = ioHelper;
   }
 
-  public filter(options: NoticesFilterFilterOptions): FilteredNotice[] {
+  public async filter(options: NoticesFilterFilterOptions): Promise<FilteredNotice[]> {
     const components = [
-      ...this.constructTreeComponents(options.outDir),
-      ...this.otherComponents(options),
+      ...(await this.constructTreeComponents(options.outDir)),
+      ...(await this.otherComponents(options)),
     ];
 
     return this.findForNamedComponents(options.data, components);
@@ -73,7 +76,25 @@ export class NoticesFilter {
   /**
    * From a set of input options, return the notices components we are searching for
    */
-  private otherComponents(options: NoticesFilterFilterOptions): ActualComponent[] {
+  private async otherComponents(options: NoticesFilterFilterOptions): Promise<ActualComponent[]> {
+    // Bootstrap environments
+    let bootstrappedEnvironments = [];
+    for (const env of options.bootstrappedEnvironments) {
+      const semverBootstrapVersion = semver.coerce(env.bootstrapStackVersion);
+      if (!semverBootstrapVersion) {
+        // we don't throw because notices should never crash the cli.
+        await this.ioHelper.defaults.warning(`While filtering notices, could not coerce bootstrap version '${env.bootstrapStackVersion}' into semver`);
+        continue;
+      }
+
+      bootstrappedEnvironments.push({
+        name: 'bootstrap',
+        version: `${semverBootstrapVersion}`,
+        dynamicName: 'ENVIRONMENTS',
+        dynamicValue: env.environment.name,
+      });
+    }
+
     return [
       // CLI
       {
@@ -89,21 +110,7 @@ export class NoticesFilter {
       },
 
       // Bootstrap environments
-      ...options.bootstrappedEnvironments.flatMap(env => {
-        const semverBootstrapVersion = semver.coerce(env.bootstrapStackVersion);
-        if (!semverBootstrapVersion) {
-          // we don't throw because notices should never crash the cli.
-          this.ioMessages.warning(`While filtering notices, could not coerce bootstrap version '${env.bootstrapStackVersion}' into semver`);
-          return [];
-        }
-
-        return [{
-          name: 'bootstrap',
-          version: `${semverBootstrapVersion}`,
-          dynamicName: 'ENVIRONMENTS',
-          dynamicValue: env.environment.name,
-        }];
-      }),
+      ...bootstrappedEnvironments,
     ];
   }
 
@@ -185,8 +192,8 @@ export class NoticesFilter {
   /**
    * Load the construct tree from the given directory and return its components
    */
-  private constructTreeComponents(manifestDir: string): ActualComponent[] {
-    const tree = loadTreeFromDir(manifestDir, (msg: string) => void this.ioMessages.notify(IO.DEFAULT_ASSEMBLY_TRACE.msg(msg)));
+  private async constructTreeComponents(manifestDir: string): Promise<ActualComponent[]> {
+    const tree = await loadTreeFromDir(manifestDir, (msg: string) => this.ioHelper.notify(IO.DEFAULT_ASSEMBLY_TRACE.msg(msg)));
     if (!tree) {
       return [];
     }

packages/@aws-cdk/toolkit-lib/lib/api/notices/notices.ts

@@ -9,7 +9,7 @@ import { NoticesFilter } from './filter';
 import type { BootstrappedEnvironment, Notice, NoticeDataSource } from './types';
 import { WebsiteNoticeDataSource } from './web-data-source';
 import type { IoHelper } from '../io/private';
-import { IO, asIoHelper, IoDefaultMessages } from '../io/private';
+import { IO, asIoHelper } from '../io/private';
 
 const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json');
 
@@ -102,7 +102,6 @@ export class Notices {
   private readonly acknowledgedIssueNumbers: Set<Number>;
   private readonly httpOptions: SdkHttpOptions;
   private readonly ioHelper: IoHelper;
-  private readonly ioMessages: IoDefaultMessages;
   private readonly cliVersion: string;
 
   private data: Set<Notice> = new Set();
@@ -116,7 +115,6 @@ export class Notices {
     this.output = props.output ?? 'cdk.out';
     this.httpOptions = props.httpOptions ?? {};
     this.ioHelper = asIoHelper(props.ioHost, 'notices' as any /* forcing a CliAction to a ToolkitAction */);
-    this.ioMessages = new IoDefaultMessages(this.ioHelper);
     this.cliVersion = props.cliVersion;
   }
 
@@ -144,16 +142,16 @@ export class Notices {
    */
   public async refresh(options: NoticesRefreshOptions = {}) {
     const innerDataSource = options.dataSource ?? new WebsiteNoticeDataSource(this.ioHelper, this.httpOptions);
-    const dataSource = new CachedDataSource(this.ioMessages, CACHE_FILE_PATH, innerDataSource, options.force ?? false);
+    const dataSource = new CachedDataSource(this.ioHelper, CACHE_FILE_PATH, innerDataSource, options.force ?? false);
     const notices = await dataSource.fetch();
     this.data = new Set(notices);
   }
 
   /**
    * Filter the data sourece for relevant notices
    */
-  public filter(options: NoticesDisplayOptions = {}): FilteredNotice[] {
-    return new NoticesFilter(this.ioMessages).filter({
+  public filter(options: NoticesDisplayOptions = {}): Promise<FilteredNotice[]> {
+    return new NoticesFilter(this.ioHelper).filter({
       data: this.noticesFromData(options.includeAcknowledged ?? false),
       cliVersion: this.cliVersion,
       outDir: this.output,
@@ -165,7 +163,7 @@ export class Notices {
    * Display the relevant notices (unless context dictates we shouldn't).
    */
   public async display(options: NoticesDisplayOptions = {}): Promise<void> {
-    const filteredNotices = this.filter(options);
+    const filteredNotices = await this.filter(options);
 
     if (filteredNotices.length > 0) {
       await this.ioHelper.notify(IO.CDK_TOOLKIT_I0100.msg([