From d149c694f336a4d23fc461451f76f3cf79773a59 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 15:30:00 -0400 Subject: [PATCH 01/14] Add stricter verification to 'rush change --verify' to disallow changefiles for nonexistent projects. --- common/reviews/api/rush-lib.api.md | 1 + .../common/config/rush/experiments.json | 9 +++++- .../src/api/ExperimentsConfiguration.ts | 8 +++++ libraries/rush-lib/src/logic/ChangeFiles.ts | 32 +++++++++++++++++++ .../src/schemas/experiments.schema.json | 4 +++ 5 files changed, 53 insertions(+), 1 deletion(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 14063d1543e..408049d9e09 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -477,6 +477,7 @@ export interface IExperimentsJson { omitImportersFromPreventManualShrinkwrapChanges?: boolean; printEventHooksOutputToConsole?: boolean; rushAlerts?: boolean; + strictChangefileValidation?: boolean; useIPCScriptsInWatchMode?: boolean; usePnpmFrozenLockfileForRushInstall?: boolean; usePnpmLockfileOnlyThenFrozenLockfileForRushUpdate?: boolean; diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json index ab57492884b..b5b8d5797a1 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json @@ -125,5 +125,12 @@ * macOS to store extended attributes on filesystems that don't support them, and should generally not * be included in the shared build cache. */ - /*[LINE "HYPOTHETICAL"]*/ "omitAppleDoubleFilesFromBuildCache": true + /*[LINE "HYPOTHETICAL"]*/ "omitAppleDoubleFilesFromBuildCache": true, + + /** + * If true, "rush change --verify" will report errors if change files reference projects that do not + * exist in the Rush configuration, or if change files target a project that belongs to a lockstepped + * version policy but is not the policy's main project. + */ + /*[LINE "HYPOTHETICAL"]*/ "strictChangefileValidation": true } diff --git a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts index f8a9ae66e34..3d2c9416dc6 100644 --- a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts @@ -136,6 +136,14 @@ export interface IExperimentsJson { * be included in the shared build cache. */ omitAppleDoubleFilesFromBuildCache?: boolean; + + /** + * If true, `rush change --verify` will perform additional validation of change files. Specifically, + * it will report errors if change files reference projects that do not exist in the Rush configuration, + * or if change files target a project that belongs to a lockstepped version policy but is not the + * policy's main project. + */ + strictChangefileValidation?: boolean; } const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 93528d345dc..1286440d26e 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/libraries/rush-lib/src/logic/ChangeFiles.ts @@ -6,6 +6,8 @@ import { Async, FileSystem, JsonFile, JsonSchema } from '@rushstack/node-core-li import type { IChangeInfo } from '../api/ChangeManagement'; import type { IChangelog } from '../api/Changelog'; import type { RushConfiguration } from '../api/RushConfiguration'; +import type { RushConfigurationProject } from '../api/RushConfigurationProject'; +import { type LockStepVersionPolicy, VersionPolicyDefinitionName } from '../api/VersionPolicy'; import schemaJson from '../schemas/change-file.schema.json'; /** @@ -60,6 +62,36 @@ export class ChangeFiles { } }); + if (rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { + const errors: string[] = []; + + for (const packageName of projectsWithChangeDescriptions) { + const project: RushConfigurationProject | undefined = rushConfiguration.getProjectByName(packageName); + + if (!project) { + errors.push( + `Change file(s) reference a project "${packageName}" that does not exist in the Rush configuration.` + ); + continue; + } + + if (project.versionPolicy?.definitionName === VersionPolicyDefinitionName.lockStepVersion) { + const { mainProject }: LockStepVersionPolicy = project.versionPolicy as LockStepVersionPolicy; + if (mainProject && mainProject !== packageName) { + errors.push( + `Change file(s) reference the project "${packageName}" which belongs to lockstepped ` + + `version policy "${project.versionPolicy.policyName}". Change files should be ` + + `created for the policy's main project "${mainProject}" instead.` + ); + } + } + } + + if (errors.length > 0) { + throw new Error(errors.join('\n')); + } + } + const projectsMissingChangeDescriptions: Set = new Set(changedPackages); projectsWithChangeDescriptions.forEach((name) => projectsMissingChangeDescriptions.delete(name)); if (projectsMissingChangeDescriptions.size > 0) { diff --git a/libraries/rush-lib/src/schemas/experiments.schema.json b/libraries/rush-lib/src/schemas/experiments.schema.json index 8a92fa9ee67..532bc26c76a 100644 --- a/libraries/rush-lib/src/schemas/experiments.schema.json +++ b/libraries/rush-lib/src/schemas/experiments.schema.json @@ -85,6 +85,10 @@ "omitAppleDoubleFilesFromBuildCache": { "description": "If true, when running on macOS, Rush will omit AppleDouble files (._*) from build cache archives when a companion file exists in the same directory. AppleDouble files are automatically created by macOS to store extended attributes on filesystems that don't support them, and should generally not be included in the shared build cache.", "type": "boolean" + }, + "strictChangefileValidation": { + "description": "If true, 'rush change --verify' will report errors if change files reference projects that do not exist in the Rush configuration, or if change files target a project that belongs to a lockstepped version policy but is not the policy's main project.", + "type": "boolean" } }, "additionalProperties": false From 64e86f21a2de13708c686d94f134e441c9005806 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 15:41:33 -0400 Subject: [PATCH 02/14] Add unit tests for the new change validation feature. --- .../src/logic/test/ChangeFiles.test.ts | 109 +++++++++++++++++- .../test/strictValidation/mainLockstep.json | 9 ++ .../strictValidation/nonMainLockstep.json | 9 ++ .../strictValidation/nonexistentProject.json | 9 ++ 4 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 libraries/rush-lib/src/logic/test/strictValidation/mainLockstep.json create mode 100644 libraries/rush-lib/src/logic/test/strictValidation/nonMainLockstep.json create mode 100644 libraries/rush-lib/src/logic/test/strictValidation/nonexistentProject.json diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 6d95500d8be..648236d465e 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -6,12 +6,19 @@ import { Path } from '@rushstack/node-core-library'; import type { IChangelog } from '../../api/Changelog'; import { ChangeFiles } from '../ChangeFiles'; import type { RushConfiguration } from '../../api/RushConfiguration'; +import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; +import { VersionPolicyDefinitionName } from '../../api/VersionPolicy'; +import type { ExperimentsConfiguration } from '../../api/ExperimentsConfiguration'; describe(ChangeFiles.name, () => { let rushConfiguration: RushConfiguration; beforeEach(() => { - rushConfiguration = {} as RushConfiguration; + rushConfiguration = { + experimentsConfiguration: { + configuration: {} + } + } as RushConfiguration; }); describe(ChangeFiles.prototype.getFilesAsync.name, () => { @@ -57,7 +64,10 @@ describe(ChangeFiles.name, () => { it('allows a hotfix in a hotfix branch.', () => { const changeFile: string = `${__dirname}/multipleHotfixChanges/change1.json`; const changedPackages: string[] = ['a']; - ChangeFiles.validate([changeFile], changedPackages, { hotfixChangeEnabled: true } as RushConfiguration); + ChangeFiles.validate([changeFile], changedPackages, { + ...rushConfiguration, + hotfixChangeEnabled: true + } as RushConfiguration); }); it('throws when there is any missing package.', () => { @@ -94,6 +104,101 @@ describe(ChangeFiles.name, () => { ChangeFiles.validate([changeFileA, changeFileB, changeFileC], changedPackages, rushConfiguration); }).not.toThrow(Error); }); + + describe('with strictChangefileValidation', () => { + let strictConfig: RushConfiguration; + + function createStrictConfig( + getProjectByName: (name: string) => RushConfigurationProject | undefined + ): RushConfiguration { + return { + experimentsConfiguration: { + configuration: { strictChangefileValidation: true } + } as ExperimentsConfiguration, + getProjectByName + } as unknown as RushConfiguration; + } + + it('throws when change file references a nonexistent project', () => { + const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; + strictConfig = createStrictConfig(() => undefined); + expect(() => { + ChangeFiles.validate([changeFile], ['nonexistent-package'], strictConfig); + }).toThrow(/does not exist in the Rush configuration/); + }); + + it('throws when change file references a non-main lockstep project', () => { + const changeFile: string = `${__dirname}/strictValidation/nonMainLockstep.json`; + strictConfig = createStrictConfig((name: string) => { + if (name === 'lockstep-secondary') { + return { + packageName: 'lockstep-secondary', + versionPolicy: { + policyName: 'myLockstep', + definitionName: VersionPolicyDefinitionName.lockStepVersion, + mainProject: 'lockstep-main' + } + } as unknown as RushConfigurationProject; + } + return undefined; + }); + expect(() => { + ChangeFiles.validate([changeFile], ['lockstep-secondary'], strictConfig); + }).toThrow(/main project "lockstep-main"/); + }); + + it('does not throw when change file references the main lockstep project', () => { + const changeFile: string = `${__dirname}/strictValidation/mainLockstep.json`; + strictConfig = createStrictConfig((name: string) => { + if (name === 'lockstep-main') { + return { + packageName: 'lockstep-main', + versionPolicy: { + policyName: 'myLockstep', + definitionName: VersionPolicyDefinitionName.lockStepVersion, + mainProject: 'lockstep-main' + } + } as unknown as RushConfigurationProject; + } + return undefined; + }); + expect(() => { + ChangeFiles.validate([changeFile], ['lockstep-main'], strictConfig); + }).not.toThrow(); + }); + + it('does not throw when change file references a lockstep project with no mainProject', () => { + const changeFile: string = `${__dirname}/strictValidation/mainLockstep.json`; + strictConfig = createStrictConfig((name: string) => { + if (name === 'lockstep-main') { + return { + packageName: 'lockstep-main', + versionPolicy: { + policyName: 'myLockstep', + definitionName: VersionPolicyDefinitionName.lockStepVersion, + mainProject: undefined + } + } as unknown as RushConfigurationProject; + } + return undefined; + }); + expect(() => { + ChangeFiles.validate([changeFile], ['lockstep-main'], strictConfig); + }).not.toThrow(); + }); + + it('does not throw when experiment is disabled', () => { + const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; + const config: RushConfiguration = { + experimentsConfiguration: { + configuration: { strictChangefileValidation: false } + } as ExperimentsConfiguration + } as unknown as RushConfiguration; + expect(() => { + ChangeFiles.validate([changeFile], ['nonexistent-package'], config); + }).not.toThrow(); + }); + }); }); describe(ChangeFiles.prototype.deleteAllAsync.name, () => { diff --git a/libraries/rush-lib/src/logic/test/strictValidation/mainLockstep.json b/libraries/rush-lib/src/logic/test/strictValidation/mainLockstep.json new file mode 100644 index 00000000000..797943db49d --- /dev/null +++ b/libraries/rush-lib/src/logic/test/strictValidation/mainLockstep.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "lockstep-main", + "type": "patch", + "comment": "Change for the main lockstep project" + } + ] +} diff --git a/libraries/rush-lib/src/logic/test/strictValidation/nonMainLockstep.json b/libraries/rush-lib/src/logic/test/strictValidation/nonMainLockstep.json new file mode 100644 index 00000000000..71c400f6f04 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/strictValidation/nonMainLockstep.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "lockstep-secondary", + "type": "patch", + "comment": "Change for a non-main lockstep project" + } + ] +} diff --git a/libraries/rush-lib/src/logic/test/strictValidation/nonexistentProject.json b/libraries/rush-lib/src/logic/test/strictValidation/nonexistentProject.json new file mode 100644 index 00000000000..4a8b812223e --- /dev/null +++ b/libraries/rush-lib/src/logic/test/strictValidation/nonexistentProject.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "nonexistent-package", + "type": "patch", + "comment": "Change for a project that does not exist" + } + ] +} From 4a8960f7c701a73d51aa56fdd18ec90aad043bb0 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 15:52:37 -0400 Subject: [PATCH 03/14] Refactor changefile validation to be async. --- .../rush-lib/src/cli/actions/ChangeAction.ts | 2 +- libraries/rush-lib/src/logic/ChangeFiles.ts | 62 ++++++++------ .../src/logic/test/ChangeFiles.test.ts | 82 +++++++++---------- 3 files changed, 75 insertions(+), 71 deletions(-) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 9167d5bca15..dabdd0bfd69 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -387,7 +387,7 @@ export class ChangeAction extends BaseRushAction { private async _validateChangeFileAsync(changedPackages: string[]): Promise { const files: string[] = await this._getChangeFilesAsync(); - ChangeFiles.validate(files, changedPackages, this.rushConfiguration); + await ChangeFiles.validateAsync(files, changedPackages, this.rushConfiguration); } private async _getChangeFilesAsync(): Promise { diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 1286440d26e..40158ca3707 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/libraries/rush-lib/src/logic/ChangeFiles.ts @@ -28,39 +28,45 @@ export class ChangeFiles { /** * Validate if the newly added change files match the changed packages. */ - public static validate( + public static async validateAsync( newChangeFilePaths: string[], changedPackages: string[], rushConfiguration: RushConfiguration - ): void { + ): Promise { const schema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); const projectsWithChangeDescriptions: Set = new Set(); - newChangeFilePaths.forEach((filePath) => { - // eslint-disable-next-line no-console - console.log(`Found change file: ${filePath}`); - - const changeFile: IChangeInfo = JsonFile.loadAndValidate(filePath, schema); - - if (rushConfiguration.hotfixChangeEnabled) { - if (changeFile && changeFile.changes) { - for (const change of changeFile.changes) { - if (change.type !== 'none' && change.type !== 'hotfix') { - throw new Error( - `Change file ${filePath} specifies a type of '${change.type}' ` + - `but only 'hotfix' and 'none' change types may be used in a branch with 'hotfixChangeEnabled'.` - ); + await Async.forEachAsync( + newChangeFilePaths, + async (filePath) => { + // eslint-disable-next-line no-console + console.log(`Found change file: ${filePath}`); + + const changeFile: IChangeInfo = JsonFile.loadAndValidate(filePath, schema); + + if (rushConfiguration.hotfixChangeEnabled) { + if (changeFile && changeFile.changes) { + for (const change of changeFile.changes) { + if (change.type !== 'none' && change.type !== 'hotfix') { + throw new Error( + `Change file ${filePath} specifies a type of '${change.type}' ` + + `but only 'hotfix' and 'none' change types may be used in a branch with 'hotfixChangeEnabled'.` + ); + } } } } - } - if (changeFile && changeFile.changes) { - changeFile.changes.forEach((change) => projectsWithChangeDescriptions.add(change.packageName)); - } else { - throw new Error(`Invalid change file: ${filePath}`); - } - }); + if (changeFile && changeFile.changes) { + for (const { packageName } of changeFile.changes) { + projectsWithChangeDescriptions.add(packageName); + } + } else { + throw new Error(`Invalid change file: ${filePath}`); + } + }, + { concurrency: 50 } + ); if (rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { const errors: string[] = []; @@ -93,10 +99,14 @@ export class ChangeFiles { } const projectsMissingChangeDescriptions: Set = new Set(changedPackages); - projectsWithChangeDescriptions.forEach((name) => projectsMissingChangeDescriptions.delete(name)); + for (const name of projectsWithChangeDescriptions) { + projectsMissingChangeDescriptions.delete(name); + } + if (projectsMissingChangeDescriptions.size > 0) { - const projectsMissingChangeDescriptionsArray: string[] = []; - projectsMissingChangeDescriptions.forEach((name) => projectsMissingChangeDescriptionsArray.push(name)); + const projectsMissingChangeDescriptionsArray: string[] = Array.from( + projectsMissingChangeDescriptions + ).sort(); throw new Error( [ 'The following projects have been changed and require change descriptions, but change descriptions were not ' + diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 648236d465e..6cd4a5b0c7b 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -50,59 +50,59 @@ describe(ChangeFiles.name, () => { }); }); - describe(ChangeFiles.validate.name, () => { - it('throws when there is a patch in a hotfix branch.', () => { + describe(ChangeFiles.validateAsync.name, () => { + it('throws when there is a patch in a hotfix branch.', async () => { const changeFile: string = `${__dirname}/leafChange/change1.json`; const changedPackages: string[] = ['d']; - expect(() => { - ChangeFiles.validate([changeFile], changedPackages, { + await expect( + ChangeFiles.validateAsync([changeFile], changedPackages, { hotfixChangeEnabled: true - } as RushConfiguration); - }).toThrow(Error); + } as RushConfiguration) + ).rejects.toThrow(Error); }); - it('allows a hotfix in a hotfix branch.', () => { + it('allows a hotfix in a hotfix branch.', async () => { const changeFile: string = `${__dirname}/multipleHotfixChanges/change1.json`; const changedPackages: string[] = ['a']; - ChangeFiles.validate([changeFile], changedPackages, { + await ChangeFiles.validateAsync([changeFile], changedPackages, { ...rushConfiguration, hotfixChangeEnabled: true } as RushConfiguration); }); - it('throws when there is any missing package.', () => { + it('throws when there is any missing package.', async () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a', 'b', 'c']; - expect(() => { - ChangeFiles.validate([changeFile], changedPackages, rushConfiguration); - }).toThrow(Error); + await expect( + ChangeFiles.validateAsync([changeFile], changedPackages, rushConfiguration) + ).rejects.toThrow(Error); }); - it('does not throw when there is no missing packages', () => { + it('does not throw when there is no missing packages', async () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a']; - expect(() => { - ChangeFiles.validate([changeFile], changedPackages, rushConfiguration); - }).not.toThrow(); + await ChangeFiles.validateAsync([changeFile], changedPackages, rushConfiguration); }); - it('throws when missing packages from categorized changes', () => { + it('throws when missing packages from categorized changes', async () => { const changeFileA: string = `${__dirname}/categorizedChanges/@ms/a/changeA.json`; const changeFileB: string = `${__dirname}/categorizedChanges/@ms/b/changeB.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; - expect(() => { - ChangeFiles.validate([changeFileA, changeFileB], changedPackages, rushConfiguration); - }).toThrow(Error); + await expect( + ChangeFiles.validateAsync([changeFileA, changeFileB], changedPackages, rushConfiguration) + ).rejects.toThrow(Error); }); - it('does not throw when no missing packages from categorized changes', () => { + it('does not throw when no missing packages from categorized changes', async () => { const changeFileA: string = `${__dirname}/categorizedChanges/@ms/a/changeA.json`; const changeFileB: string = `${__dirname}/categorizedChanges/@ms/b/changeB.json`; const changeFileC: string = `${__dirname}/categorizedChanges/changeC.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; - expect(() => { - ChangeFiles.validate([changeFileA, changeFileB, changeFileC], changedPackages, rushConfiguration); - }).not.toThrow(Error); + await ChangeFiles.validateAsync( + [changeFileA, changeFileB, changeFileC], + changedPackages, + rushConfiguration + ); }); describe('with strictChangefileValidation', () => { @@ -119,15 +119,15 @@ describe(ChangeFiles.name, () => { } as unknown as RushConfiguration; } - it('throws when change file references a nonexistent project', () => { + it('throws when change file references a nonexistent project', async () => { const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; strictConfig = createStrictConfig(() => undefined); - expect(() => { - ChangeFiles.validate([changeFile], ['nonexistent-package'], strictConfig); - }).toThrow(/does not exist in the Rush configuration/); + await expect( + ChangeFiles.validateAsync([changeFile], ['nonexistent-package'], strictConfig) + ).rejects.toThrow(/does not exist in the Rush configuration/); }); - it('throws when change file references a non-main lockstep project', () => { + it('throws when change file references a non-main lockstep project', async () => { const changeFile: string = `${__dirname}/strictValidation/nonMainLockstep.json`; strictConfig = createStrictConfig((name: string) => { if (name === 'lockstep-secondary') { @@ -142,12 +142,12 @@ describe(ChangeFiles.name, () => { } return undefined; }); - expect(() => { - ChangeFiles.validate([changeFile], ['lockstep-secondary'], strictConfig); - }).toThrow(/main project "lockstep-main"/); + await expect( + ChangeFiles.validateAsync([changeFile], ['lockstep-secondary'], strictConfig) + ).rejects.toThrow(/main project "lockstep-main"/); }); - it('does not throw when change file references the main lockstep project', () => { + it('does not throw when change file references the main lockstep project', async () => { const changeFile: string = `${__dirname}/strictValidation/mainLockstep.json`; strictConfig = createStrictConfig((name: string) => { if (name === 'lockstep-main') { @@ -162,12 +162,10 @@ describe(ChangeFiles.name, () => { } return undefined; }); - expect(() => { - ChangeFiles.validate([changeFile], ['lockstep-main'], strictConfig); - }).not.toThrow(); + await ChangeFiles.validateAsync([changeFile], ['lockstep-main'], strictConfig); }); - it('does not throw when change file references a lockstep project with no mainProject', () => { + it('does not throw when change file references a lockstep project with no mainProject', async () => { const changeFile: string = `${__dirname}/strictValidation/mainLockstep.json`; strictConfig = createStrictConfig((name: string) => { if (name === 'lockstep-main') { @@ -182,21 +180,17 @@ describe(ChangeFiles.name, () => { } return undefined; }); - expect(() => { - ChangeFiles.validate([changeFile], ['lockstep-main'], strictConfig); - }).not.toThrow(); + await ChangeFiles.validateAsync([changeFile], ['lockstep-main'], strictConfig); }); - it('does not throw when experiment is disabled', () => { + it('does not throw when experiment is disabled', async () => { const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; const config: RushConfiguration = { experimentsConfiguration: { configuration: { strictChangefileValidation: false } } as ExperimentsConfiguration } as unknown as RushConfiguration; - expect(() => { - ChangeFiles.validate([changeFile], ['nonexistent-package'], config); - }).not.toThrow(); + await ChangeFiles.validateAsync([changeFile], ['nonexistent-package'], config); }); }); }); From aeaf8251d4652b27e2c3bb0a5988ba4e8b7b903d Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 15:59:42 -0400 Subject: [PATCH 04/14] Add a --verify-all flag to rush change. --- .../RushCommandLine.test.ts.snap | 8 +++ .../rush-lib/src/cli/actions/ChangeAction.ts | 60 +++++++++++++++++-- .../CommandLineHelp.test.ts.snap | 13 +++- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/libraries/rush-lib/src/api/test/__snapshots__/RushCommandLine.test.ts.snap b/libraries/rush-lib/src/api/test/__snapshots__/RushCommandLine.test.ts.snap index c47899ac293..ed2fbd8721d 100644 --- a/libraries/rush-lib/src/api/test/__snapshots__/RushCommandLine.test.ts.snap +++ b/libraries/rush-lib/src/api/test/__snapshots__/RushCommandLine.test.ts.snap @@ -91,6 +91,14 @@ Object { "required": false, "shortName": "-v", }, + Object { + "description": "Validate all change files in the repository, not just those added in the current branch. Reports errors for change files that reference nonexistent projects or target non-main projects in a lockstepped version policy. Requires the \\"strictChangefileValidation\\" experiment to be enabled.", + "environmentVariable": undefined, + "kind": "Flag", + "longName": "--verify-all", + "required": false, + "shortName": undefined, + }, Object { "description": "Skips fetching the baseline branch before running \\"git diff\\" to detect changes.", "environmentVariable": undefined, diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index dabdd0bfd69..03a216ee0c9 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -38,6 +38,7 @@ const BULK_BUMP_TYPE_LONG_NAME: string = '--bump-type'; export class ChangeAction extends BaseRushAction { private readonly _git: Git; private readonly _verifyParameter: CommandLineFlagParameter; + private readonly _verifyAllParameter: CommandLineFlagParameter; private readonly _noFetchParameter: CommandLineFlagParameter; private readonly _targetBranchParameter: CommandLineStringParameter; private readonly _changeEmailParameter: CommandLineStringParameter; @@ -98,6 +99,14 @@ export class ChangeAction extends BaseRushAction { description: 'Verify the change file has been generated and that it is a valid JSON file' }); + this._verifyAllParameter = this.defineFlagParameter({ + parameterLongName: '--verify-all', + description: + 'Validate all change files in the repository, not just those added in the current branch. ' + + 'Reports errors for change files that reference nonexistent projects or target non-main projects ' + + 'in a lockstepped version policy. Requires the "strictChangefileValidation" experiment to be enabled.' + }); + this._noFetchParameter = this.defineFlagParameter({ parameterLongName: '--no-fetch', description: 'Skips fetching the baseline branch before running "git diff" to detect changes.' @@ -162,29 +171,64 @@ export class ChangeAction extends BaseRushAction { } public async runAsync(): Promise { + if (this._verifyAllParameter.value) { + const incompatibleParameters: ( + | CommandLineFlagParameter + | CommandLineStringParameter + | CommandLineChoiceParameter + )[] = [ + this._verifyParameter, + this._bulkChangeParameter, + this._bulkChangeMessageParameter, + this._bulkChangeBumpTypeParameter, + this._overwriteFlagParameter, + this._commitChangesFlagParameter + ]; + const errors: string[] = incompatibleParameters + .filter((parameter) => parameter.value) + .map( + (parameter) => + `The ${parameter.longName} parameter cannot be provided with the ` + + `${this._verifyAllParameter.longName} parameter` + ); + if (errors.length > 0) { + errors.forEach((error) => { + this.terminal.writeErrorLine(error); + }); + throw new AlreadyReportedError(); + } + + await this._validateAllChangeFilesAsync(); + return; + } + const targetBranch: string = await this._getTargetBranchAsync(); // eslint-disable-next-line no-console console.log(`The target branch is ${targetBranch}`); if (this._verifyParameter.value) { - const errors: string[] = [ + const incompatibleParameters: ( + | CommandLineFlagParameter + | CommandLineStringParameter + | CommandLineChoiceParameter + )[] = [ this._bulkChangeParameter, this._bulkChangeMessageParameter, this._bulkChangeBumpTypeParameter, this._overwriteFlagParameter, this._commitChangesFlagParameter - ] + ]; + const errors: string[] = incompatibleParameters .map((parameter) => { return parameter.value - ? `The {${this._bulkChangeParameter.longName} parameter cannot be provided with the ` + + ? `The ${parameter.longName} parameter cannot be provided with the ` + `${this._verifyParameter.longName} parameter` : ''; }) .filter((error) => error !== ''); if (errors.length > 0) { errors.forEach((error) => { - // eslint-disable-next-line no-console - console.error(error); + this.terminal.writeErrorLine(error); }); throw new AlreadyReportedError(); } @@ -390,6 +434,12 @@ export class ChangeAction extends BaseRushAction { await ChangeFiles.validateAsync(files, changedPackages, this.rushConfiguration); } + private async _validateAllChangeFilesAsync(): Promise { + const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); + const allChangeFiles: string[] = await changeFiles.getFilesAsync(); + await ChangeFiles.validateAsync(allChangeFiles, [], this.rushConfiguration); + } + private async _getChangeFilesAsync(): Promise { const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); const relativeChangesFolder: string = path.relative(repoRoot, this.rushConfiguration.changesFolder); diff --git a/libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap b/libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap index df9d2c0a674..e7752e174c5 100644 --- a/libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap +++ b/libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap @@ -342,9 +342,10 @@ Optional arguments: `; exports[`CommandLineHelp prints the help for each action: change 1`] = ` -"usage: rush change [-h] [-v] [--no-fetch] [-b BRANCH] [--overwrite] [-c] - [--commit-message COMMIT_MESSAGE] [--email EMAIL] [--bulk] - [--message MESSAGE] [--bump-type {major,minor,patch,none}] +"usage: rush change [-h] [-v] [--verify-all] [--no-fetch] [-b BRANCH] + [--overwrite] [-c] [--commit-message COMMIT_MESSAGE] + [--email EMAIL] [--bulk] [--message MESSAGE] + [--bump-type {major,minor,patch,none}] Asks a series of questions and then generates a -.json @@ -370,6 +371,12 @@ Optional arguments: -h, --help Show this help message and exit. -v, --verify Verify the change file has been generated and that it is a valid JSON file + --verify-all Validate all change files in the repository, not just + those added in the current branch. Reports errors for + change files that reference nonexistent projects or + target non-main projects in a lockstepped version + policy. Requires the \\"strictChangefileValidation\\" + experiment to be enabled. --no-fetch Skips fetching the baseline branch before running \\"git diff\\" to detect changes. -b BRANCH, --target-branch BRANCH From 470dff481f86a1469f76434c8a3f47e7555b8652 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 16:05:20 -0400 Subject: [PATCH 05/14] Improve verification logging. --- libraries/rush-lib/src/logic/ChangeFiles.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 40158ca3707..32b22fb2e74 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/libraries/rush-lib/src/logic/ChangeFiles.ts @@ -35,7 +35,8 @@ export class ChangeFiles { ): Promise { const schema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); - const projectsWithChangeDescriptions: Set = new Set(); + const projectsWithChangeDescriptions: Set = new Set(); + const changefilesByProjectName: Map = new Map(); await Async.forEachAsync( newChangeFilePaths, async (filePath) => { @@ -60,6 +61,13 @@ export class ChangeFiles { if (changeFile && changeFile.changes) { for (const { packageName } of changeFile.changes) { projectsWithChangeDescriptions.add(packageName); + let files: string[] | undefined = changefilesByProjectName.get(packageName); + if (!files) { + files = []; + changefilesByProjectName.set(packageName, files); + } + + files.push(filePath); } } else { throw new Error(`Invalid change file: ${filePath}`); @@ -72,11 +80,13 @@ export class ChangeFiles { const errors: string[] = []; for (const packageName of projectsWithChangeDescriptions) { + const affectedFiles: string[] = changefilesByProjectName.get(packageName) ?? []; + const fileList: string = affectedFiles.map((f) => ` - ${f}`).join('\n'); const project: RushConfigurationProject | undefined = rushConfiguration.getProjectByName(packageName); if (!project) { errors.push( - `Change file(s) reference a project "${packageName}" that does not exist in the Rush configuration.` + `Change file(s) reference a project "${packageName}" that does not exist in the Rush configuration:\n${fileList}` ); continue; } @@ -87,7 +97,7 @@ export class ChangeFiles { errors.push( `Change file(s) reference the project "${packageName}" which belongs to lockstepped ` + `version policy "${project.versionPolicy.policyName}". Change files should be ` + - `created for the policy's main project "${mainProject}" instead.` + `created for the policy's main project "${mainProject}" instead:\n${fileList}` ); } } From 2e61d9f711f34f1fad93c4a3d0382d192b3d5d7b Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 16:19:42 -0400 Subject: [PATCH 06/14] Snapshot test output. --- .../rush-lib/src/cli/actions/ChangeAction.ts | 41 +++---- .../rush-lib/src/cli/actions/PublishAction.ts | 2 +- .../rush-lib/src/cli/actions/VersionAction.ts | 1 + libraries/rush-lib/src/logic/ChangeFiles.ts | 34 ++++-- libraries/rush-lib/src/logic/ChangeManager.ts | 5 +- .../rush-lib/src/logic/VersionManager.ts | 4 +- .../src/logic/test/ChangeFiles.test.ts | 58 ++++++--- .../src/logic/test/VersionManager.test.ts | 27 +++- .../__snapshots__/ChangeFiles.test.ts.snap | 115 ++++++++++++++++++ .../__snapshots__/VersionManager.test.ts.snap | 19 +++ 10 files changed, 243 insertions(+), 63 deletions(-) create mode 100644 libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap create mode 100644 libraries/rush-lib/src/logic/test/__snapshots__/VersionManager.test.ts.snap diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 03a216ee0c9..7563dad7c87 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -203,8 +203,7 @@ export class ChangeAction extends BaseRushAction { } const targetBranch: string = await this._getTargetBranchAsync(); - // eslint-disable-next-line no-console - console.log(`The target branch is ${targetBranch}`); + this.terminal.writeLine(`The target branch is ${targetBranch}`); if (this._verifyParameter.value) { const incompatibleParameters: ( @@ -305,8 +304,7 @@ export class ChangeAction extends BaseRushAction { if (errors.length > 0) { for (const error of errors) { - // eslint-disable-next-line no-console - console.error(error); + this.terminal.writeErrorLine(error); } throw new AlreadyReportedError(); @@ -320,6 +318,7 @@ export class ChangeAction extends BaseRushAction { interactiveMode = true; const existingChangeComments: Map = ChangeFiles.getChangeComments( + this.terminal, await this._getChangeFilesAsync() ); changeFileData = await this._promptForChangeFileDataAsync( @@ -431,13 +430,13 @@ export class ChangeAction extends BaseRushAction { private async _validateChangeFileAsync(changedPackages: string[]): Promise { const files: string[] = await this._getChangeFilesAsync(); - await ChangeFiles.validateAsync(files, changedPackages, this.rushConfiguration); + await ChangeFiles.validateAsync(this.terminal, files, changedPackages, this.rushConfiguration); } private async _validateAllChangeFilesAsync(): Promise { const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); const allChangeFiles: string[] = await changeFiles.getFilesAsync(); - await ChangeFiles.validateAsync(allChangeFiles, [], this.rushConfiguration); + await ChangeFiles.validateAsync(this.terminal, allChangeFiles, [], this.rushConfiguration); } private async _getChangeFilesAsync(): Promise { @@ -502,15 +501,12 @@ export class ChangeAction extends BaseRushAction { packageName: string, existingChangeComments: Map ): Promise { - // eslint-disable-next-line no-console - console.log(`\n${packageName}`); + this.terminal.writeLine(`\n${packageName}`); const comments: string[] | undefined = existingChangeComments.get(packageName); if (comments) { - // eslint-disable-next-line no-console - console.log(`Found existing comments:`); + this.terminal.writeLine(`Found existing comments:`); comments.forEach((comment) => { - // eslint-disable-next-line no-console - console.log(` > ${comment}`); + this.terminal.writeLine(` > ${comment}`); }); const { appendComment }: { appendComment: 'skip' | 'append' } = await promptModule({ name: 'appendComment', @@ -645,8 +641,7 @@ export class ChangeAction extends BaseRushAction { .toString() .replace(/(\r\n|\n|\r)/gm, ''); } catch (err) { - // eslint-disable-next-line no-console - console.log('There was an issue detecting your Git email...'); + this.terminal.writeLine('There was an issue detecting your Git email...'); return undefined; } } @@ -696,8 +691,7 @@ export class ChangeAction extends BaseRushAction { try { const hasUnstagedChanges: boolean = await this._git.hasUnstagedChangesAsync(); if (hasUnstagedChanges) { - // eslint-disable-next-line no-console - console.log( + this.terminal.writeLine( '\n' + Colorize.yellow( 'Warning: You have unstaged changes, which do not trigger prompting for change ' + @@ -706,8 +700,7 @@ export class ChangeAction extends BaseRushAction { ); } } catch (error) { - // eslint-disable-next-line no-console - console.log(`An error occurred when detecting unstaged changes: ${error}`); + this.terminal.writeLine(`An error occurred when detecting unstaged changes: ${error}`); } } @@ -776,8 +769,7 @@ export class ChangeAction extends BaseRushAction { if (overwrite) { return true; } else { - // eslint-disable-next-line no-console - console.log(`Not overwriting ${filePath}`); + this.terminal.writeLine(`Not overwriting ${filePath}`); return false; } } @@ -788,16 +780,13 @@ export class ChangeAction extends BaseRushAction { private _writeFile(fileName: string, output: string, isOverwrite: boolean): void { FileSystem.writeFile(fileName, output, { ensureFolderExists: true }); if (isOverwrite) { - // eslint-disable-next-line no-console - console.log(`Overwrote file: ${fileName}`); + this.terminal.writeLine(`Overwrote file: ${fileName}`); } else { - // eslint-disable-next-line no-console - console.log(`Created file: ${fileName}`); + this.terminal.writeLine(`Created file: ${fileName}`); } } private _logNoChangeFileRequired(): void { - // eslint-disable-next-line no-console - console.log('No changes were detected to relevant packages on this branch. Nothing to do.'); + this.terminal.writeLine('No changes were detected to relevant packages on this branch. Nothing to do.'); } } diff --git a/libraries/rush-lib/src/cli/actions/PublishAction.ts b/libraries/rush-lib/src/cli/actions/PublishAction.ts index cb05204508f..328e1b9cce2 100644 --- a/libraries/rush-lib/src/cli/actions/PublishAction.ts +++ b/libraries/rush-lib/src/cli/actions/PublishAction.ts @@ -298,7 +298,7 @@ export class PublishAction extends BaseRushAction { // Make changes to package.json and change logs. changeManager.apply(this._apply.value); - await changeManager.updateChangelogAsync(this._apply.value); + await changeManager.updateChangelogAsync(this.terminal, this._apply.value); this._setDependenciesBeforeCommit(); diff --git a/libraries/rush-lib/src/cli/actions/VersionAction.ts b/libraries/rush-lib/src/cli/actions/VersionAction.ts index de854535680..6eff2c1176d 100644 --- a/libraries/rush-lib/src/cli/actions/VersionAction.ts +++ b/libraries/rush-lib/src/cli/actions/VersionAction.ts @@ -135,6 +135,7 @@ export class VersionAction extends BaseRushAction { } else if (this._bumpVersion.value) { const tempBranch: string = 'version/bump-' + new Date().getTime(); await versionManager.bumpAsync( + this.terminal, this._versionPolicy.value, this._overwriteBump.value ? Enum.getValueByKey(BumpType, this._overwriteBump.value) : undefined, this._prereleaseIdentifier.value, diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 32b22fb2e74..29e7414ab5f 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/libraries/rush-lib/src/logic/ChangeFiles.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import { Async, FileSystem, JsonFile, JsonSchema } from '@rushstack/node-core-library'; +import type { ITerminal } from '@rushstack/terminal'; import type { IChangeInfo } from '../api/ChangeManagement'; import type { IChangelog } from '../api/Changelog'; @@ -29,6 +30,7 @@ export class ChangeFiles { * Validate if the newly added change files match the changed packages. */ public static async validateAsync( + terminal: ITerminal, newChangeFilePaths: string[], changedPackages: string[], rushConfiguration: RushConfiguration @@ -40,8 +42,7 @@ export class ChangeFiles { await Async.forEachAsync( newChangeFilePaths, async (filePath) => { - // eslint-disable-next-line no-console - console.log(`Found change file: ${filePath}`); + terminal.writeLine(`Found change file: ${filePath}`); const changeFile: IChangeInfo = JsonFile.loadAndValidate(filePath, schema); @@ -129,12 +130,11 @@ export class ChangeFiles { } } - public static getChangeComments(newChangeFilePaths: string[]): Map { + public static getChangeComments(terminal: ITerminal, newChangeFilePaths: string[]): Map { const changes: Map = new Map(); newChangeFilePaths.forEach((filePath) => { - // eslint-disable-next-line no-console - console.log(`Found change file: ${filePath}`); + terminal.writeLine(`Found change file: ${filePath}`); const changeRequest: IChangeInfo = JsonFile.load(filePath); if (changeRequest && changeRequest.changes) { changeRequest.changes!.forEach((change) => { @@ -174,7 +174,11 @@ export class ChangeFiles { /** * Delete all change files */ - public async deleteAllAsync(shouldDelete: boolean, updatedChangelogs?: IChangelog[]): Promise { + public async deleteAllAsync( + terminal: ITerminal, + shouldDelete: boolean, + updatedChangelogs?: IChangelog[] + ): Promise { if (updatedChangelogs) { // Skip changes files if the package's change log is not updated. const packagesToInclude: Set = new Set(); @@ -203,24 +207,28 @@ export class ChangeFiles { { concurrency: 5 } ); - return await this._deleteFilesAsync(filesToDelete, shouldDelete); + return await this._deleteFilesAsync(terminal, filesToDelete, shouldDelete); } else { // Delete all change files. const files: string[] = await this.getFilesAsync(); - return await this._deleteFilesAsync(files, shouldDelete); + return await this._deleteFilesAsync(terminal, files, shouldDelete); } } - private async _deleteFilesAsync(files: string[], shouldDelete: boolean): Promise { + private async _deleteFilesAsync( + terminal: ITerminal, + files: string[], + shouldDelete: boolean + ): Promise { if (files.length) { - // eslint-disable-next-line no-console - console.log(`\n* ${shouldDelete ? 'DELETING:' : 'DRYRUN: Deleting'} ${files.length} change file(s).`); + terminal.writeLine( + `\n* ${shouldDelete ? 'DELETING:' : 'DRYRUN: Deleting'} ${files.length} change file(s).` + ); await Async.forEachAsync( files, async (filePath) => { - // eslint-disable-next-line no-console - console.log(` - ${filePath}`); + terminal.writeLine(` - ${filePath}`); if (shouldDelete) { await FileSystem.deleteFileAsync(filePath); } diff --git a/libraries/rush-lib/src/logic/ChangeManager.ts b/libraries/rush-lib/src/logic/ChangeManager.ts index 254ae58bb50..5e1f2fac01c 100644 --- a/libraries/rush-lib/src/logic/ChangeManager.ts +++ b/libraries/rush-lib/src/logic/ChangeManager.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import type { IPackageJson } from '@rushstack/node-core-library'; +import type { ITerminal } from '@rushstack/terminal'; import type { IChangeInfo } from '../api/ChangeManagement'; import type { IChangelog } from '../api/Changelog'; @@ -117,7 +118,7 @@ export class ChangeManager { return updatedPackages; } - public async updateChangelogAsync(shouldCommit: boolean): Promise { + public async updateChangelogAsync(terminal: ITerminal, shouldCommit: boolean): Promise { // Do not update changelog or delete the change files for prerelease. // Save them for the official release. if (!this._prereleaseToken.hasValue) { @@ -130,7 +131,7 @@ export class ChangeManager { ); // Remove the change request files only if "-a" was provided. - await this._changeFiles.deleteAllAsync(shouldCommit, updatedChangelogs); + await this._changeFiles.deleteAllAsync(terminal, shouldCommit, updatedChangelogs); } } } diff --git a/libraries/rush-lib/src/logic/VersionManager.ts b/libraries/rush-lib/src/logic/VersionManager.ts index 7a2f57840e9..ec5095d3c8e 100644 --- a/libraries/rush-lib/src/logic/VersionManager.ts +++ b/libraries/rush-lib/src/logic/VersionManager.ts @@ -6,6 +6,7 @@ import * as path from 'node:path'; import * as semver from 'semver'; import { type IPackageJson, JsonFile, FileConstants } from '@rushstack/node-core-library'; +import type { ITerminal } from '@rushstack/terminal'; import { type VersionPolicy, type BumpType, LockStepVersionPolicy } from '../api/VersionPolicy'; import { ChangeFile } from '../api/ChangeFile'; @@ -65,6 +66,7 @@ export class VersionManager { * @param shouldCommit - whether the changes will be written to disk */ public async bumpAsync( + terminal: ITerminal, lockStepVersionPolicyName?: string, bumpType?: BumpType, identifier?: string, @@ -94,7 +96,7 @@ export class VersionManager { changeManager.apply(!!shouldCommit)!.forEach((packageJson) => { this.updatedProjects.set(packageJson.name, packageJson); }); - await changeManager.updateChangelogAsync(!!shouldCommit); + await changeManager.updateChangelogAsync(terminal, !!shouldCommit); } // Refresh rush configuration again, since we've further modified the package.json files diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 6cd4a5b0c7b..630d87a45ba 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -9,16 +9,31 @@ import type { RushConfiguration } from '../../api/RushConfiguration'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { VersionPolicyDefinitionName } from '../../api/VersionPolicy'; import type { ExperimentsConfiguration } from '../../api/ExperimentsConfiguration'; +import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; describe(ChangeFiles.name, () => { let rushConfiguration: RushConfiguration; + let terminalProvider: StringBufferTerminalProvider; + let terminal: Terminal; + beforeEach(() => { rushConfiguration = { experimentsConfiguration: { configuration: {} } } as RushConfiguration; + + terminalProvider = new StringBufferTerminalProvider(); + terminal = new Terminal(terminalProvider); + }); + + afterEach(() => { + expect( + terminalProvider + .getAllOutputAsChunks({ asLines: true }) + .map((chunk) => chunk.replace(__dirname, '')) + ).toMatchSnapshot(); }); describe(ChangeFiles.prototype.getFilesAsync.name, () => { @@ -55,7 +70,7 @@ describe(ChangeFiles.name, () => { const changeFile: string = `${__dirname}/leafChange/change1.json`; const changedPackages: string[] = ['d']; await expect( - ChangeFiles.validateAsync([changeFile], changedPackages, { + ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, { hotfixChangeEnabled: true } as RushConfiguration) ).rejects.toThrow(Error); @@ -64,7 +79,7 @@ describe(ChangeFiles.name, () => { it('allows a hotfix in a hotfix branch.', async () => { const changeFile: string = `${__dirname}/multipleHotfixChanges/change1.json`; const changedPackages: string[] = ['a']; - await ChangeFiles.validateAsync([changeFile], changedPackages, { + await ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, { ...rushConfiguration, hotfixChangeEnabled: true } as RushConfiguration); @@ -74,14 +89,14 @@ describe(ChangeFiles.name, () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a', 'b', 'c']; await expect( - ChangeFiles.validateAsync([changeFile], changedPackages, rushConfiguration) + ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, rushConfiguration) ).rejects.toThrow(Error); }); it('does not throw when there is no missing packages', async () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a']; - await ChangeFiles.validateAsync([changeFile], changedPackages, rushConfiguration); + await ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, rushConfiguration); }); it('throws when missing packages from categorized changes', async () => { @@ -89,7 +104,7 @@ describe(ChangeFiles.name, () => { const changeFileB: string = `${__dirname}/categorizedChanges/@ms/b/changeB.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; await expect( - ChangeFiles.validateAsync([changeFileA, changeFileB], changedPackages, rushConfiguration) + ChangeFiles.validateAsync(terminal, [changeFileA, changeFileB], changedPackages, rushConfiguration) ).rejects.toThrow(Error); }); @@ -99,6 +114,7 @@ describe(ChangeFiles.name, () => { const changeFileC: string = `${__dirname}/categorizedChanges/changeC.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; await ChangeFiles.validateAsync( + terminal, [changeFileA, changeFileB, changeFileC], changedPackages, rushConfiguration @@ -122,9 +138,13 @@ describe(ChangeFiles.name, () => { it('throws when change file references a nonexistent project', async () => { const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; strictConfig = createStrictConfig(() => undefined); - await expect( - ChangeFiles.validateAsync([changeFile], ['nonexistent-package'], strictConfig) - ).rejects.toThrow(/does not exist in the Rush configuration/); + try { + await ChangeFiles.validateAsync(terminal, [changeFile], ['nonexistent-package'], strictConfig); + fail('Expected validateAsync to throw'); + } catch (error) { + const normalizedMessage: string = error.message.replace(__dirname, ''); + expect(normalizedMessage).toMatchSnapshot(); + } }); it('throws when change file references a non-main lockstep project', async () => { @@ -142,9 +162,13 @@ describe(ChangeFiles.name, () => { } return undefined; }); - await expect( - ChangeFiles.validateAsync([changeFile], ['lockstep-secondary'], strictConfig) - ).rejects.toThrow(/main project "lockstep-main"/); + try { + await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-secondary'], strictConfig); + fail('Expected validateAsync to throw'); + } catch (error) { + const normalizedMessage: string = error.message.replace(__dirname, ''); + expect(normalizedMessage).toMatchSnapshot(); + } }); it('does not throw when change file references the main lockstep project', async () => { @@ -162,7 +186,7 @@ describe(ChangeFiles.name, () => { } return undefined; }); - await ChangeFiles.validateAsync([changeFile], ['lockstep-main'], strictConfig); + await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-main'], strictConfig); }); it('does not throw when change file references a lockstep project with no mainProject', async () => { @@ -180,7 +204,7 @@ describe(ChangeFiles.name, () => { } return undefined; }); - await ChangeFiles.validateAsync([changeFile], ['lockstep-main'], strictConfig); + await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-main'], strictConfig); }); it('does not throw when experiment is disabled', async () => { @@ -190,7 +214,7 @@ describe(ChangeFiles.name, () => { configuration: { strictChangefileValidation: false } } as ExperimentsConfiguration } as unknown as RushConfiguration; - await ChangeFiles.validateAsync([changeFile], ['nonexistent-package'], config); + await ChangeFiles.validateAsync(terminal, [changeFile], ['nonexistent-package'], config); }); }); }); @@ -199,7 +223,7 @@ describe(ChangeFiles.name, () => { it('delete all files when there are no prerelease packages', async () => { const changesPath: string = `${__dirname}/multipleChangeFiles`; const changeFiles: ChangeFiles = new ChangeFiles(changesPath); - expect(await changeFiles.deleteAllAsync(false)).toEqual(3); + expect(await changeFiles.deleteAllAsync(terminal, false)).toEqual(3); }); it('does not delete change files for package whose change logs do not get updated. ', async () => { @@ -215,13 +239,13 @@ describe(ChangeFiles.name, () => { entries: [] } ]; - expect(await changeFiles.deleteAllAsync(false, updatedChangelogs)).toEqual(2); + expect(await changeFiles.deleteAllAsync(terminal, false, updatedChangelogs)).toEqual(2); }); it('delete all files when there are hotfixes', async () => { const changesPath: string = `${__dirname}/multipleHotfixChanges`; const changeFiles: ChangeFiles = new ChangeFiles(changesPath); - expect(await changeFiles.deleteAllAsync(false)).toEqual(3); + expect(await changeFiles.deleteAllAsync(terminal, false)).toEqual(3); }); }); }); diff --git a/libraries/rush-lib/src/logic/test/VersionManager.test.ts b/libraries/rush-lib/src/logic/test/VersionManager.test.ts index fe8ec8b11a1..259ec579a05 100644 --- a/libraries/rush-lib/src/logic/test/VersionManager.test.ts +++ b/libraries/rush-lib/src/logic/test/VersionManager.test.ts @@ -8,6 +8,7 @@ import type { ChangeFile } from '../../api/ChangeFile'; import { ChangeType, type IChangeInfo } from '../../api/ChangeManagement'; import { RushConfiguration } from '../../api/RushConfiguration'; import { VersionManager } from '../VersionManager'; +import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; function _getChanges(changeFiles: Map, packageName: string): IChangeInfo[] | undefined { const changeFile: ChangeFile | undefined = changeFiles.get(packageName); @@ -22,12 +23,22 @@ describe(VersionManager.name, () => { const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFile); let versionManager: VersionManager; + let terminalProvider: StringBufferTerminalProvider; + let terminal: Terminal; + beforeEach(() => { versionManager = new VersionManager( rushConfiguration, 'test@microsoft.com', rushConfiguration.versionPolicyConfiguration ); + + terminalProvider = new StringBufferTerminalProvider(); + terminal = new Terminal(terminalProvider); + }); + + afterEach(() => { + expect(terminalProvider.getAllOutputAsChunks({ asLines: true })).toMatchSnapshot(); }); /* eslint-disable dot-notation */ @@ -88,7 +99,7 @@ describe(VersionManager.name, () => { describe(VersionManager.prototype.bumpAsync.name, () => { it('bumps a lockStepPolicy to prerelease version', async () => { - await versionManager.bumpAsync('testPolicy1', BumpType.prerelease, 'dev', false); + await versionManager.bumpAsync(terminal, 'testPolicy1', BumpType.prerelease, 'dev', false); const updatedPackages: Map = versionManager.updatedProjects; const changeFiles: Map = versionManager.changeFiles; @@ -102,7 +113,7 @@ describe(VersionManager.name, () => { }); it('bumps a lockStepPolicy without bumpType to prerelease version', async () => { - await versionManager.bumpAsync('lockStepWithoutNextBump', BumpType.prerelease, 'dev', false); + await versionManager.bumpAsync(terminal, 'lockStepWithoutNextBump', BumpType.prerelease, 'dev', false); const updatedPackages: Map = versionManager.updatedProjects; const changeFiles: Map = versionManager.changeFiles; @@ -120,12 +131,22 @@ describe(`${VersionManager.name} (workspace)`, () => { const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFile); let versionManager: VersionManager; + let terminalProvider: StringBufferTerminalProvider; + let terminal: Terminal; + beforeEach(() => { versionManager = new VersionManager( rushConfiguration, 'test@microsoft.com', rushConfiguration.versionPolicyConfiguration ); + + terminalProvider = new StringBufferTerminalProvider(); + terminal = new Terminal(terminalProvider); + }); + + afterEach(() => { + expect(terminalProvider.getAllOutputAsChunks({ asLines: true })).toMatchSnapshot(); }); /* eslint-disable dot-notation */ @@ -186,7 +207,7 @@ describe(`${VersionManager.name} (workspace)`, () => { describe(VersionManager.prototype.bumpAsync.name, () => { it('bumps to prerelease version', async () => { - await versionManager.bumpAsync('testPolicy1', BumpType.prerelease, 'dev', false); + await versionManager.bumpAsync(terminal, 'testPolicy1', BumpType.prerelease, 'dev', false); const updatedPackages: Map = versionManager.updatedProjects; const expectedVersion: string = '10.10.1-dev.0'; diff --git a/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap b/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap new file mode 100644 index 00000000000..5d08f60c3e6 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap @@ -0,0 +1,115 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ChangeFiles deleteAllAsync delete all files when there are hotfixes 1`] = ` +Array [ + "[ log] [n]", + "[ log] * DRYRUN: Deleting 3 change file(s).[n]", + "[ log] - /multipleHotfixChanges/change1.json[n]", + "[ log] - /multipleHotfixChanges/change2.json[n]", + "[ log] - /multipleHotfixChanges/change3.json[n]", +] +`; + +exports[`ChangeFiles deleteAllAsync delete all files when there are no prerelease packages 1`] = ` +Array [ + "[ log] [n]", + "[ log] * DRYRUN: Deleting 3 change file(s).[n]", + "[ log] - /multipleChangeFiles/a.json[n]", + "[ log] - /multipleChangeFiles/b.json[n]", + "[ log] - /multipleChangeFiles/c.json[n]", +] +`; + +exports[`ChangeFiles deleteAllAsync does not delete change files for package whose change logs do not get updated. 1`] = ` +Array [ + "[ log] [n]", + "[ log] * DRYRUN: Deleting 2 change file(s).[n]", + "[ log] - /multipleChangeFiles/a.json[n]", + "[ log] - /multipleChangeFiles/b.json[n]", +] +`; + +exports[`ChangeFiles getFilesAsync returns correctly when change files are categorized 1`] = `Array []`; + +exports[`ChangeFiles getFilesAsync returns correctly when there is one change file 1`] = `Array []`; + +exports[`ChangeFiles getFilesAsync returns empty array when no change files 1`] = `Array []`; + +exports[`ChangeFiles validateAsync allows a hotfix in a hotfix branch. 1`] = ` +Array [ + "[ log] Found change file: /multipleHotfixChanges/change1.json[n]", +] +`; + +exports[`ChangeFiles validateAsync does not throw when no missing packages from categorized changes 1`] = ` +Array [ + "[ log] Found change file: /categorizedChanges/@ms/a/changeA.json[n]", + "[ log] Found change file: /categorizedChanges/@ms/b/changeB.json[n]", + "[ log] Found change file: /categorizedChanges/changeC.json[n]", +] +`; + +exports[`ChangeFiles validateAsync does not throw when there is no missing packages 1`] = ` +Array [ + "[ log] Found change file: /verifyChanges/changes.json[n]", +] +`; + +exports[`ChangeFiles validateAsync throws when missing packages from categorized changes 1`] = ` +Array [ + "[ log] Found change file: /categorizedChanges/@ms/a/changeA.json[n]", + "[ log] Found change file: /categorizedChanges/@ms/b/changeB.json[n]", +] +`; + +exports[`ChangeFiles validateAsync throws when there is a patch in a hotfix branch. 1`] = ` +Array [ + "[ log] Found change file: /leafChange/change1.json[n]", +] +`; + +exports[`ChangeFiles validateAsync throws when there is any missing package. 1`] = ` +Array [ + "[ log] Found change file: /verifyChanges/changes.json[n]", +] +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation does not throw when change file references a lockstep project with no mainProject 1`] = ` +Array [ + "[ log] Found change file: /strictValidation/mainLockstep.json[n]", +] +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation does not throw when change file references the main lockstep project 1`] = ` +Array [ + "[ log] Found change file: /strictValidation/mainLockstep.json[n]", +] +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation does not throw when experiment is disabled 1`] = ` +Array [ + "[ log] Found change file: /strictValidation/nonexistentProject.json[n]", +] +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation throws when change file references a non-main lockstep project 1`] = ` +"Change file(s) reference the project \\"lockstep-secondary\\" which belongs to lockstepped version policy \\"myLockstep\\". Change files should be created for the policy's main project \\"lockstep-main\\" instead: + - /strictValidation/nonMainLockstep.json" +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation throws when change file references a non-main lockstep project 2`] = ` +Array [ + "[ log] Found change file: /strictValidation/nonMainLockstep.json[n]", +] +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation throws when change file references a nonexistent project 1`] = ` +"Change file(s) reference a project \\"nonexistent-package\\" that does not exist in the Rush configuration: + - /strictValidation/nonexistentProject.json" +`; + +exports[`ChangeFiles validateAsync with strictChangefileValidation throws when change file references a nonexistent project 2`] = ` +Array [ + "[ log] Found change file: /strictValidation/nonexistentProject.json[n]", +] +`; diff --git a/libraries/rush-lib/src/logic/test/__snapshots__/VersionManager.test.ts.snap b/libraries/rush-lib/src/logic/test/__snapshots__/VersionManager.test.ts.snap new file mode 100644 index 00000000000..734e8fe51c6 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/__snapshots__/VersionManager.test.ts.snap @@ -0,0 +1,19 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`VersionManager (workspace) bumpAsync bumps to prerelease version 1`] = `Array []`; + +exports[`VersionManager (workspace) ensure does not change packageJson if not needed by individual version policy 1`] = `Array []`; + +exports[`VersionManager (workspace) ensure fixes lock step versions 1`] = `Array []`; + +exports[`VersionManager (workspace) ensure fixes major version for individual version policy 1`] = `Array []`; + +exports[`VersionManager bumpAsync bumps a lockStepPolicy to prerelease version 1`] = `Array []`; + +exports[`VersionManager bumpAsync bumps a lockStepPolicy without bumpType to prerelease version 1`] = `Array []`; + +exports[`VersionManager ensure does not change packageJson if not needed by individual version policy 1`] = `Array []`; + +exports[`VersionManager ensure fixes lock step versions 1`] = `Array []`; + +exports[`VersionManager ensure fixes major version for individual version policy 1`] = `Array []`; From a018f18fba11cb964aa40a33489b44b4b88f29cf Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 16:21:06 -0400 Subject: [PATCH 07/14] Rush change. --- ...strict-changefile-validation_2026-03-21-20-19.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/@microsoft/rush-lib/strict-changefile-validation_2026-03-21-20-19.json diff --git a/common/changes/@microsoft/rush-lib/strict-changefile-validation_2026-03-21-20-19.json b/common/changes/@microsoft/rush-lib/strict-changefile-validation_2026-03-21-20-19.json new file mode 100644 index 00000000000..2261b518ddc --- /dev/null +++ b/common/changes/@microsoft/rush-lib/strict-changefile-validation_2026-03-21-20-19.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add \"strictChangefileValidation\" experiment and \"--verify-all\" flag for \"rush change\". When the experiment is enabled, \"rush change --verify\" and \"rush change --verify-all\" will report errors if change files reference nonexistent projects or target non-main projects in a lockstepped version policy.", + "type": "none" + } + ], + "packageName": "@microsoft/rush", + "email": "iclanton@users.noreply.github.com" +} From aecb3a44e22e7b7c1c5d4af40830885a650976f3 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 16:22:17 -0400 Subject: [PATCH 08/14] fixup! Add a --verify-all flag to rush change. --- libraries/rush-lib/src/cli/actions/ChangeAction.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 7563dad7c87..c8ff01ac4c0 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -434,6 +434,13 @@ export class ChangeAction extends BaseRushAction { } private async _validateAllChangeFilesAsync(): Promise { + if (!this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { + throw new Error( + `The ${this._verifyAllParameter.longName} parameter requires the ` + + '"strictChangefileValidation" experiment to be enabled.' + ); + } + const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); const allChangeFiles: string[] = await changeFiles.getFilesAsync(); await ChangeFiles.validateAsync(this.terminal, allChangeFiles, [], this.rushConfiguration); From d8fda40da5519ba35551be8ac35b87fd4138abc4 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 21 Mar 2026 17:54:20 -0400 Subject: [PATCH 09/14] fixup! Snapshot test output. --- libraries/rush-lib/src/logic/test/ChangeFiles.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 630d87a45ba..3167f7fe4be 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -11,6 +11,8 @@ import { VersionPolicyDefinitionName } from '../../api/VersionPolicy'; import type { ExperimentsConfiguration } from '../../api/ExperimentsConfiguration'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; +const FORWARD_SLASH_DIRNAME: string = Path.convertToSlashes(__dirname); + describe(ChangeFiles.name, () => { let rushConfiguration: RushConfiguration; @@ -32,7 +34,7 @@ describe(ChangeFiles.name, () => { expect( terminalProvider .getAllOutputAsChunks({ asLines: true }) - .map((chunk) => chunk.replace(__dirname, '')) + .map((chunk) => chunk.replace(FORWARD_SLASH_DIRNAME, '')) ).toMatchSnapshot(); }); @@ -142,7 +144,7 @@ describe(ChangeFiles.name, () => { await ChangeFiles.validateAsync(terminal, [changeFile], ['nonexistent-package'], strictConfig); fail('Expected validateAsync to throw'); } catch (error) { - const normalizedMessage: string = error.message.replace(__dirname, ''); + const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); expect(normalizedMessage).toMatchSnapshot(); } }); @@ -166,7 +168,7 @@ describe(ChangeFiles.name, () => { await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-secondary'], strictConfig); fail('Expected validateAsync to throw'); } catch (error) { - const normalizedMessage: string = error.message.replace(__dirname, ''); + const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); expect(normalizedMessage).toMatchSnapshot(); } }); From d8159aae7635d644fbc4c64dfeb6f82f65f309d5 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 22 Mar 2026 03:37:24 -0400 Subject: [PATCH 10/14] Copilot WIP --- .../rush-lib/src/cli/actions/ChangeAction.ts | 80 ++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index c8ff01ac4c0..48cf33ce74e 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -11,7 +11,7 @@ import type { CommandLineStringParameter, CommandLineChoiceParameter } from '@rushstack/ts-command-line'; -import { FileSystem, AlreadyReportedError } from '@rushstack/node-core-library'; +import { Async, FileSystem, JsonFile, AlreadyReportedError } from '@rushstack/node-core-library'; import { Colorize } from '@rushstack/terminal'; import { getRepoRoot } from '@rushstack/package-deps-hash'; @@ -387,6 +387,10 @@ export class ChangeAction extends BaseRushAction { } else { this._logNoChangeFileRequired(); } + + if (this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { + await this._detectDeletedProjectChangeFilesAsync(); + } } private async _getTargetBranchAsync(): Promise { @@ -446,6 +450,80 @@ export class ChangeAction extends BaseRushAction { await ChangeFiles.validateAsync(this.terminal, allChangeFiles, [], this.rushConfiguration); } + /** + * Detects projects that were deleted from rush.json on this branch and checks whether + * any existing change files still reference them. + */ + private async _detectDeletedProjectChangeFilesAsync(): Promise { + const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); + const targetBranch: string = await this._getTargetBranchAsync(); + const mergeBase: string = await this._git.getMergeBaseAsync(targetBranch, this.terminal); + + // Read the old rush.json from the merge base + let oldRushJsonContent: string; + try { + const rushJsonRelativePath: string = path.relative(repoRoot, this.rushConfiguration.rushJsonFile); + oldRushJsonContent = await this._git.getBlobContentAsync({ + blobSpec: `${mergeBase}:${rushJsonRelativePath}`, + repositoryRoot: repoRoot + }); + } catch { + // If rush.json didn't exist on the target branch, nothing to compare + return; + } + + const oldRushJson: { projects?: { packageName: string }[] } = JSON.parse(oldRushJsonContent); + const oldProjectNames: Set = new Set( + (oldRushJson.projects ?? []).map((p) => p.packageName) + ); + + const currentProjectNames: Set = new Set( + this.rushConfiguration.projects.map((p) => p.packageName) + ); + + const deletedProjectNames: Set = new Set(); + for (const name of oldProjectNames) { + if (!currentProjectNames.has(name)) { + deletedProjectNames.add(name); + } + } + + if (deletedProjectNames.size === 0) { + return; + } + + // Scan all change files for references to deleted projects + const changeFilesInstance: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); + const allChangeFilePaths: string[] = await changeFilesInstance.getFilesAsync(); + + const defunctChangeFiles: string[] = []; + await Async.forEachAsync( + allChangeFilePaths, + async (filePath) => { + const changeFile: IChangeInfo = await JsonFile.loadAsync(filePath); + if (changeFile?.changes) { + for (const change of changeFile.changes) { + if (deletedProjectNames.has(change.packageName)) { + defunctChangeFiles.push(filePath); + break; + } + } + } + }, + { concurrency: 50 } + ); + + if (defunctChangeFiles.length > 0) { + throw new Error( + [ + `The following change files reference projects that were removed from ${RushConstants.rushJsonFilename}. ` + + `Please delete them:`, + ...defunctChangeFiles.map((filePath) => `- ${filePath}`) + ].join('\n') + ); + } + } + private async _getChangeFilesAsync(): Promise { const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); const relativeChangesFolder: string = path.relative(repoRoot, this.rushConfiguration.changesFolder); From 523f4b91ec3c1437e637f10c92f2d26fd2c8cf7c Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 22 Mar 2026 03:38:42 -0400 Subject: [PATCH 11/14] fixup! Copilot WIP --- .../rush-lib/src/cli/actions/ChangeAction.ts | 104 ++++++++---------- libraries/rush-lib/src/logic/ChangeFiles.ts | 20 +++- 2 files changed, 60 insertions(+), 64 deletions(-) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 48cf33ce74e..6e5410df5f1 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -11,11 +11,12 @@ import type { CommandLineStringParameter, CommandLineChoiceParameter } from '@rushstack/ts-command-line'; -import { Async, FileSystem, JsonFile, AlreadyReportedError } from '@rushstack/node-core-library'; +import { FileSystem, JsonFile, AlreadyReportedError } from '@rushstack/node-core-library'; import { Colorize } from '@rushstack/terminal'; import { getRepoRoot } from '@rushstack/package-deps-hash'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; +import type { IRushConfigurationJson } from '../../api/RushConfiguration'; import { type IChangeFile, type IChangeInfo, ChangeType } from '../../api/ChangeManagement'; import { ChangeFile } from '../../api/ChangeFile'; import { BaseRushAction } from './BaseRushAction'; @@ -382,14 +383,33 @@ export class ChangeAction extends BaseRushAction { private async _verifyAsync(): Promise { const changedPackages: string[] = await this._getChangedProjectNamesAsync(); - if (changedPackages.length > 0) { - await this._validateChangeFileAsync(changedPackages); + const strictValidation: boolean = + !!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. + let filesToValidate: string[]; + if (strictValidation) { + const changeFilesInstance: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); + filesToValidate = await changeFilesInstance.getFilesAsync(); } else { - this._logNoChangeFileRequired(); + filesToValidate = await this._getChangeFilesAsync(); } - if (this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { - await this._detectDeletedProjectChangeFilesAsync(); + if (changedPackages.length > 0 || filesToValidate.length > 0) { + const deletedProjectNames: Set | undefined = strictValidation + ? await this._getDeletedProjectNamesAsync() + : undefined; + + await ChangeFiles.validateAsync( + this.terminal, + filesToValidate, + changedPackages, + this.rushConfiguration, + deletedProjectNames + ); + } else { + this._logNoChangeFileRequired(); } } @@ -432,11 +452,6 @@ export class ChangeAction extends BaseRushAction { return Array.from(changedProjectNames); } - private async _validateChangeFileAsync(changedPackages: string[]): Promise { - const files: string[] = await this._getChangeFilesAsync(); - await ChangeFiles.validateAsync(this.terminal, files, changedPackages, this.rushConfiguration); - } - private async _validateAllChangeFilesAsync(): Promise { if (!this.rushConfiguration.experimentsConfiguration.configuration.strictChangefileValidation) { throw new Error( @@ -447,19 +462,25 @@ export class ChangeAction extends BaseRushAction { const changeFiles: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); const allChangeFiles: string[] = await changeFiles.getFilesAsync(); - await ChangeFiles.validateAsync(this.terminal, allChangeFiles, [], this.rushConfiguration); + const deletedProjectNames: Set = await this._getDeletedProjectNamesAsync(); + await ChangeFiles.validateAsync( + this.terminal, + allChangeFiles, + [], + this.rushConfiguration, + deletedProjectNames + ); } /** - * Detects projects that were deleted from rush.json on this branch and checks whether - * any existing change files still reference them. + * Compares the current rush.json project list against the target branch to find + * projects that were removed. */ - private async _detectDeletedProjectChangeFilesAsync(): Promise { + private async _getDeletedProjectNamesAsync(): Promise> { const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); const targetBranch: string = await this._getTargetBranchAsync(); const mergeBase: string = await this._git.getMergeBaseAsync(targetBranch, this.terminal); - // Read the old rush.json from the merge base let oldRushJsonContent: string; try { const rushJsonRelativePath: string = path.relative(repoRoot, this.rushConfiguration.rushJsonFile); @@ -468,60 +489,23 @@ export class ChangeAction extends BaseRushAction { repositoryRoot: repoRoot }); } catch { - // If rush.json didn't exist on the target branch, nothing to compare - return; + // If rush.json didn't exist on the target branch, no projects were deleted + return new Set(); } - const oldRushJson: { projects?: { packageName: string }[] } = JSON.parse(oldRushJsonContent); - const oldProjectNames: Set = new Set( - (oldRushJson.projects ?? []).map((p) => p.packageName) - ); - + const oldRushJson: IRushConfigurationJson = JsonFile.parseString(oldRushJsonContent); const currentProjectNames: Set = new Set( this.rushConfiguration.projects.map((p) => p.packageName) ); const deletedProjectNames: Set = new Set(); - for (const name of oldProjectNames) { - if (!currentProjectNames.has(name)) { - deletedProjectNames.add(name); + for (const project of oldRushJson.projects) { + if (!currentProjectNames.has(project.packageName)) { + deletedProjectNames.add(project.packageName); } } - if (deletedProjectNames.size === 0) { - return; - } - - // Scan all change files for references to deleted projects - const changeFilesInstance: ChangeFiles = new ChangeFiles(this.rushConfiguration.changesFolder); - const allChangeFilePaths: string[] = await changeFilesInstance.getFilesAsync(); - - const defunctChangeFiles: string[] = []; - await Async.forEachAsync( - allChangeFilePaths, - async (filePath) => { - const changeFile: IChangeInfo = await JsonFile.loadAsync(filePath); - if (changeFile?.changes) { - for (const change of changeFile.changes) { - if (deletedProjectNames.has(change.packageName)) { - defunctChangeFiles.push(filePath); - break; - } - } - } - }, - { concurrency: 50 } - ); - - if (defunctChangeFiles.length > 0) { - throw new Error( - [ - `The following change files reference projects that were removed from ${RushConstants.rushJsonFilename}. ` + - `Please delete them:`, - ...defunctChangeFiles.map((filePath) => `- ${filePath}`) - ].join('\n') - ); - } + return deletedProjectNames; } private async _getChangeFilesAsync(): Promise { diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 29e7414ab5f..11ee44563a3 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/libraries/rush-lib/src/logic/ChangeFiles.ts @@ -28,12 +28,16 @@ export class ChangeFiles { /** * 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 + rushConfiguration: RushConfiguration, + deletedProjectNames?: ReadonlySet ): Promise { const schema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); @@ -86,9 +90,17 @@ export class ChangeFiles { const project: RushConfigurationProject | undefined = rushConfiguration.getProjectByName(packageName); if (!project) { - errors.push( - `Change file(s) reference a project "${packageName}" that does not exist in the Rush configuration:\n${fileList}` - ); + if (deletedProjectNames?.has(packageName)) { + errors.push( + `The project "${packageName}" was removed from rush.json, but the following change ` + + `files still reference it. Please delete them:\n${fileList}` + ); + } else { + errors.push( + `Change file(s) reference a project "${packageName}" that does not exist in the Rush ` + + `configuration:\n${fileList}` + ); + } continue; } From 22d75a392f0174b9ba257312376fc17877164fa1 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 22 Mar 2026 18:10:50 -0400 Subject: [PATCH 12/14] Refactor ChangeFiles API. --- .../rush-lib/src/cli/actions/ChangeAction.ts | 36 +++--- .../rush-lib/src/cli/actions/PublishAction.ts | 6 +- libraries/rush-lib/src/logic/ChangeFiles.ts | 54 +++++---- libraries/rush-lib/src/logic/ChangeManager.ts | 4 +- .../rush-lib/src/logic/PublishUtilities.ts | 2 +- .../rush-lib/src/logic/VersionManager.ts | 2 +- .../src/logic/test/ChangeFiles.test.ts | 112 +++++++++++++----- .../src/logic/test/ChangeManager.test.ts | 57 ++++++--- .../src/logic/test/PublishUtilities.test.ts | 60 +++++----- .../__snapshots__/ChangeFiles.test.ts.snap | 6 +- 10 files changed, 210 insertions(+), 129 deletions(-) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 6e5410df5f1..f7f84d8b699 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -382,32 +382,31 @@ export class ChangeAction extends BaseRushAction { } private async _verifyAsync(): Promise { - 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 | 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 = await this._getDeletedProjectNamesAsync(); - await ChangeFiles.validateAsync( - this.terminal, - allChangeFiles, - [], - this.rushConfiguration, + await changeFiles.validateAsync({ + terminal: this.terminal, + filesToValidate: allChangeFiles, + changedProjectNames: [], deletedProjectNames - ); + }); } /** diff --git a/libraries/rush-lib/src/cli/actions/PublishAction.ts b/libraries/rush-lib/src/cli/actions/PublishAction.ts index 328e1b9cce2..0a8094b07ce 100644 --- a/libraries/rush-lib/src/cli/actions/PublishAction.ts +++ b/libraries/rush-lib/src/cli/actions/PublishAction.ts @@ -281,11 +281,7 @@ export class PublishAction extends BaseRushAction { allPackages: ReadonlyMap ): Promise { 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; diff --git a/libraries/rush-lib/src/logic/ChangeFiles.ts b/libraries/rush-lib/src/logic/ChangeFiles.ts index 11ee44563a3..d63e011dbd7 100644 --- a/libraries/rush-lib/src/logic/ChangeFiles.ts +++ b/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; + changedProjectNames: Iterable; + /** + * Optional set of project names that were removed from rush.json. + * When provided, produces a more specific error message for these projects. + */ + deletedProjectNames?: ReadonlySet; +} + /** * 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 - ): Promise { + public async validateAsync(options: IValidateOptions): Promise { + 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 = new Set(); const changefilesByProjectName: Map = 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 = new Set(changedPackages); + const projectsMissingChangeDescriptions: Set = 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 { + public async getAllChangeFilesAsync(): Promise { 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) { diff --git a/libraries/rush-lib/src/logic/ChangeManager.ts b/libraries/rush-lib/src/logic/ChangeManager.ts index 5e1f2fac01c..7b891d37b7d 100644 --- a/libraries/rush-lib/src/logic/ChangeManager.ts +++ b/libraries/rush-lib/src/logic/ChangeManager.ts @@ -34,12 +34,10 @@ 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 { @@ -47,7 +45,7 @@ export class ChangeManager { this._prereleaseToken = prereleaseToken; - this._changeFiles = new ChangeFiles(changesPath); + this._changeFiles = new ChangeFiles(this._rushConfiguration); this._allChanges = await PublishUtilities.findChangeRequestsAsync( this._allPackages, this._rushConfiguration, diff --git a/libraries/rush-lib/src/logic/PublishUtilities.ts b/libraries/rush-lib/src/logic/PublishUtilities.ts index b5d3c2a89cc..a76f946f92e 100644 --- a/libraries/rush-lib/src/logic/PublishUtilities.ts +++ b/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) { diff --git a/libraries/rush-lib/src/logic/VersionManager.ts b/libraries/rush-lib/src/logic/VersionManager.ts index ec5095d3c8e..ac41623f215 100644 --- a/libraries/rush-lib/src/logic/VersionManager.ts +++ b/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) => { diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 3167f7fe4be..6cc5f63c137 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -38,24 +38,30 @@ describe(ChangeFiles.name, () => { ).toMatchSnapshot(); }); - describe(ChangeFiles.prototype.getFilesAsync.name, () => { + describe(ChangeFiles.prototype.getAllChangeFilesAsync.name, () => { it('returns correctly when there is one change file', async () => { const changesPath: string = `${__dirname}/leafChange`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); const expectedPath: string = Path.convertToSlashes(`${changesPath}/change1.json`); - expect(await changeFiles.getFilesAsync()).toEqual([expectedPath]); + expect(await changeFiles.getAllChangeFilesAsync()).toEqual([expectedPath]); }); it('returns empty array when no change files', async () => { const changesPath: string = `${__dirname}/noChange`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); - expect(await changeFiles.getFilesAsync()).toHaveLength(0); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); + expect(await changeFiles.getAllChangeFilesAsync()).toHaveLength(0); }); it('returns correctly when change files are categorized', async () => { const changesPath: string = `${__dirname}/categorizedChanges`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); - const files: string[] = await changeFiles.getFilesAsync(); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); + const files: string[] = await changeFiles.getAllChangeFilesAsync(); expect(files).toHaveLength(3); const expectedPathA: string = Path.convertToSlashes(`${changesPath}/@ms/a/changeA.json`); @@ -67,38 +73,57 @@ describe(ChangeFiles.name, () => { }); }); - describe(ChangeFiles.validateAsync.name, () => { + describe(ChangeFiles.prototype.validateAsync.name, () => { it('throws when there is a patch in a hotfix branch.', async () => { const changeFile: string = `${__dirname}/leafChange/change1.json`; const changedPackages: string[] = ['d']; await expect( - ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, { - hotfixChangeEnabled: true - } as RushConfiguration) + new ChangeFiles({ + hotfixChangeEnabled: true, + experimentsConfiguration: { + configuration: {} + } + } as unknown as RushConfiguration).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: changedPackages + }) ).rejects.toThrow(Error); }); it('allows a hotfix in a hotfix branch.', async () => { const changeFile: string = `${__dirname}/multipleHotfixChanges/change1.json`; const changedPackages: string[] = ['a']; - await ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, { + await new ChangeFiles({ ...rushConfiguration, hotfixChangeEnabled: true - } as RushConfiguration); + } as unknown as RushConfiguration).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: changedPackages + }); }); it('throws when there is any missing package.', async () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a', 'b', 'c']; await expect( - ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, rushConfiguration) + new ChangeFiles(rushConfiguration).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: changedPackages + }) ).rejects.toThrow(Error); }); it('does not throw when there is no missing packages', async () => { const changeFile: string = `${__dirname}/verifyChanges/changes.json`; const changedPackages: string[] = ['a']; - await ChangeFiles.validateAsync(terminal, [changeFile], changedPackages, rushConfiguration); + await new ChangeFiles(rushConfiguration).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: changedPackages + }); }); it('throws when missing packages from categorized changes', async () => { @@ -106,7 +131,11 @@ describe(ChangeFiles.name, () => { const changeFileB: string = `${__dirname}/categorizedChanges/@ms/b/changeB.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; await expect( - ChangeFiles.validateAsync(terminal, [changeFileA, changeFileB], changedPackages, rushConfiguration) + new ChangeFiles(rushConfiguration).validateAsync({ + terminal, + filesToValidate: [changeFileA, changeFileB], + changedProjectNames: changedPackages + }) ).rejects.toThrow(Error); }); @@ -115,12 +144,11 @@ describe(ChangeFiles.name, () => { const changeFileB: string = `${__dirname}/categorizedChanges/@ms/b/changeB.json`; const changeFileC: string = `${__dirname}/categorizedChanges/changeC.json`; const changedPackages: string[] = ['@ms/a', '@ms/b', 'c']; - await ChangeFiles.validateAsync( + await new ChangeFiles(rushConfiguration).validateAsync({ terminal, - [changeFileA, changeFileB, changeFileC], - changedPackages, - rushConfiguration - ); + filesToValidate: [changeFileA, changeFileB, changeFileC], + changedProjectNames: changedPackages + }); }); describe('with strictChangefileValidation', () => { @@ -141,7 +169,11 @@ describe(ChangeFiles.name, () => { const changeFile: string = `${__dirname}/strictValidation/nonexistentProject.json`; strictConfig = createStrictConfig(() => undefined); try { - await ChangeFiles.validateAsync(terminal, [changeFile], ['nonexistent-package'], strictConfig); + await new ChangeFiles(strictConfig).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: ['nonexistent-package'] + }); fail('Expected validateAsync to throw'); } catch (error) { const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); @@ -165,7 +197,11 @@ describe(ChangeFiles.name, () => { return undefined; }); try { - await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-secondary'], strictConfig); + await new ChangeFiles(strictConfig).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: ['lockstep-secondary'] + }); fail('Expected validateAsync to throw'); } catch (error) { const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); @@ -188,7 +224,11 @@ describe(ChangeFiles.name, () => { } return undefined; }); - await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-main'], strictConfig); + await new ChangeFiles(strictConfig).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: ['lockstep-main'] + }); }); it('does not throw when change file references a lockstep project with no mainProject', async () => { @@ -206,7 +246,11 @@ describe(ChangeFiles.name, () => { } return undefined; }); - await ChangeFiles.validateAsync(terminal, [changeFile], ['lockstep-main'], strictConfig); + await new ChangeFiles(strictConfig).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: ['lockstep-main'] + }); }); it('does not throw when experiment is disabled', async () => { @@ -216,7 +260,11 @@ describe(ChangeFiles.name, () => { configuration: { strictChangefileValidation: false } } as ExperimentsConfiguration } as unknown as RushConfiguration; - await ChangeFiles.validateAsync(terminal, [changeFile], ['nonexistent-package'], config); + await new ChangeFiles(config).validateAsync({ + terminal, + filesToValidate: [changeFile], + changedProjectNames: ['nonexistent-package'] + }); }); }); }); @@ -224,13 +272,17 @@ describe(ChangeFiles.name, () => { describe(ChangeFiles.prototype.deleteAllAsync.name, () => { it('delete all files when there are no prerelease packages', async () => { const changesPath: string = `${__dirname}/multipleChangeFiles`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); expect(await changeFiles.deleteAllAsync(terminal, false)).toEqual(3); }); it('does not delete change files for package whose change logs do not get updated. ', async () => { const changesPath: string = `${__dirname}/multipleChangeFiles`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); const updatedChangelogs: IChangelog[] = [ { name: 'a', @@ -246,7 +298,9 @@ describe(ChangeFiles.name, () => { it('delete all files when there are hotfixes', async () => { const changesPath: string = `${__dirname}/multipleHotfixChanges`; - const changeFiles: ChangeFiles = new ChangeFiles(changesPath); + const changeFiles: ChangeFiles = new ChangeFiles({ + changesFolder: changesPath + } as unknown as RushConfiguration); expect(await changeFiles.deleteAllAsync(terminal, false)).toEqual(3); }); }); diff --git a/libraries/rush-lib/src/logic/test/ChangeManager.test.ts b/libraries/rush-lib/src/logic/test/ChangeManager.test.ts index d305bb4b1fc..f953c238357 100644 --- a/libraries/rush-lib/src/logic/test/ChangeManager.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeManager.test.ts @@ -18,7 +18,8 @@ describe(ChangeManager.name, () => { /* eslint-disable dot-notation */ it('can apply changes to the package.json files in the dictionary', async () => { - await changeManager.loadAsync(`${__dirname}/multipleChanges`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/multipleChanges`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('2.0.0'); @@ -33,7 +34,8 @@ describe(ChangeManager.name, () => { }); it('can update explicit version dependency', async () => { - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('c')!.packageJson.version).toEqual('1.0.1'); @@ -42,7 +44,8 @@ describe(ChangeManager.name, () => { }); it('can update a project using lockStepVersion policy with no nextBump from changefiles', async () => { - await changeManager.loadAsync(`${__dirname}/lockstepWithoutNextBump`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/lockstepWithoutNextBump`; + await changeManager.loadAsync(); changeManager.apply(false); const policy: LockStepVersionPolicy = rushConfiguration.versionPolicyConfiguration.getVersionPolicy( @@ -55,7 +58,8 @@ describe(ChangeManager.name, () => { }); it('can update explicit cyclic dependency', async () => { - await changeManager.loadAsync(`${__dirname}/cyclicDepsExplicit`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDepsExplicit`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-explicit-1')!.packageJson.version).toEqual('2.0.0'); @@ -76,7 +80,8 @@ describe(ChangeManager.name, () => { const prereleaseName: string = 'alpha.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/rootPatchChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/rootPatchChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.1-' + prereleaseName); @@ -95,7 +100,8 @@ describe(ChangeManager.name, () => { const prereleaseName: string = 'beta.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0'); @@ -112,7 +118,8 @@ describe(ChangeManager.name, () => { const prereleaseName: string = 'beta.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/cyclicDeps`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDeps`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-1')!.packageJson.version).toEqual( @@ -133,7 +140,8 @@ describe(ChangeManager.name, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/rootPatchChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/rootPatchChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0-' + suffix); @@ -148,7 +156,8 @@ describe(ChangeManager.name, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0'); @@ -163,7 +172,8 @@ describe(ChangeManager.name, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/cyclicDeps`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDeps`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-1')!.packageJson.version).toEqual('1.0.0-' + suffix); @@ -190,7 +200,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { /* eslint-disable dot-notation */ it('can apply changes to the package.json files in the dictionary', async () => { - await changeManager.loadAsync(`${__dirname}/multipleChanges`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/multipleChanges`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('2.0.0'); @@ -213,7 +224,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { }); it('can update explicit version dependency', async () => { - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('c')!.packageJson.version).toEqual('1.0.1'); @@ -222,7 +234,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { }); it('can update explicit cyclic dependency', async () => { - await changeManager.loadAsync(`${__dirname}/cyclicDepsExplicit`); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDepsExplicit`; + await changeManager.loadAsync(); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-explicit-1')!.packageJson.version).toEqual('2.0.0'); @@ -243,7 +256,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const prereleaseName: string = 'alpha.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/rootPatchChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/rootPatchChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.1-' + prereleaseName); @@ -262,7 +276,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const prereleaseName: string = 'beta.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0'); @@ -281,7 +296,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const prereleaseName: string = 'beta.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(prereleaseName); - await changeManager.loadAsync(`${__dirname}/cyclicDeps`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDeps`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-1')!.packageJson.version).toEqual( @@ -302,7 +318,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/rootPatchChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/rootPatchChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0-' + suffix); @@ -321,7 +338,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/explicitVersionChange`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/explicitVersionChange`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('a')!.packageJson.version).toEqual('1.0.0'); @@ -340,7 +358,8 @@ describe(`${ChangeManager.name} (workspace)`, () => { const suffix: string = 'dk.1'; const prereleaseToken: PrereleaseToken = new PrereleaseToken(undefined, suffix); - await changeManager.loadAsync(`${__dirname}/cyclicDeps`, prereleaseToken); + (rushConfiguration as { changesFolder: string }).changesFolder = `${__dirname}/cyclicDeps`; + await changeManager.loadAsync(prereleaseToken); changeManager.apply(false); expect(changeManager.allPackages.get('cyclic-dep-1')!.packageJson.version).toEqual('1.0.0-' + suffix); diff --git a/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts b/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts index a0d127208de..3ac792fe0f6 100644 --- a/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts +++ b/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts @@ -7,6 +7,10 @@ import type { RushConfigurationProject } from '../../api/RushConfigurationProjec import { PublishUtilities, type IChangeRequests } from '../PublishUtilities'; import { ChangeFiles } from '../ChangeFiles'; +function createChangeFiles(changesFolder: string): ChangeFiles { + return new ChangeFiles({ changesFolder } as unknown as RushConfiguration); +} + function generateChangeSnapshot( allPackages: ReadonlyMap, allChanges: IChangeRequests @@ -85,7 +89,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/noChange`) + createChangeFiles(`${__dirname}/noChange`) ); expect(allChanges.packageChanges.size).toEqual(0); @@ -98,7 +102,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/leafChange`) + createChangeFiles(`${__dirname}/leafChange`) ); expect(allChanges.packageChanges.size).toEqual(1); @@ -114,7 +118,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootPatchChange`) + createChangeFiles(`${__dirname}/rootPatchChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -148,7 +152,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootHotfixChange`) + createChangeFiles(`${__dirname}/rootHotfixChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -180,7 +184,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootMajorChange`) + createChangeFiles(`${__dirname}/rootMajorChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -214,7 +218,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/lockstepWithoutNextBump`) + createChangeFiles(`${__dirname}/lockstepWithoutNextBump`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -248,7 +252,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/cyclicDeps`) + createChangeFiles(`${__dirname}/cyclicDeps`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -282,7 +286,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/hotfixWithPatchChanges`) + createChangeFiles(`${__dirname}/hotfixWithPatchChanges`) ) ).rejects.toThrow('Cannot apply hotfix alongside patch change on same package'); }); @@ -298,7 +302,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootHotfixChange`) + createChangeFiles(`${__dirname}/rootHotfixChange`) ) ).rejects.toThrow('Cannot add hotfix change; hotfixChangeEnabled is false in configuration.'); }); @@ -309,7 +313,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/multipleChanges`) + createChangeFiles(`${__dirname}/multipleChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -343,7 +347,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/orderedChanges`) + createChangeFiles(`${__dirname}/orderedChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -377,7 +381,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/multipleHotfixChanges`) + createChangeFiles(`${__dirname}/multipleHotfixChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -409,7 +413,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/explicitVersionChange`) + createChangeFiles(`${__dirname}/explicitVersionChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -440,7 +444,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, repoRushConfiguration, - new ChangeFiles(`${__dirname}/repo/changes`), + createChangeFiles(`${__dirname}/repo/changes`), false, undefined, new Set(['a', 'b', 'e']) @@ -480,7 +484,7 @@ describe(PublishUtilities.sortChangeRequests.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, rushConfiguration, - new ChangeFiles(`${__dirname}/multipleChanges`) + createChangeFiles(`${__dirname}/multipleChanges`) ); const orderedChanges: IChangeInfo[] = PublishUtilities.sortChangeRequests(allChanges.packageChanges); @@ -569,7 +573,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/noChange`) + createChangeFiles(`${__dirname}/noChange`) ); expect(allChanges.packageChanges.size).toEqual(0); @@ -582,7 +586,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/leafChange`) + createChangeFiles(`${__dirname}/leafChange`) ); expect(allChanges.packageChanges.size).toEqual(1); @@ -598,7 +602,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootPatchChange`) + createChangeFiles(`${__dirname}/rootPatchChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -632,7 +636,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootHotfixChange`) + createChangeFiles(`${__dirname}/rootHotfixChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -664,7 +668,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootMajorChange`) + createChangeFiles(`${__dirname}/rootMajorChange`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -698,7 +702,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/cyclicDeps`) + createChangeFiles(`${__dirname}/cyclicDeps`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -732,7 +736,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/hotfixWithPatchChanges`) + createChangeFiles(`${__dirname}/hotfixWithPatchChanges`) ) ).rejects.toThrow('Cannot apply hotfix alongside patch change on same package'); }); @@ -748,7 +752,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/rootHotfixChange`) + createChangeFiles(`${__dirname}/rootHotfixChange`) ) ).rejects.toThrow('Cannot add hotfix change; hotfixChangeEnabled is false in configuration.'); }); @@ -759,7 +763,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/multipleChanges`) + createChangeFiles(`${__dirname}/multipleChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -793,7 +797,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/orderedChanges`) + createChangeFiles(`${__dirname}/orderedChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -827,7 +831,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/multipleHotfixChanges`) + createChangeFiles(`${__dirname}/multipleHotfixChanges`) ); expect(generateChangeSnapshot(allPackages, allChanges)).toMatchInlineSnapshot(` @@ -859,7 +863,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, packagesRushConfiguration, - new ChangeFiles(`${__dirname}/explicitVersionChange`) + createChangeFiles(`${__dirname}/explicitVersionChange`) ); expect(allChanges.packageChanges.size).toEqual(2); @@ -877,7 +881,7 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { const allChanges: IChangeRequests = await PublishUtilities.findChangeRequestsAsync( allPackages, repoRushConfiguration, - new ChangeFiles(`${__dirname}/repo/changes`), + createChangeFiles(`${__dirname}/repo/changes`), false, undefined, new Set(['a', 'b', 'e']) diff --git a/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap b/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap index 5d08f60c3e6..8c24e63b4d6 100644 --- a/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap +++ b/libraries/rush-lib/src/logic/test/__snapshots__/ChangeFiles.test.ts.snap @@ -29,11 +29,11 @@ Array [ ] `; -exports[`ChangeFiles getFilesAsync returns correctly when change files are categorized 1`] = `Array []`; +exports[`ChangeFiles getAllChangeFilesAsync returns correctly when change files are categorized 1`] = `Array []`; -exports[`ChangeFiles getFilesAsync returns correctly when there is one change file 1`] = `Array []`; +exports[`ChangeFiles getAllChangeFilesAsync returns correctly when there is one change file 1`] = `Array []`; -exports[`ChangeFiles getFilesAsync returns empty array when no change files 1`] = `Array []`; +exports[`ChangeFiles getAllChangeFilesAsync returns empty array when no change files 1`] = `Array []`; exports[`ChangeFiles validateAsync allows a hotfix in a hotfix branch. 1`] = ` Array [ From f28caaeee48a127adcae223f686b07d074e3d0ef Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 22 Mar 2026 18:22:16 -0400 Subject: [PATCH 13/14] Minor cleanup. --- .../rush-lib/src/cli/actions/ChangeAction.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index f7f84d8b699..477e97799c5 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -320,7 +320,7 @@ export class ChangeAction extends BaseRushAction { const existingChangeComments: Map = ChangeFiles.getChangeComments( this.terminal, - await this._getChangeFilesAsync() + await this._getChangeFilesSinceBaseBranchAsync() ); changeFileData = await this._promptForChangeFileDataAsync( promptModule, @@ -393,7 +393,7 @@ export class ChangeAction extends BaseRushAction { if (strictValidation) { filesToValidate = await changeFilesInstance.getAllChangeFilesAsync(); } else { - filesToValidate = await this._getChangeFilesAsync(); + filesToValidate = await this._getChangeFilesSinceBaseBranchAsync(); } if (changedProjectNames.length > 0 || filesToValidate.length > 0) { @@ -492,21 +492,20 @@ export class ChangeAction extends BaseRushAction { } const oldRushJson: IRushConfigurationJson = JsonFile.parseString(oldRushJsonContent); - const currentProjectNames: Set = new Set( - this.rushConfiguration.projects.map((p) => p.packageName) - ); + const currentProjectsByName: ReadonlyMap = + this.rushConfiguration.projectsByName; const deletedProjectNames: Set = new Set(); - for (const project of oldRushJson.projects) { - if (!currentProjectNames.has(project.packageName)) { - deletedProjectNames.add(project.packageName); + for (const { packageName } of oldRushJson.projects) { + if (!currentProjectsByName.has(packageName)) { + deletedProjectNames.add(packageName); } } return deletedProjectNames; } - private async _getChangeFilesAsync(): Promise { + private async _getChangeFilesSinceBaseBranchAsync(): Promise { const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); const relativeChangesFolder: string = path.relative(repoRoot, this.rushConfiguration.changesFolder); const targetBranch: string = await this._getTargetBranchAsync(); From 4b0ffccd45df518b0b20d6cb3c7a7912975cd084 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 22 Mar 2026 22:01:55 -0700 Subject: [PATCH 14/14] fixup! Snapshot test output. --- .../rush-lib/src/logic/test/ChangeFiles.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts index 6cc5f63c137..e32a40aea85 100644 --- a/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts +++ b/libraries/rush-lib/src/logic/test/ChangeFiles.test.ts @@ -34,7 +34,7 @@ describe(ChangeFiles.name, () => { expect( terminalProvider .getAllOutputAsChunks({ asLines: true }) - .map((chunk) => chunk.replace(FORWARD_SLASH_DIRNAME, '')) + .map((chunk) => Path.convertToSlashes(chunk).replace(FORWARD_SLASH_DIRNAME, '')) ).toMatchSnapshot(); }); @@ -176,7 +176,10 @@ describe(ChangeFiles.name, () => { }); fail('Expected validateAsync to throw'); } catch (error) { - const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); + const normalizedMessage: string = Path.convertToSlashes(error.message).replace( + FORWARD_SLASH_DIRNAME, + '' + ); expect(normalizedMessage).toMatchSnapshot(); } }); @@ -204,7 +207,10 @@ describe(ChangeFiles.name, () => { }); fail('Expected validateAsync to throw'); } catch (error) { - const normalizedMessage: string = error.message.replace(FORWARD_SLASH_DIRNAME, ''); + const normalizedMessage: string = Path.convertToSlashes(error.message).replace( + FORWARD_SLASH_DIRNAME, + '' + ); expect(normalizedMessage).toMatchSnapshot(); } });