Skip to content

Commit

Permalink
Merge pull request #3360 from dmichon-msft/unassociated-parameters
Browse files Browse the repository at this point in the history
[rush-lib] Relax association for custom parameters
  • Loading branch information
iclanton authored Apr 23, 2022
2 parents 9152be2 + ef0d7a8 commit bd8e561
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -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"
}
18 changes: 2 additions & 16 deletions libraries/rush-lib/src/api/CommandLineConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,6 @@ export class CommandLineConfiguration {

this.parameters.push(normalizedParameter);

let parameterHasAssociatedPhases: boolean = false;

// Do some basic validation
switch (normalizedParameter.parameterKind) {
case 'choice': {
Expand All @@ -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 =
Expand All @@ -462,10 +459,6 @@ export class CommandLineConfiguration {
} else {
associatedCommand.associatedParameters.add(normalizedParameter);
parameterHasAssociatedCommands = true;

if (associatedCommand.commandKind !== RushConstants.phasedCommandKind) {
parameterIsOnlyAssociatedWithPhasedCommands = false;
}
}
}
}
Expand All @@ -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;
}
}
}
Expand All @@ -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.
}
}
}
Expand Down
67 changes: 37 additions & 30 deletions libraries/rush-lib/src/api/test/CommandLineConfiguration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
});

0 comments on commit bd8e561

Please sign in to comment.