Refactor ChangeFiles API.

Author: iclanton

libraries/rush-lib/src/cli/actions/ChangeAction.ts

@@ -382,32 +382,31 @@ export class ChangeAction extends BaseRushAction {
   }
 
   private async _verifyAsync(): Promise<void> {
-    const changedPackages: string[] = await this._getChangedProjectNamesAsync();
-    const strictValidation: boolean =
-      !!this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation;
+    const changedProjectNames: string[] = await this._getChangedProjectNamesAsync();
+    const strictValidation: boolean | undefined =
+      this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation;
 
     // When strict validation is enabled, validate ALL change files to catch references to
     // deleted or nonexistent projects. Otherwise, only validate change files added on this branch.
+    const changeFilesInstance: ChangeFiles = new ChangeFiles(this.rushConfiguration);
     let filesToValidate: string[];
     if (strictValidation) {
-      const changeFilesInstance: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder);
-      filesToValidate = await changeFilesInstance.getFilesAsync();
+      filesToValidate = await changeFilesInstance.getAllChangeFilesAsync();
     } else {
       filesToValidate = await this._getChangeFilesAsync();
     }
 
-    if (changedPackages.length > 0 || filesToValidate.length > 0) {
+    if (changedProjectNames.length > 0 || filesToValidate.length > 0) {
       const deletedProjectNames: Set<string> | undefined = strictValidation
         ? await this._getDeletedProjectNamesAsync()
         : undefined;
 
-      await ChangeFiles.validateAsync(
-        this.terminal,
+      await changeFilesInstance.validateAsync({
+        terminal: this.terminal,
         filesToValidate,
-        changedPackages,
-        this.rushConfiguration,
+        changedProjectNames,
         deletedProjectNames
-      );
+      });
     } else {
       this._logNoChangeFileRequired();
     }
@@ -460,16 +459,15 @@ export class ChangeAction extends BaseRushAction {
       );
     }
 
-    const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder);
-    const allChangeFiles: string[] = await changeFiles.getFilesAsync();
+    const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration);
+    const allChangeFiles: string[] = await changeFiles.getAllChangeFilesAsync();
     const deletedProjectNames: Set<string> = await this._getDeletedProjectNamesAsync();
-    await ChangeFiles.validateAsync(
-      this.terminal,
-      allChangeFiles,
-      [],
-      this.rushConfiguration,
+    await changeFiles.validateAsync({
+      terminal: this.terminal,
+      filesToValidate: allChangeFiles,
+      changedProjectNames: [],
       deletedProjectNames
-    );
+    });
   }
 
   /**

libraries/rush-lib/src/cli/actions/PublishAction.ts

@@ -281,11 +281,7 @@ export class PublishAction extends BaseRushAction {
     allPackages: ReadonlyMap<string, RushConfigurationProject>
   ): Promise<void> {
     const changeManager: ChangeManager = new ChangeManager(this.rushConfiguration);
-    await changeManager.loadAsync(
-      this.rushConfiguration.changesFolder,
-      this._prereleaseToken,
-      this._addCommitDetails.value
-    );
+    await changeManager.loadAsync(this._prereleaseToken, this._addCommitDetails.value);
 
     if (changeManager.hasChanges()) {
       const orderedChanges: IChangeInfo[] = changeManager.packageChanges;

libraries/rush-lib/src/logic/ChangeFiles.ts

@@ -11,6 +11,17 @@ import type { RushConfigurationProject } from '../api/RushConfigurationProject';
 import { type LockStepVersionPolicy, VersionPolicyDefinitionName } from '../api/VersionPolicy';
 import schemaJson from '../schemas/change-file.schema.json';
 
+export interface IValidateOptions {
+  terminal: ITerminal;
+  filesToValidate: Iterable<string>;
+  changedProjectNames: Iterable<string>;
+  /**
+   * Optional set of project names that were removed from rush.json.
+   * When provided, produces a more specific error message for these projects.
+   */
+  deletedProjectNames?: ReadonlySet<string>;
+}
+
 /**
  * This class represents the collection of change files existing in the repo and provides operations
  * for those change files.
@@ -20,37 +31,38 @@ export class ChangeFiles {
    * Change file path relative to changes folder.
    */
   private _files: string[] | undefined;
-  private _changesPath: string;
+  private readonly _rushConfiguration: RushConfiguration;
+  private readonly _changesPath: string;
 
