chore(toolkit-lib): cleanup unused and private refactor exports (#722)

Removes unused and internal exports for refactor. `MappingSource` is not
used anymore for now. Mark `RefactorOptions` as `readonly` as per our
best practices. Move `mappingsByEnvironment` and `parseMappingGroups` to
be internal as intended.

Also makes the API Extractor task fail on errors now, to prevent future
issues with missing exports. This is how I noticed the unintentional
exports in the first place.

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

Author: mrgrain

.projenrc.ts

@@ -1052,7 +1052,7 @@ toolkitLib.gitignore.addPatterns(
 const apiExtractorDocsTask = toolkitLib.addTask('docs', {
   exec: [
     // Run api-extractor to generate the API model
-    'api-extractor run || true',
+    'api-extractor run',
     // Create a directory for the API model
     'mkdir -p dist/api-extractor-docs/cdk/api/toolkit-lib',
     // Copy the API model to the directory (with error handling)

packages/@aws-cdk/toolkit-lib/.projen/tasks.json

@@ -13,7 +13,7 @@ import type { ResourcesToImport } from '../../../api/resource-import';
 import { removeNonImportResources, ResourceMigrator } from '../../../api/resource-import';
 import { ToolkitError } from '../../../toolkit/toolkit-error';
 import { deserializeStructure, formatErrorMessage } from '../../../util';
-import { mappingsByEnvironment } from '../../refactor/index';
+import { mappingsByEnvironment } from '../../refactor/private/mapping-helpers';
 
 export function prepareDiff(
   ioHelper: IoHelper,

packages/@aws-cdk/toolkit-lib/lib/actions/diff/private/helpers.ts

@@ -1,69 +1,4 @@
-import type * as cxapi from '@aws-cdk/cx-api';
 import type { StackSelector } from '../../api';
-import type { SdkProvider } from '../../api/aws-auth/sdk-provider';
-import type { ExcludeList } from '../../api/refactoring';
-import { groupStacks, InMemoryExcludeList, NeverExclude, RefactoringContext } from '../../api/refactoring';
-import { ToolkitError } from '../../toolkit/toolkit-error';
-
-type MappingType = 'auto' | 'explicit';
-
-/**
- * The source of the resource mappings to be used for refactoring.
- */
-export class MappingSource {
-  /**
-   * The mapping will be automatically generated based on a comparison of
-   * the deployed stacks and the local stacks.
-   *
-   * @param exclude - A list of resource locations to exclude from the mapping.
-   */
-  public static auto(exclude: string[] = []): MappingSource {
-    const excludeList = new InMemoryExcludeList(exclude);
-    return new MappingSource('auto', [], excludeList);
-  }
-
-  /**
-   * An explicitly provided list of mappings, which will be used for refactoring.
-   */
-  public static explicit(groups: MappingGroup[]): MappingSource {
-    return new MappingSource('explicit', groups, new NeverExclude());
-  }
-
-  /**
-   * An explicitly provided list of mappings, which will be used for refactoring,
-   * but in reverse, that is, the source locations will become the destination
-   * locations and vice versa.
-   */
-  public static reverse(groups: MappingGroup[]): MappingSource {
-    const reverseGroups = groups.map((group) => ({
-      ...group,
-      resources: Object.fromEntries(Object.entries(group.resources).map(([src, dst]) => [dst, src])),
-    }));
-
-    return MappingSource.explicit(reverseGroups);
-  }
-
-  /**
-   * @internal
-   */
-  public readonly source: MappingType;
-
-  /**
-   * @internal
-   */
-  public readonly groups: MappingGroup[];
-
-  /**
-   * @internal
-   */
-  public readonly exclude: ExcludeList;
-
-  private constructor(source: MappingType, groups: MappingGroup[], exclude: ExcludeList) {
-    this.source = source;
-    this.groups = groups;
-    this.exclude = exclude;
-  }
-}
 
 export interface RefactorOptions {
   /**
@@ -77,30 +12,30 @@ export interface RefactorOptions {
    * List of overrides to be applied to resolve possible ambiguities in the
    * computed list of mappings.
    */
-  overrides?: MappingGroup[];
+  readonly overrides?: MappingGroup[];
 
   /**
    * Criteria for selecting stacks to compare with the deployed stacks in the
    * target environment.
    */
-  stacks?: StackSelector;
+  readonly stacks?: StackSelector;
 
   /**
    * A list of names of additional deployed stacks to be included in the comparison.
    */
-  additionalStackNames?: string[];
+  readonly additionalStackNames?: string[];
 }
 
 export interface MappingGroup {
   /**
    * The account ID of the environment in which the mapping is valid.
    */
-  account: string;
+  readonly account: string;
 
   /**
    * The region of the environment in which the mapping is valid.
    */
-  region: string;
+  readonly region: string;
 
   /**
    * A collection of resource mappings, where each key is the source location
@@ -110,62 +45,7 @@ export interface MappingGroup {
    * location that is not already occupied by any resource.
    *
    */
-  resources: {
-    [key: string]: string;
+  readonly resources: {
+    readonly [key: string]: string;
   };
 }
-
-export function parseMappingGroups(s: string) {
-  const mappingGroups = doParse();
-
-  // Validate that there are no duplicate destinations.
-  // By construction, there are no duplicate sources, already.
-  for (let group of mappingGroups) {
-    const destinations = new Set<string>();
-
-    for (const destination of Object.values(group.resources)) {
-      if (destinations.has(destination)) {
-        throw new ToolkitError(
-          `Duplicate destination resource '${destination}' in environment ${group.account}/${group.region}`,
-        );
-      }
-      destinations.add(destination);
-    }
-  }
-
-  return mappingGroups;
-
-  function doParse(): MappingGroup[] {
-    const content = JSON.parse(s);
-    if (content.environments || !Array.isArray(content.environments)) {
-      return content.environments;
-    } else {
-      throw new ToolkitError("Expected an 'environments' array");
-    }
-  }
-}
-
-export interface EnvironmentSpecificMappings {
-  readonly environment: cxapi.Environment;
-  readonly mappings: Record<string, string>;
-}
-
-export async function mappingsByEnvironment(
-  stackArtifacts: cxapi.CloudFormationStackArtifact[],
-  sdkProvider: SdkProvider,
-  ignoreModifications?: boolean,
-): Promise<EnvironmentSpecificMappings[]> {
-  const groups = await groupStacks(sdkProvider, stackArtifacts, []);
-  return groups.map((group) => {
-    const context = new RefactoringContext({
-      ...group,
-      ignoreModifications,
-    });
-    return {
-      environment: context.environment,
-      mappings: Object.fromEntries(
-        context.mappings.map((m) => [m.source.toLocationString(), m.destination.toLocationString()]),
-      ),
-    };
-  });
-}

packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts

@@ -0,0 +1,61 @@
+import type * as cxapi from '@aws-cdk/cx-api';
+import type { MappingGroup } from '..';
+import type { SdkProvider } from '../../../api/aws-auth/sdk-provider';
+import { groupStacks, RefactoringContext } from '../../../api/refactoring';
+import { ToolkitError } from '../../../toolkit/toolkit-error';
+
+export function parseMappingGroups(s: string) {
+  const mappingGroups = doParse();
+
+  // Validate that there are no duplicate destinations.
+  // By construction, there are no duplicate sources, already.
+  for (let group of mappingGroups) {
+    const destinations = new Set<string>();
+
+    for (const destination of Object.values(group.resources)) {
+      if (destinations.has(destination)) {
+        throw new ToolkitError(
+          `Duplicate destination resource '${destination}' in environment ${group.account}/${group.region}`,
+        );
+      }
+      destinations.add(destination);
+    }
+  }
+
+  return mappingGroups;
+
+  function doParse(): MappingGroup[] {
+    const content = JSON.parse(s);
+    if (content.environments || !Array.isArray(content.environments)) {
+      return content.environments;
+    } else {
+      throw new ToolkitError("Expected an 'environments' array");
+    }
+  }
+}
+
+interface EnvironmentSpecificMappings {
+  readonly environment: cxapi.Environment;
+  readonly mappings: Record<string, string>;
+}
+
+export async function mappingsByEnvironment(
+  stackArtifacts: cxapi.CloudFormationStackArtifact[],
+  sdkProvider: SdkProvider,
+  ignoreModifications?: boolean,
+): Promise<EnvironmentSpecificMappings[]> {
+  const groups = await groupStacks(sdkProvider, stackArtifacts, []);
+  return groups.map((group) => {
+    const context = new RefactoringContext({
+      ...group,
+      ignoreModifications,
+    });
+    return {
+      environment: context.environment,
+      mappings: Object.fromEntries(
+        context.mappings.map((m) => [m.source.toLocationString(), m.destination.toLocationString()]),
+      ),
+    };
+  });
+}
+

packages/@aws-cdk/toolkit-lib/lib/actions/refactor/private/mapping-helpers.ts

@@ -0,0 +1,2 @@
+/* eslint-disable import/no-relative-packages */
+export { mappingsByEnvironment, parseMappingGroups } from '../../../@aws-cdk/toolkit-lib/lib/actions/refactor/private/mapping-helpers';

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

@@ -3,7 +3,7 @@ import { format } from 'util';
 import { RequireApproval } from '@aws-cdk/cloud-assembly-schema';
 import * as cxapi from '@aws-cdk/cx-api';
 import type { DeploymentMethod, ToolkitAction, ToolkitOptions } from '@aws-cdk/toolkit-lib';
-import { parseMappingGroups, PermissionChangeType, Toolkit, ToolkitError, mappingsByEnvironment } from '@aws-cdk/toolkit-lib';
+import { PermissionChangeType, Toolkit, ToolkitError } from '@aws-cdk/toolkit-lib';
 import * as chalk from 'chalk';
 import * as chokidar from 'chokidar';
 import * as fs from 'fs-extra';
@@ -32,6 +32,7 @@ import type { BootstrapEnvironmentOptions } from '../api/bootstrap';
 import { Bootstrapper } from '../api/bootstrap';
 import { ExtendedStackSelection, StackCollection } from '../api/cloud-assembly';
 import type { Deployments, SuccessfulDeployStackResult } from '../api/deployments';
+import { mappingsByEnvironment, parseMappingGroups } from '../api/refactor';
 import { type Tag } from '../api/tags';
 import { StackActivityProgress } from '../commands/deploy';
 import { listStacks } from '../commands/list-stacks';