fix(commander): match commander options scope behavior in different modes

.github/workflows/shell-completions.yml

@@ -11,7 +11,7 @@ on:
 jobs:
   shell-argv-protocol:
     name: Shell argv protocol
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
 
     steps:
       - name: Checkout

src/commander.ts

@@ -1,16 +1,28 @@
 import type { Command as CommanderCommand } from 'commander';
-import t, { Command as TabCommand, type RootCommand, OptionHandler } from './t';
+import t, {
+  type Command as TabCommand,
+  type RootCommand,
+  type OptionHandler,
+} from './t';
 
 // rawArgs is available on (just) the Commander root command, but is not included in the TypeScript types.
 interface CommandWithRawArgs extends CommanderCommand {
   rawArgs: string[];
 }
 
+// Commander does not expose a getter for this setting.
+interface CommandWithPositionalOptions extends CommanderCommand {
+  _enablePositionalOptions: boolean;
+}
+
 export default function tab(
   instance: CommanderCommand,
   completionConfig?: { completionCommandName?: string }
 ): RootCommand {
   const programName = instance.name();
+  const positionalOptions = (instance as CommandWithPositionalOptions)
+    ._enablePositionalOptions;
+  t.optionLookupMode = positionalOptions ? 'local' : 'parent-first';
 
   // Make a `completion` command with a required command-argument.
   const completionCommandName =

src/t.ts

@@ -140,18 +140,68 @@ export class RootCommand extends Command {
   commands = new Map<string, Command>();
   completions: Completion[] = [];
   directive = ShellCompDirective.ShellCompDirectiveDefault;
+  optionLookupMode?: 'parent-first' | 'local';
 
   constructor() {
     super('', '');
   }
 
   command(value: string, description: string) {
     const c = new Command(value, description);
+    const parentPath = value.split(' ').slice(0, -1).join(' ');
+    c.parent = parentPath ? this.commands.get(parentPath) : this;
     this.commands.set(value, c);
     return c;
   }
 
+  private scanArguments(args: string[]): {
+    command: Command;
+    parts: string[];
+  } {
+    const parts: string[] = [];
+    let matchedCommand: Command | undefined;
+    let canMatchCommand = true;
+    let i = 0;
+
+    while (i < args.length) {
+      const arg = args[i];
+
+      if (arg.startsWith('-')) {
+        i++;
+
+        const option = this.findOptionInScope(matchedCommand ?? this, arg);
+        const isBoolean = option?.isBoolean ?? false;
+
+        // skip the next argument if this is not a boolean option and the next arg doesn't start with -
+        if (!isBoolean && i < args.length && !args[i].startsWith('-')) {
+          i++;
+        }
+      } else {
+        parts.push(arg);
+
+        if (canMatchCommand) {
+          const commandPath = parts.join(' ');
+          const command = this.commands.get(commandPath);
+
+          if (command) {
+            matchedCommand = command;
+          } else {
+            canMatchCommand = false;
+          }
+        }
+
+        i++;
+      }
+    }
+
+    return { command: matchedCommand ?? this, parts };
+  }
+
   private stripOptions(args: string[]): string[] {
+    if (this.optionLookupMode) {
+      return this.scanArguments(args).parts;
+    }
+
     const parts: string[] = [];
     let i = 0;
 
@@ -191,6 +241,13 @@ export class RootCommand extends Command {
   }
 
   private matchCommand(args: string[]): [Command, string[]] {
+    if (this.optionLookupMode) {
+      const { command, parts } = this.scanArguments(args);
+      const commandPartCount =
+        command === this ? 0 : command.value.split(' ').length;
+      return [command, parts.slice(commandPartCount)];
+    }
+
     args = this.stripOptions(args);
     const parts: string[] = [];
     let remaining: string[] = [];
@@ -213,6 +270,7 @@ export class RootCommand extends Command {
   }
 
   private shouldCompleteFlags(
+    command: Command,
     lastPrevArg: string | undefined,
     toComplete: string
   ): boolean {
@@ -222,12 +280,18 @@ export class RootCommand extends Command {
 
     // previous argument was an option, check if it expects a value
     if (lastPrevArg?.startsWith('-')) {
-      let option = this.findOption(this, lastPrevArg);
-      if (!option) {
-        // subcommand options
-        for (const [, command] of this.commands) {
-          option = this.findOption(command, lastPrevArg);
-          if (option) break;
+      let option: Option | undefined;
+
+      if (this.optionLookupMode) {
+        option = this.findOptionInScope(command, lastPrevArg);
+      } else {
+        option = this.findOption(this, lastPrevArg);
+        if (!option) {
+          // subcommand options
+          for (const [, candidate] of this.commands) {
+            option = this.findOption(candidate, lastPrevArg);
+            if (option) break;
+          }
         }
       }
 
@@ -261,21 +325,21 @@ export class RootCommand extends Command {
       const [flag] = toComplete.split('=');
       optionName = flag;
     } else if (lastPrevArg?.startsWith('-')) {
-      const option = this.findOption(command, lastPrevArg);
+      const option = this.findOptionForCompletion(command, lastPrevArg);
       if (option && !option.isBoolean) {
         optionName = lastPrevArg;
       }
     }
 
     if (optionName) {
-      const option = this.findOption(command, optionName);
+      const option = this.findOptionForCompletion(command, optionName);
       if (option?.handler) {
         const suggestions: Completion[] = [];
         option.handler.call(
           option,
           (value: string, description: string) =>
             suggestions.push({ value, description }),
-          command.options
+          option.command.options
         );
 
         this.completions = suggestions;
@@ -288,26 +352,67 @@ export class RootCommand extends Command {
         toComplete.startsWith('-') && !toComplete.startsWith('--');
       const cleanToComplete = toComplete.replace(/^-+/, '');
 
-      for (const [name, option] of command.options) {
-        if (
-          isShortFlag &&
-          option.alias &&
-          `-${option.alias}`.startsWith(toComplete)
-        ) {
-          this.completions.push({
-            value: `-${option.alias}`,
-            description: option.description,
-          });
-        } else if (!isShortFlag && name.startsWith(cleanToComplete)) {
-          this.completions.push({
-            value: `--${name}`,
-            description: option.description,
-          });
+      for (const optionCommand of this.optionCommandsFor(command)) {
+        for (const [name, option] of optionCommand.options) {
+          if (
+            isShortFlag &&
+            option.alias &&
+            `-${option.alias}`.startsWith(toComplete)
+          ) {
+            this.completions.push({
+              value: `-${option.alias}`,
+              description: option.description,
+            });
+          } else if (!isShortFlag && name.startsWith(cleanToComplete)) {
+            this.completions.push({
+              value: `--${name}`,
+              description: option.description,
+            });
+          }
         }
       }
     }
   }
 
+  private optionCommandsFor(command: Command): Command[] {
+    if (!this.optionLookupMode) {
+      return [command];
+    }
+
+    if (this.optionLookupMode === 'local') {
+      return [command];
+    }
+
+    const commands: Command[] = [];
+    let candidate: Command | undefined = command;
+    while (candidate) {
+      commands.unshift(candidate);
+      candidate = candidate.parent;
+    }
+    return commands;
+  }
+
+  private findOptionInScope(
+    command: Command,
+    optionName: string
+  ): Option | undefined {
+    for (const candidate of this.optionCommandsFor(command)) {
+      const option = this.findOption(candidate, optionName);
+      if (option) return option;
+    }
+    return undefined;
+  }
+
+  private findOptionForCompletion(
+    command: Command,
+    optionName: string
+  ): Option | undefined {
+    if (this.optionLookupMode) {
+      return this.findOptionInScope(command, optionName);
+    }
+    return this.findOption(command, optionName);
+  }
+
   // find option by name or alias
   private findOption(command: Command, optionName: string): Option | undefined {
     let option = command.options.get(optionName);
@@ -432,7 +537,7 @@ export class RootCommand extends Command {
     const [matchedCommand] = this.matchCommand(previousArgs);
     const lastPrevArg = previousArgs[previousArgs.length - 1];
 
-    if (this.shouldCompleteFlags(lastPrevArg, toComplete)) {
+    if (this.shouldCompleteFlags(matchedCommand, lastPrevArg, toComplete)) {
       this.handleFlagCompletion(
         matchedCommand,
         previousArgs,

tests/commander-option-scope.test.ts

@@ -0,0 +1,111 @@
+import { execFile } from 'node:child_process';
+import { createRequire } from 'node:module';
+import { promisify } from 'node:util';
+import { describe, expect, it } from 'vitest';
+
+const execFileAsync = promisify(execFile);
+const require = createRequire(import.meta.url);
+const tsxCli = require.resolve('tsx/cli');
+const fixture = 'tests/fixtures/commander-option-scope.ts';
+
+async function complete(
+  args: string[],
+  positionalOptions: boolean
+): Promise<string[]> {
+  const { stdout } = await execFileAsync(
+    process.execPath,
+    [tsxCli, fixture, ...args],
+    {
+      cwd: process.cwd(),
+      env: {
+        ...process.env,
+        COMMANDER_POSITIONAL: positionalOptions ? '1' : '0',
+      },
+    }
+  );
+
+  return stdout
+    .split('\n')
+    .filter((line) => line && !line.startsWith(':'))
+    .map((line) => line.split('\t')[0]);
+}
+
+describe('Commander option scope', () => {
+  describe('default options', () => {
+    it('uses the root option when a subcommand reuses its short flag', async () => {
+      await expect(complete(['serve', '-c', ''], false)).resolves.toEqual([
+        'top',
+        'global',
+      ]);
+    });
+
+    it('keeps a non-conflicting serve long option available', async () => {
+      await expect(
+        complete(['serve', '--serve-choice', ''], false)
+      ).resolves.toEqual(['serve', 'local']);
+    });
+
+    it('keeps root option values available after a subcommand', async () => {
+      await expect(
+        complete(['serve', '--root-choice', ''], false)
+      ).resolves.toEqual(['top', 'global']);
+    });
+
+    it('suggests root option names after a subcommand', async () => {
+      await expect(complete(['serve', '--root-c'], false)).resolves.toEqual([
+        '--root-choice',
+      ]);
+    });
+
+    it('uses root option arity for a colliding short flag', async () => {
+      const values = await complete(['serve', '-b', ''], false);
+
+      expect(values).toEqual(expect.arrayContaining(['prod', 'staging']));
+      expect(values).not.toEqual(expect.arrayContaining(['blue', 'green']));
+    });
+
+    it('uses an ancestor option before a nested command option', async () => {
+      await expect(
+        complete(['serve', 'nested', '-a', ''], false)
+      ).resolves.toEqual(['parent', 'ancestor']);
+    });
+  });
+
+  describe('positional options', () => {
+    it('uses the subcommand option when it reuses a root short flag', async () => {
+      await expect(complete(['serve', '-c', ''], true)).resolves.toEqual([
+        'serve',
+        'local',
+      ]);
+    });
+
+    it('does not complete root option values after a subcommand', async () => {
+      await expect(
+        complete(['serve', '--root-choice', ''], true)
+      ).resolves.toEqual([]);
+    });
+
+    it('does not suggest root option names after a subcommand', async () => {
+      await expect(complete(['serve', '--root-c'], true)).resolves.toEqual([]);
+    });
+
+    it('still accepts root options before the subcommand', async () => {
+      await expect(
+        complete(['--root-choice', 'top', 'serve', '-c', ''], true)
+      ).resolves.toEqual(['serve', 'local']);
+    });
+
+    it('uses serve option arity for a colliding short flag', async () => {
+      await expect(complete(['serve', '-b', ''], true)).resolves.toEqual([
+        'blue',
+        'green',
+      ]);
+    });
+
+    it('limits nested lookup to the matched command', async () => {
+      await expect(
+        complete(['serve', 'nested', '-a', ''], true)
+      ).resolves.toEqual(['deep', 'nested']);
+    });
+  });
+});