-  public constructor(changesPath: string) {
-    this._changesPath = changesPath;
+  public constructor(rushConfiguration: RushConfiguration) {
+    this._rushConfiguration = rushConfiguration;
+    this._changesPath = rushConfiguration.changesFolder;
   }
 
   /**
    * Validate if the newly added change files match the changed packages.
-   *
-   * @param deletedProjectNames - Optional set of project names that were removed from rush.json.
-   *   When provided, produces a more specific error message for these projects.
    */
-  public static async validateAsync(
-    terminal: ITerminal,
-    newChangeFilePaths: string[],
-    changedPackages: string[],
-    rushConfiguration: RushConfiguration,
-    deletedProjectNames?: ReadonlySet<string>
-  ): Promise<void> {
+  public async validateAsync(options: IValidateOptions): Promise<void> {
+    const { terminal, filesToValidate, changedProjectNames, deletedProjectNames } = options;
     const schema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson);
+    const rushConfiguration: RushConfiguration = this._rushConfiguration;
+    const {
+      hotfixChangeEnabled,
+      experimentsConfiguration: {
+        configuration: { strictChangefileValidation }
+      }
+    } = rushConfiguration;
 
     const projectsWithChangeDescriptions: Set<string> = new Set();
     const changefilesByProjectName: Map<string, string[]> = new Map();
     await Async.forEachAsync(
-      newChangeFilePaths,
+      filesToValidate,
       async (filePath) => {
         terminal.writeLine(`Found change file: ${filePath}`);
 
         const changeFile: IChangeInfo = JsonFile.loadAndValidate(filePath, schema);
 
-        if (rushConfiguration.hotfixChangeEnabled) {
+        if (hotfixChangeEnabled) {
           if (changeFile && changeFile.changes) {
             for (const change of changeFile.changes) {
               if (change.type !== 'none' && change.type !== 'hotfix') {
@@ -81,7 +93,7 @@ export class ChangeFiles {
       { concurrency: 50 }
     );
 
-    if (rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) {
+    if (strictChangefileValidation) {
       const errors: string[] = [];
 
       for (const packageName of projectsWithChangeDescriptions) {
@@ -121,7 +133,7 @@ export class ChangeFiles {
       }
     }
 
-    const projectsMissingChangeDescriptions: Set<string> = new Set(changedPackages);
+    const projectsMissingChangeDescriptions: Set<string> = new Set(changedProjectNames);
     for (const name of projectsWithChangeDescriptions) {
       projectsMissingChangeDescriptions.delete(name);
     }
@@ -167,7 +179,7 @@ export class ChangeFiles {
   /**
    * Get the array of absolute paths of change files.
    */
-  public async getFilesAsync(): Promise<string[]> {
+  public async getAllChangeFilesAsync(): Promise<string[]> {
     if (!this._files) {
       const { default: glob } = await import('fast-glob');
       this._files = (await glob('**/*.json', { cwd: this._changesPath, absolute: true })) || [];
@@ -198,7 +210,7 @@ export class ChangeFiles {
         packagesToInclude.add(changelog.name);
       });
 
-      const files: string[] = await this.getFilesAsync();
+      const files: string[] = await this.getAllChangeFilesAsync();
       const filesToDelete: string[] = [];
       await Async.forEachAsync(
         files,
@@ -222,7 +234,7 @@ export class ChangeFiles {
       return await this._deleteFilesAsync(terminal, filesToDelete, shouldDelete);
     } else {
       // Delete all change files.
-      const files: string[] = await this.getFilesAsync();
+      const files: string[] = await this.getAllChangeFilesAsync();
       return await this._deleteFilesAsync(terminal, files, shouldDelete);
     }
   }
@@ -238,7 +250,7 @@ export class ChangeFiles {
       );
 
       await Async.forEachAsync(
-        files,
+        files.sort(),
         async (filePath) => {
           terminal.writeLine(` - ${filePath}`);
           if (shouldDelete) {

libraries/rush-lib/src/logic/ChangeManager.ts

@@ -34,20 +34,18 @@ export class ChangeManager {
 
   /**
    * Load changes from change files
-   * @param changesPath - location of change files
    * @param prereleaseToken - prerelease token
    * @param includeCommitDetails - whether commit details need to be included in changes
    */
   public async loadAsync(
-    changesPath: string,
     prereleaseToken: PrereleaseToken = new PrereleaseToken(),
     includeCommitDetails: boolean = false
   ): Promise<void> {
     this._allPackages = this._rushConfiguration.projectsByName;
 
     this._prereleaseToken = prereleaseToken;
 
-    this._changeFiles = new ChangeFiles(changesPath);
+    this._changeFiles = new ChangeFiles(this._rushConfiguration);
     this._allChanges = await PublishUtilities.findChangeRequestsAsync(
       this._allPackages,
       this._rushConfiguration,

libraries/rush-lib/src/logic/PublishUtilities.ts

@@ -78,7 +78,7 @@ export class PublishUtilities {
     // eslint-disable-next-line no-console
     console.log(`Finding changes in: ${changeFiles.getChangesPath()}`);
 
-    const files: string[] = await changeFiles.getFilesAsync();
+    const files: string[] = await changeFiles.getAllChangeFilesAsync();
 
     // Add the minimum changes defined by the change descriptions.
     for (const changeFilePath of files) {

libraries/rush-lib/src/logic/VersionManager.ts

@@ -90,7 +90,7 @@ export class VersionManager {
       this._getManuallyVersionedProjects()
     );
 
-    await changeManager.loadAsync(this._rushConfiguration.changesFolder);
+    await changeManager.loadAsync();
     if (changeManager.hasChanges()) {
       changeManager.validateChanges(this._versionPolicyConfiguration);
       changeManager.apply(!!shouldCommit)!.forEach((packageJson) => {