Skip to content

Commit 7cdea43

Browse files
Copilotdmichon-msft
andcommitted
Refactor parameter helpers into cli/parsing directory
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
1 parent 9d7ca3a commit 7cdea43

5 files changed

Lines changed: 91 additions & 74 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import { InternalError } from '@rushstack/node-core-library';
5+
import type { CommandLineParameter } from '@rushstack/ts-command-line';
6+
7+
import type { IParameterJson, IPhase } from '../../api/CommandLineConfiguration';
8+
9+
/**
10+
* Associates command line parameters with their associated phases.
11+
* This helper is used to populate the `associatedParameters` set on each phase
12+
* based on the `associatedPhases` property of each parameter.
13+
*
14+
* @param customParameters - Map of parameter definitions to their CommandLineParameter instances
15+
* @param knownPhases - Map of phase names to IPhase objects
16+
*/
17+
export function associateParametersByPhase(
18+
customParameters: ReadonlyMap<IParameterJson, CommandLineParameter>,
19+
knownPhases: ReadonlyMap<string, IPhase>
20+
): void {
21+
for (const [parameterJson, tsCommandLineParameter] of customParameters) {
22+
if (parameterJson.associatedPhases) {
23+
for (const phaseName of parameterJson.associatedPhases) {
24+
const phase: IPhase | undefined = knownPhases.get(phaseName);
25+
if (!phase) {
26+
throw new InternalError(`Could not find a phase matching ${phaseName}.`);
27+
}
28+
phase.associatedParameters.add(tsCommandLineParameter);
29+
}
30+
}
31+
}
32+
}

libraries/rush-lib/src/utilities/CommandLineParameterHelpers.ts renamed to libraries/rush-lib/src/cli/parsing/defineCustomParameters.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,23 @@
33

44
import type { CommandLineAction, CommandLineParameter } from '@rushstack/ts-command-line';
55

6-
import type { IParameterJson } from '../api/CommandLineConfiguration';
7-
import { RushConstants } from '../logic/RushConstants';
8-
import type { ParameterJson } from '../api/CommandLineJson';
6+
import type { IParameterJson } from '../../api/CommandLineConfiguration';
7+
import { RushConstants } from '../../logic/RushConstants';
8+
import type { ParameterJson } from '../../api/CommandLineJson';
99

1010
/**
1111
* Helper function to create CommandLineParameter instances from parameter definitions.
1212
* This centralizes the logic for defining parameters based on their kind.
1313
*
1414
* @param action - The CommandLineAction to define the parameters on
1515
* @param associatedParameters - The set of parameter definitions
16-
* @returns A map from parameter longName to the created CommandLineParameter instance
16+
* @param targetMap - The map to populate with parameter definitions to CommandLineParameter instances
1717
*/
18-
export function createCommandLineParameters(
18+
export function defineCustomParameters(
1919
action: CommandLineAction,
20-
associatedParameters: Iterable<IParameterJson>
21-
): Map<string, CommandLineParameter> {
22-
const customParameters: Map<string, CommandLineParameter> = new Map();
23-
20+
associatedParameters: Iterable<IParameterJson>,
21+
targetMap: Map<IParameterJson, CommandLineParameter>
22+
): void {
2423
for (const parameter of associatedParameters) {
2524
let tsCommandLineParameter: CommandLineParameter | undefined;
2625

@@ -96,8 +95,6 @@ export function createCommandLineParameters(
9695
);
9796
}
9897

99-
customParameters.set(parameter.longName, tsCommandLineParameter);
98+
targetMap.set(parameter, tsCommandLineParameter);
10099
}
101-
102-
return customParameters;
103100
}

libraries/rush-lib/src/cli/scriptActions/BaseScriptAction.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { CommandLineParameter } from '@rushstack/ts-command-line';
55

66
import { BaseRushAction, type IBaseRushActionOptions } from '../actions/BaseRushAction';
77
import type { Command, CommandLineConfiguration, IParameterJson } from '../../api/CommandLineConfiguration';
8-
import { createCommandLineParameters } from '../../utilities/CommandLineParameterHelpers';
8+
import { defineCustomParameters } from '../parsing/defineCustomParameters';
99

