Merge pull request #4405 from D4N14L/user/danade/PreventAmbiguousAbbreviations

[ts-command-line] Prevent ambiguous abbreviations of parameters defined on parent tool or action

Author: D4N14L

apps/heft/package.json

@@ -40,7 +40,6 @@
     "@rushstack/rig-package": "workspace:*",
     "@rushstack/ts-command-line": "workspace:*",
     "@types/tapable": "1.0.6",
-    "argparse": "~1.0.9",
     "chokidar": "~3.4.0",
     "fast-glob": "~3.3.1",
     "git-repo-info": "~2.1.0",
@@ -54,7 +53,6 @@
     "local-eslint-config": "workspace:*",
     "@rushstack/heft": "0.62.0",
     "@rushstack/heft-node-rig": "2.3.2",
-    "@types/argparse": "1.0.38",
     "@types/heft-jest": "1.0.1",
     "@types/node": "18.17.15",
     "@types/watchpack": "2.4.0",

apps/heft/src/cli/HeftCommandLineParser.ts

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import { ArgumentParser } from 'argparse';
 import {
   CommandLineParser,
   type AliasCommandLineAction,
@@ -20,12 +19,13 @@ import { MetricsCollector } from '../metrics/MetricsCollector';
 import { HeftConfiguration } from '../configuration/HeftConfiguration';
 import { InternalHeftSession } from '../pluginFramework/InternalHeftSession';
 import { LoggingManager } from '../pluginFramework/logging/LoggingManager';
-import { Constants } from '../utilities/Constants';
 import { CleanAction } from './actions/CleanAction';
 import { PhaseAction } from './actions/PhaseAction';
 import { RunAction } from './actions/RunAction';
 import type { IHeftActionOptions } from './actions/IHeftAction';
 import { AliasAction } from './actions/AliasAction';
+import { getToolParameterNamesFromArgs } from '../utilities/CliUtilities';
+import { Constants } from '../utilities/Constants';
 
 /**
  * This interfaces specifies values for parameters that must be parsed before the CLI
@@ -233,13 +233,11 @@ export class HeftCommandLineParser extends CommandLineParser {
       throw new InternalError('onDefineParameters() has not yet been called.');
     }
 
-    // This is a rough parsing of the --debug parameter
-    const parser: ArgumentParser = new ArgumentParser({ addHelp: false });
-    parser.addArgument(this._debugFlag.longName, { dest: 'debug', action: 'storeTrue' });
-    parser.addArgument(this._unmanagedFlag.longName, { dest: 'unmanaged', action: 'storeTrue' });
-
-    const [result]: IPreInitializationArgumentValues[] = parser.parseKnownArgs(args);
-    return result;
+    const toolParameters: Set<string> = getToolParameterNamesFromArgs(args);
+    return {
+      debug: toolParameters.has(this._debugFlag.longName),
+      unmanaged: toolParameters.has(this._unmanagedFlag.longName)
+    };
   }
 
   private async _reportErrorAndSetExitCode(error: Error): Promise<void> {

apps/heft/src/cli/actions/RunAction.ts

@@ -78,21 +78,18 @@ export function definePhaseScopingParameters(action: IHeftAction): IScopingParam
   return {
     toParameter: action.defineStringListParameter({
       parameterLongName: Constants.toParameterLongName,
-      parameterShortName: Constants.toParameterShortName,
       description: `The phase to ${action.actionName} to, including all transitive dependencies.`,
       argumentName: 'PHASE',
       parameterGroup: ScopedCommandLineAction.ScopingParameterGroup
     }),
     toExceptParameter: action.defineStringListParameter({
       parameterLongName: Constants.toExceptParameterLongName,
-      parameterShortName: Constants.toExceptParameterShortName,
       description: `The phase to ${action.actionName} to (but not include), including all transitive dependencies.`,
       argumentName: 'PHASE',
       parameterGroup: ScopedCommandLineAction.ScopingParameterGroup
     }),
     onlyParameter: action.defineStringListParameter({
       parameterLongName: Constants.onlyParameterLongName,
-      parameterShortName: Constants.onlyParameterShortName,
       description: `The phase to ${action.actionName}.`,
       argumentName: 'PHASE',
       parameterGroup: ScopedCommandLineAction.ScopingParameterGroup

apps/heft/src/startWithVersionSelector.ts

@@ -8,6 +8,7 @@
 import * as path from 'path';
 import * as fs from 'fs';
 import type { IPackageJson } from '@rushstack/node-core-library';
+import { getToolParameterNamesFromArgs } from './utilities/CliUtilities';
 import { Constants } from './utilities/Constants';
 
 const HEFT_PACKAGE_NAME: string = '@rushstack/heft';
@@ -49,14 +50,15 @@ function tryGetPackageFolderFor(resolvedFileOrFolderPath: string): string | unde
  * Use "heft --unmanaged" to bypass this feature.
  */
 function tryStartLocalHeft(): boolean {
-  if (process.argv.indexOf(Constants.unmanagedParameterLongName) >= 0) {
+  const toolParameters: Set<string> = getToolParameterNamesFromArgs();
+  if (toolParameters.has(Constants.unmanagedParameterLongName)) {
     console.log(
       `Bypassing the Heft version selector because ${JSON.stringify(Constants.unmanagedParameterLongName)} ` +
         'was specified.'
     );
     console.log();
     return false;
-  } else if (process.argv.indexOf(Constants.debugParameterLongName) >= 0) {
+  } else if (toolParameters.has(Constants.debugParameterLongName)) {
     // The unmanaged flag could be undiscoverable if it's not in their locally installed version
     console.log(
       'Searching for a locally installed version of Heft. Use the ' +

apps/heft/src/utilities/CliUtilities.ts

@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
+// See LICENSE in the project root for license information.
+
+/**
+ * Parse the arguments to the tool being executed and return the tool argument names.
+ *
+ * @param argv - The arguments to parse. Defaults to `process.argv`.
+ */
+export function getToolParameterNamesFromArgs(argv: string[] = process.argv): Set<string> {
+  const toolParameters: Set<string> = new Set();
+  // Skip the first two arguments, which are the path to the Node executable and the path to the Heft
+  // entrypoint. The remaining arguments are the tool arguments. Grab them until we reach a non-"-"-prefixed
+  // argument. We can do this simple parsing because the Heft tool only has simple optional flags.
+  for (let i: number = 2; i < argv.length; ++i) {
+    const arg: string = argv[i];
+    if (!arg.startsWith('-')) {
+      break;
+    }
+    toolParameters.add(arg);
+  }
+  return toolParameters;
+}

apps/heft/src/utilities/Constants.ts

@@ -20,18 +20,12 @@ export class Constants {
 
   public static onlyParameterLongName: string = '--only';
 
-  public static onlyParameterShortName: string = '-o';
-
   public static productionParameterLongName: string = '--production';
 
   public static toParameterLongName: string = '--to';
 
-  public static toParameterShortName: string = '-t';
-
   public static toExceptParameterLongName: string = '--to-except';
 
-  public static toExceptParameterShortName: string = '-T';
-
   public static unmanagedParameterLongName: string = '--unmanaged';
 
   public static verboseParameterLongName: string = '--verbose';

common/changes/@rushstack/heft/user-danade-PreventAmbiguousAbbreviations_2023-10-23-20-01.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/heft",
+      "comment": "Fix an issue with parsing of the \"--debug\" and \"--unmanaged\" flags for Heft",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@rushstack/heft"
+}

common/changes/@rushstack/heft/user-danade-PreventAmbiguousAbbreviations_2023-10-24-18-11.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/heft",
+      "comment": "[BREAKING CHANGE] Remove \"heft run\" short-parameters for \"--to\" (\"-t\"), \"--to-except\" (\"-T\"), and \"--only\" (\"-o\").",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/heft"
+}

common/changes/@rushstack/ts-command-line/user-danade-PreventAmbiguousAbbreviations_2023-10-23-20-01.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/ts-command-line",
+      "comment": "Consider parent tool and action parameters when determining ambiguous abbreviations. For example, if a CLI tool `mytool` has a parameter `--myparam` and an action `myaction`, then `myaction` would not accept a parameter named `--myparam` (i.e. - `mytool --myparam myaction` is valid, `mytool myaction --myparam` is not). Additionally, any parameter that can be abbreviated to `--myparam` must be uniquely provided (i.e. - `--myparam-2` can only be abbreviated to `--myparam-`, since any shorter abbreviation would be ambiguous with the original `--myparam` on the tool).",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/ts-command-line"
+}