[rush-lib] Relax association for custom parameters

microsoft · Apr 19, 2022 · ef0d7a8 · ef0d7a8
1 parent 7386ee3
commit ef0d7a8
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 46 deletions.
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
@@ -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"
+}
diff --git 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
@@ -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);
     });
   });
 });