From ef0d7a8effcf38ec043b76d97c616735d1cd3649 Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 19 Apr 2022 13:28:48 -0700 Subject: [PATCH] [rush-lib] Relax association for custom parameters --- ...ssociated-parameters_2022-04-19-20-28.json | 10 +++ .../src/api/CommandLineConfiguration.ts | 18 +---- .../api/test/CommandLineConfiguration.test.ts | 67 ++++++++++--------- 3 files changed, 49 insertions(+), 46 deletions(-) create mode 100644 common/changes/@microsoft/rush/unassociated-parameters_2022-04-19-20-28.json diff --git a/common/changes/@microsoft/rush/unassociated-parameters_2022-04-19-20-28.json b/common/changes/@microsoft/rush/unassociated-parameters_2022-04-19-20-28.json new file mode 100644 index 00000000000..a26136b2ff5 --- /dev/null +++ b/common/changes/@microsoft/rush/unassociated-parameters_2022-04-19-20-28.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Remove requirement that custom parameters associated with a phased command must also be associated with one or more phases. This allows for custom parameters that will only be interpreted by plugins.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/libraries/rush-lib/src/api/CommandLineConfiguration.ts b/libraries/rush-lib/src/api/CommandLineConfiguration.ts index 06afef6840a..45165180ed5 100644 --- a/libraries/rush-lib/src/api/CommandLineConfiguration.ts +++ b/libraries/rush-lib/src/api/CommandLineConfiguration.ts @@ -418,8 +418,6 @@ export class CommandLineConfiguration { this.parameters.push(normalizedParameter); - let parameterHasAssociatedPhases: boolean = false; - // Do some basic validation switch (normalizedParameter.parameterKind) { case 'choice': { @@ -441,7 +439,6 @@ export class CommandLineConfiguration { } let parameterHasAssociatedCommands: boolean = false; - let parameterIsOnlyAssociatedWithPhasedCommands: boolean = true; if (normalizedParameter.associatedCommands) { for (const associatedCommandName of normalizedParameter.associatedCommands) { const syntheticPhase: IPhase | undefined = @@ -462,10 +459,6 @@ export class CommandLineConfiguration { } else { associatedCommand.associatedParameters.add(normalizedParameter); parameterHasAssociatedCommands = true; - - if (associatedCommand.commandKind !== RushConstants.phasedCommandKind) { - parameterIsOnlyAssociatedWithPhasedCommands = false; - } } } } @@ -478,9 +471,6 @@ export class CommandLineConfiguration { `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + `that is associated with a phase "${associatedPhaseName}" that does not exist.` ); - } else { - // Defer association to PhasedScriptAction so that it can map to the ts-command-line object - parameterHasAssociatedPhases = true; } } } @@ -492,12 +482,8 @@ export class CommandLineConfiguration { ); } - if (parameterIsOnlyAssociatedWithPhasedCommands && !parameterHasAssociatedPhases) { - throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + - `that is only associated with phased commands, but lists no associated phases.` - ); - } + // In the presence of plugins, there is utility to defining parameters that are associated with a phased + // command but no phases. Don't enforce that a parameter is associated with at least one phase. } } } diff --git a/libraries/rush-lib/src/api/test/CommandLineConfiguration.test.ts b/libraries/rush-lib/src/api/test/CommandLineConfiguration.test.ts index 77632fb9b88..ea069525820 100644 --- a/libraries/rush-lib/src/api/test/CommandLineConfiguration.test.ts +++ b/libraries/rush-lib/src/api/test/CommandLineConfiguration.test.ts @@ -2,7 +2,7 @@ // See LICENSE in the project root for license information. import { RushConstants } from '../../logic/RushConstants'; -import { Command, CommandLineConfiguration, IParameterJson } from '../CommandLineConfiguration'; +import { Command, CommandLineConfiguration, IParameterJson, IPhase } from '../CommandLineConfiguration'; describe(CommandLineConfiguration.name, () => { it('Forbids a misnamed phase', () => { @@ -224,35 +224,42 @@ describe(CommandLineConfiguration.name, () => { expect(parametersArray[0].longName).toEqual('--flag'); }); - it('does not allow a parameter to only be associated with phased commands but not have any associated phases', () => { - expect( - () => - new CommandLineConfiguration({ - commands: [ - { - commandKind: 'phased', - name: 'custom-phased', - summary: 'custom-phased', - enableParallelism: true, - safeForSimultaneousRushProcesses: false, - phases: ['_phase:a'] - } - ], - phases: [ - { - name: '_phase:a' - } - ], - parameters: [ - { - parameterKind: 'flag', - longName: '--flag', - associatedCommands: ['custom-phased'], - description: 'flag' - } - ] - }) - ).toThrowErrorMatchingSnapshot(); + it('allows a parameter to only be associated with phased commands but not have any associated phases', () => { + const commandLineConfiguration: CommandLineConfiguration = new CommandLineConfiguration({ + commands: [ + { + commandKind: 'phased', + name: 'custom-phased', + summary: 'custom-phased', + enableParallelism: true, + safeForSimultaneousRushProcesses: false, + phases: ['_phase:a'] + } + ], + phases: [ + { + name: '_phase:a' + } + ], + parameters: [ + { + parameterKind: 'flag', + longName: '--flag', + associatedCommands: ['custom-phased'], + description: 'flag' + } + ] + }); + + const command: Command | undefined = commandLineConfiguration.commands.get('custom-phased'); + expect(command).toBeDefined(); + const parametersArray: IParameterJson[] = Array.from(command!.associatedParameters); + expect(parametersArray).toHaveLength(1); + expect(parametersArray[0].longName).toEqual('--flag'); + const phase: IPhase | undefined = commandLineConfiguration.phases.get('_phase:a'); + expect(phase).toBeDefined(); + const phaseParametersArray: unknown[] = Array.from(phase!.associatedParameters); + expect(phaseParametersArray).toHaveLength(0); }); }); });