diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index de1eca71dc9..cfbede85e96 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -139,7 +139,9 @@ export class CommandLineConfiguration { */ public constructor(commandLineJson: ICommandLineJson | undefined) { if (commandLineJson?.phases) { - const phaseNamePrefix: string = RushConstants.phaseNamePrefix; + const phaseNameRegexp: RegExp = new RegExp( + `^${RushConstants.phaseNamePrefix}[a-z][a-z0-9]*([-][a-z0-9]+)*$` + ); for (const phase of commandLineJson.phases) { if (this.phases.has(phase.name)) { throw new Error( @@ -148,17 +150,13 @@ export class CommandLineConfiguration { ); } - if (phase.name.substring(0, phaseNamePrefix.length) !== phaseNamePrefix) { + if (!phase.name.match(phaseNameRegexp)) { throw new Error( `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `does not begin with the required prefix "${phaseNamePrefix}".` - ); - } - - if (phase.name.length <= phaseNamePrefix.length) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `must have characters after "${phaseNamePrefix}"` + 'is not a valid phase name. Phase names must begin with the ' + + `required prefix "${RushConstants.phaseNamePrefix}" followed by a name containing ` + + 'lowercase letters, numbers, or hyphens. The name must start with a letter and ' + + 'must not end with a hyphen.' ); } @@ -441,9 +439,9 @@ export class CommandLineConfiguration { } private _checkForPhaseSelfCycles(phase: IPhase, checkedPhases: Set = new Set()): void { - const dependencies: string[] | undefined = phase.dependencies?.self; - if (dependencies) { - for (const dependencyName of dependencies) { + const phaseSelfDependencies: string[] | undefined = phase.dependencies?.self; + if (phaseSelfDependencies) { + for (const dependencyName of phaseSelfDependencies) { if (checkedPhases.has(dependencyName)) { const dependencyNameForError: string = typeof dependencyName === 'string' ? dependencyName : ''; @@ -457,7 +455,7 @@ export class CommandLineConfiguration { if (!dependency) { return; // Ignore, we check for this separately } else { - if (dependencies.length > 1) { + if (phaseSelfDependencies.length > 1) { this._checkForPhaseSelfCycles( dependency, // Clone the set of checked phases if there are multiple branches we need to check @@ -549,6 +547,12 @@ export class CommandLineConfiguration { this._additionalPathFolders.unshift(pathFolder); } + /** + * This function replaces colons (":") with underscores ("_"). + * + * ts-command-line restricts command names to lowercase letters, numbers, underscores, and colons. + * Replacing colons with underscores produces a filesystem-safe name. + */ private _normalizeNameForLogFilenameIdentifiers(name: string): string { return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe } diff --git a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts index 4cc7626cf07..d3c0082b927 100644 --- a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts +++ b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts @@ -26,6 +26,36 @@ describe(CommandLineConfiguration.name, () => { ] }) ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:0' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:A' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:A-' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); }); it('Detects a missing phase', () => { @@ -41,7 +71,7 @@ describe(CommandLineConfiguration.name, () => { safeForSimultaneousRushProcesses: false, enableParallelism: true, - phases: ['_phase:A'] + phases: ['_phase:a'] } ] }) @@ -54,9 +84,9 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - upstream: ['_phase:B'] + upstream: ['_phase:b'] } } ] @@ -68,9 +98,9 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - self: ['_phase:B'] + self: ['_phase:b'] } } ] @@ -84,21 +114,21 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - self: ['_phase:B'] + self: ['_phase:b'] } }, { - name: '_phase:B', + name: '_phase:b', dependencies: { - self: ['_phase:C'] + self: ['_phase:c'] } }, { - name: '_phase:C', + name: '_phase:c', dependencies: { - self: ['_phase:A'] + self: ['_phase:a'] } } ] @@ -168,19 +198,19 @@ describe(CommandLineConfiguration.name, () => { summary: 'custom-phased', enableParallelism: true, safeForSimultaneousRushProcesses: false, - phases: ['_phase:A'] + phases: ['_phase:a'] } ], phases: [ { - name: '_phase:A' + name: '_phase:a' } ], parameters: [ { parameterKind: 'flag', longName: '--flag', - associatedPhases: ['_phase:A'], + associatedPhases: ['_phase:a'], description: 'flag' } ] @@ -202,25 +232,25 @@ describe(CommandLineConfiguration.name, () => { summary: 'custom-phased', enableParallelism: true, safeForSimultaneousRushProcesses: false, - phases: ['_phase:A'] + phases: ['_phase:a'] } ], phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - upstream: ['_phase:B'] + upstream: ['_phase:b'] } }, { - name: '_phase:B' + name: '_phase:b' } ], parameters: [ { parameterKind: 'flag', longName: '--flag', - associatedPhases: ['_phase:B'], + associatedPhases: ['_phase:b'], description: 'flag' } ] diff --git a/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap b/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap index 83e3d560365..55015a7af18 100644 --- a/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap +++ b/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap @@ -1,13 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`CommandLineConfiguration Detects a cycle among phases 1`] = `"In command-line.json, there exists a cycle within the set of _phase:B dependencies: _phase:B, _phase:C, _phase:A"`; +exports[`CommandLineConfiguration Detects a cycle among phases 1`] = `"In command-line.json, there exists a cycle within the set of _phase:b dependencies: _phase:b, _phase:c, _phase:a"`; -exports[`CommandLineConfiguration Detects a missing phase 1`] = `"In command-line.json, in the \\"phases\\" property of the \\"example\\" command, the phase \\"_phase:A\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase 1`] = `"In command-line.json, in the \\"phases\\" property of the \\"example\\" command, the phase \\"_phase:a\\" does not exist."`; -exports[`CommandLineConfiguration Detects a missing phase dependency 1`] = `"In command-line.json, in the phase \\"_phase:A\\", the upstream dependency phase \\"_phase:B\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase dependency 1`] = `"In command-line.json, in the phase \\"_phase:a\\", the upstream dependency phase \\"_phase:b\\" does not exist."`; -exports[`CommandLineConfiguration Detects a missing phase dependency 2`] = `"In command-line.json, in the phase \\"_phase:A\\", the self dependency phase \\"_phase:B\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase dependency 2`] = `"In command-line.json, in the phase \\"_phase:a\\", the self dependency phase \\"_phase:b\\" does not exist."`; -exports[`CommandLineConfiguration Forbids a misnamed phase 1`] = `"In command-line.json, the phase \\"_faze:A\\"'s name does not begin with the required prefix \\"_phase:\\"."`; +exports[`CommandLineConfiguration Forbids a misnamed phase 1`] = `"In command-line.json, the phase \\"_faze:A\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; -exports[`CommandLineConfiguration Forbids a misnamed phase 2`] = `"In command-line.json, the phase \\"_phase:\\"'s name must have characters after \\"_phase:\\""`; +exports[`CommandLineConfiguration Forbids a misnamed phase 2`] = `"In command-line.json, the phase \\"_phase:\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 3`] = `"In command-line.json, the phase \\"_phase:0\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 4`] = `"In command-line.json, the phase \\"_phase:A\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 5`] = `"In command-line.json, the phase \\"_phase:A-\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`;