1010
/**
1111
* Constructor parameters for BaseScriptAction
@@ -42,19 +42,6 @@ export abstract class BaseScriptAction<TCommand extends Command> extends BaseRus
4242
}
4343

4444
// Use the centralized helper to create CommandLineParameter instances
45-
const parametersByLongName: Map<string, CommandLineParameter> = createCommandLineParameters(
46-
this,
47-
this.command.associatedParameters
48-
);
49-
50-
// Map them by IParameterJson for internal use
51-
for (const parameter of this.command.associatedParameters) {
52-
const tsCommandLineParameter: CommandLineParameter | undefined = parametersByLongName.get(
53-
parameter.longName
54-
);
55-
if (tsCommandLineParameter) {
56-
this.customParameters.set(parameter, tsCommandLineParameter);
57-
}
58-
}
45+
defineCustomParameters(this, this.command.associatedParameters, this.customParameters);
5946
}
6047
}

libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import type { AsyncSeriesHook } from 'tapable';
55

6-
import { AlreadyReportedError, InternalError } from '@rushstack/node-core-library';
6+
import { AlreadyReportedError } from '@rushstack/node-core-library';
77
import { type ITerminal, Terminal, Colorize } from '@rushstack/terminal';
88
import type {
99
CommandLineFlagParameter,
@@ -33,6 +33,7 @@ import { SelectionParameterSet } from '../parsing/SelectionParameterSet';
3333
import type { IPhase, IPhasedCommandConfig } from '../../api/CommandLineConfiguration';
3434
import type { Operation } from '../../logic/operations/Operation';
3535
import type { OperationExecutionRecord } from '../../logic/operations/OperationExecutionRecord';
36+
import { associateParametersByPhase } from '../parsing/associateParametersByPhase';
3637
import { PhasedOperationPlugin } from '../../logic/operations/PhasedOperationPlugin';
3738
import { ShellOperationRunnerPlugin } from '../../logic/operations/ShellOperationRunnerPlugin';
3839
import { Event } from '../../api/EventHooks';
@@ -327,17 +328,8 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> i
327328

328329
this.defineScriptParameters();
329330

330-
for (const [{ associatedPhases }, tsCommandLineParameter] of this.customParameters) {
331-
if (associatedPhases) {
332-
for (const phaseName of associatedPhases) {
333-
const phase: IPhase | undefined = this._knownPhases.get(phaseName);
334-
if (!phase) {
335-
throw new InternalError(`Could not find a phase matching ${phaseName}.`);
336-
}
337-
phase.associatedParameters.add(tsCommandLineParameter);
338-
}
339-
}
340-
}
331+
// Associate parameters with their respective phases
332+
associateParametersByPhase(this.customParameters, this._knownPhases);
341333
}
342334

343335
public async runAsync(): Promise<void> {

libraries/rush-lib/src/logic/operations/test/ShellOperationRunnerPlugin.test.ts

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,15 @@
44
import path from 'node:path';
55
import { JsonFile } from '@rushstack/node-core-library';
66
import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal';
7-
import {
8-
CommandLineAction,
9-
type CommandLineParameter,
10-
type CommandLineFlagParameter,
11-
type CommandLineStringParameter,
12-
type CommandLineChoiceParameter,
13-
type CommandLineStringListParameter
14-
} from '@rushstack/ts-command-line';
7+
import { CommandLineAction, type CommandLineParameter } from '@rushstack/ts-command-line';
158

169
import { RushConfiguration } from '../../../api/RushConfiguration';
17-
import { CommandLineConfiguration, type IPhasedCommandConfig } from '../../../api/CommandLineConfiguration';
10+
import {
11+
CommandLineConfiguration,
12+
type IPhasedCommandConfig,
13+
type IParameterJson,
14+
type IPhase
15+
} from '../../../api/CommandLineConfiguration';
1816
import type { Operation } from '../Operation';
1917
import type { ICommandLineJson } from '../../../api/CommandLineJson';
2018
import { PhasedOperationPlugin } from '../PhasedOperationPlugin';
@@ -24,7 +22,8 @@ import {
2422
PhasedCommandHooks
2523
} from '../../../pluginFramework/PhasedCommandHooks';
2624
import { RushProjectConfiguration } from '../../../api/RushProjectConfiguration';
27-
import { createCommandLineParameters } from '../../../utilities/CommandLineParameterHelpers';
25+
import { defineCustomParameters } from '../../../cli/parsing/defineCustomParameters';
26+
import { associateParametersByPhase } from '../../../cli/parsing/associateParametersByPhase';
2827

2928
interface ISerializedOperation {
3029
name: string;
@@ -174,48 +173,58 @@ describe(ShellOperationRunnerPlugin.name, () => {
174173
});
175174

176175
// Create CommandLineParameter instances from the parameter definitions
177-
const customParametersMap: Map<string, CommandLineParameter> = createCommandLineParameters(
178-
action,
179-
buildCommand.associatedParameters
180-
);
176+
const customParametersMap: Map<IParameterJson, CommandLineParameter> = new Map();
177+
defineCustomParameters(action, buildCommand.associatedParameters, customParametersMap);
181178

182179
// Set values on the parameters to test filtering
180+
// Create a map by longName for easier lookup
181+
const paramsByLongName: Map<string, { param: IParameterJson; cli: CommandLineParameter }> = new Map();
182+
for (const [param, cli] of customParametersMap) {
183+
paramsByLongName.set(param.longName, { param, cli });
184+
}
185+
183186
// Set --production flag
184-
const productionParam = customParametersMap.get('--production') as CommandLineFlagParameter | undefined;
185-
if (productionParam) {
186-
(productionParam as unknown as { _setValue(value: boolean): void })._setValue(true);
187+
const production = paramsByLongName.get('--production');
188+
if (production) {
189+
(production.cli as unknown as { _setValue(value: boolean): void })._setValue(true);
187190
}
188191

189192
// Set --verbose flag
190-
const verboseParam = customParametersMap.get('--verbose') as CommandLineFlagParameter | undefined;
191-
if (verboseParam) {
192-
(verboseParam as unknown as { _setValue(value: boolean): void })._setValue(true);
193+
const verbose = paramsByLongName.get('--verbose');
194+
if (verbose) {
195+
(verbose.cli as unknown as { _setValue(value: boolean): void })._setValue(true);
193196
}
194197

195198
// Set --config parameter
196-
const configParam = customParametersMap.get('--config') as CommandLineStringParameter | undefined;
197-
if (configParam) {
198-
(configParam as unknown as { _setValue(value: string): void })._setValue('/path/to/config.json');
199+
const config = paramsByLongName.get('--config');
200+
if (config) {
201+
(config.cli as unknown as { _setValue(value: string): void })._setValue('/path/to/config.json');
199202
}
200203

201204
// Set --mode parameter
202-
const modeParam = customParametersMap.get('--mode') as CommandLineChoiceParameter | undefined;
203-
if (modeParam) {
204-
(modeParam as unknown as { _setValue(value: string): void })._setValue('prod');
205+
const mode = paramsByLongName.get('--mode');
206+
if (mode) {
207+
(mode.cli as unknown as { _setValue(value: string): void })._setValue('prod');
205208
}
206209

207210
// Set --tags parameter
208-
const tagsParam = customParametersMap.get('--tags') as CommandLineStringListParameter | undefined;
209-
if (tagsParam) {
210-
(tagsParam as unknown as { _setValue(value: string[]): void })._setValue(['tag1', 'tag2']);
211+
const tags = paramsByLongName.get('--tags');
212+
if (tags) {
213+
(tags.cli as unknown as { _setValue(value: string[]): void })._setValue(['tag1', 'tag2']);
211214
}
212215

213-
// Update the phase's associatedParameters to use our created CommandLineParameters
216+
// Associate parameters with phases using the helper
217+
// Create a map of phase names to phases for the helper
218+
const phasesMap: Map<string, IPhase> = new Map();
214219
for (const phase of buildCommand.phases) {
215-
phase.associatedParameters.clear();
216-
for (const param of customParametersMap.values()) {
217-
phase.associatedParameters.add(param);
218-
}
220+
phasesMap.set(phase.name, phase);
221+
}
222+
associateParametersByPhase(customParametersMap, phasesMap);
223+
224+
// Create customParameters map for ICreateOperationsContext (keyed by longName)
225+
const customParametersForContext: Map<string, CommandLineParameter> = new Map();
226+
for (const [param, cli] of customParametersMap) {
227+
customParametersForContext.set(param.longName, cli);
219228
}
220229

221230
const fakeCreateOperationsContext: Pick<
@@ -234,7 +243,7 @@ describe(ShellOperationRunnerPlugin.name, () => {
234243
projectsInUnknownState: new Set(rushConfiguration.projects),
235244
projectConfigurations,
236245
rushConfiguration,
237-
customParameters: customParametersMap
246+
customParameters: customParametersForContext
238247
};
239248

240249
const hooks: PhasedCommandHooks = new PhasedCommandHooks();

0 commit comments

Comments
 (0)