Fix completion of choice values containing quotes, colons or spaces

.changeset/escape-completion-choice-values.md

@@ -0,0 +1,9 @@
+---
+"effect": patch
+---
+
+Fix shell completion of `Flag.choice` / `Argument.choice` values containing quotes, colons, spaces or other shell metacharacters.
+
+Choice values were interpolated verbatim into the generated completion script, which broke the script as a whole rather than just the affected parameter: zsh and fish refused to load it, bash loaded it but offered no candidates, and fish fell back to listing the current directory.
+
+Escaping the value for the enclosing quotes is not enough, because the word list is parsed a second time — `compgen -W` in bash re-expands it, and fish fully expands the `complete -a` list, so a value such as `(cmd)` was executed as a command substitution when the user pressed TAB. Bash now filters an explicitly quoted list instead of relying on `compgen -W`, while zsh and fish escape values for both rounds using a deny-by-default character class. Bash additionally requotes each match with `printf %q` so the selected value is inserted literally — except when the user has already opened a quote, where bash quotes the match itself — and trims the leading `COMP_WORDBREAKS` segment from each match, without which a value such as `node:20` was appended to the typed text rather than replacing it (`node:` + TAB produced `node:node:20`).

packages/effect/src/unstable/cli/internal/completions/bash.ts

@@ -16,6 +16,24 @@ const escapeForBash = (s: string): string => s.replace(/'/g, "'\\''")
 
 const sanitizeFunctionName = (s: string): string => s.replace(/[^a-zA-Z0-9_]/g, "_")
 
+/**
+ * Every function name `generateFunction` will emit. `sanitizeFunctionName` maps
+ * any character to `_`, so a subcommand can produce any name — the shared helper
+ * has to pick one that is provably not among them.
+ */
+const emittedFunctionNames = (
+  descriptor: Completions.CommandDescriptor,
+  parentPath: ReadonlyArray<string>,
+  names: Set<string>
+): Set<string> => {
+  const currentPath = [...parentPath, descriptor.name]
+  names.add(`_${currentPath.map(sanitizeFunctionName).join("_")}`)
+  for (const sub of descriptor.subcommands) {
+    emittedFunctionNames(sub, currentPath, names)
+  }
+  return names
+}
+
 const flagNamesForWordlist = (flag: Completions.FlagDescriptor): Array<string> => {
   const names: Array<string> = [`--${flag.name}`]
   for (const alias of flag.aliases) {
@@ -60,12 +78,95 @@ const buildFlagGroupDeclarations = (
   lines.push(``)
 }
 
-const flagValueCompletion = (type: Completions.FlagType): string | undefined => {
+/**
+ * Emit the shared choice-completion helper.
+ *
+ * `compgen -W` re-expands every word of its list, which mangles values holding
+ * quotes, spaces or glob characters, so matches are filtered from an explicitly
+ * quoted list instead. The rest of the helper deals with how readline inserts
+ * the match:
+ *
+ * - An unquoted word is replaced verbatim, so the match is escaped with
+ *   `printf %q`. Inside a quote the user opened, bash closes the quote for us
+ *   but escapes nothing, so the match is escaped for that quote context —
+ *   including the `\'` splice, since a single quote cannot be escaped within
+ *   single quotes.
+ * - Readline replaces only the text after the last COMP_WORDBREAKS character,
+ *   so that head is trimmed from every match; otherwise a value like `node:20`
+ *   is appended to what was typed rather than replacing it. Wordbreaks that are
+ *   backslash-escaped or inside quotes do not split the word, so they must not
+ *   be treated as the boundary.
+ *
+ * Prefix matching uses the dequoted word. Dequoting is best effort: it strips
+ * one opening quote and any backslash escapes, so a value whose own text
+ * contains a backslash will not match once the user types it escaped.
+ */
+const choicesHelper = (helperName: string, lines: Array<string>): void => {
+  lines.push(`${helperName}()`)
+  lines.push(`{`)
+  lines.push(`  local _cur="$1"; shift`)
+  lines.push(`  local _prefix="\${_cur#[\\"\\']}"; _prefix="\${_prefix//\\\\/}"`)
+  lines.push(`  local _open=""`)
+  lines.push(`  [[ "$_cur" == [\\"\\']* ]] && _open="\${_cur:0:1}"`)
+  lines.push(``)
+  lines.push(`  COMPREPLY=()`)
+  lines.push(`  local _choice _match`)
+  lines.push(`  for _choice in "$@"; do`)
+  lines.push(`    [[ "$_choice" == "$_prefix"* ]] || continue`)
+  lines.push(`    case "$_open" in`)
+  lines.push(`      '"')`)
+  lines.push(`        _match="\${_choice//\\\\/\\\\\\\\}"`)
+  lines.push(`        _match="\${_match//\\$/\\\\$}"`)
+  lines.push(`        _match="\${_match//\\\`/\\\\\\\`}"`)
+  lines.push(`        _match="\${_match//\\"/\\\\\\"}"`)
+  lines.push(`        ;;`)
+  lines.push(`      "'")`)
+  lines.push(`        # bare assignment: inside double quotes \\' is not an escape`)
+  lines.push(`        _match=\${_choice//\\'/\\'\\\\\\'\\'}`)
+  lines.push(`        ;;`)
+  lines.push(`      *)`)
+  lines.push(`        printf -v _match '%q' "$_choice"`)
+  lines.push(`        ;;`)
+  lines.push(`    esac`)
+  lines.push(`    COMPREPLY+=("$_match")`)
+  lines.push(`  done`)
+  lines.push(``)
+  lines.push(`  # Boundary = last wordbreak character that is neither escaped nor quoted`)
+  lines.push(`  local _i _c _quote="" _escaped=0 _cut=0`)
+  lines.push(`  for ((_i = 0; _i < \${#_cur}; _i++)); do`)
+  lines.push(`    _c="\${_cur:_i:1}"`)
+  lines.push(`    if ((_escaped)); then _escaped=0; continue; fi`)
+  lines.push(`    case "$_c" in`)
+  lines.push(`      \\\\) _escaped=1 ;;`)
+  lines.push(`      \\"|\\')`)
+  lines.push(`        if [[ -z "$_quote" ]]; then _quote="$_c"`)
+  lines.push(`        elif [[ "$_quote" == "$_c" ]]; then _quote=""`)
+  lines.push(`        fi`)
+  lines.push(`        ;;`)
+  lines.push(`      *)`)
+  lines.push(`        if [[ -z "$_quote" && "$COMP_WORDBREAKS" == *"$_c"* ]]; then _cut=$((_i + 1)); fi`)
+  lines.push(`        ;;`)
+  lines.push(`    esac`)
+  lines.push(`  done`)
+  lines.push(`  if ((_cut > 0)); then`)
+  lines.push(`    local _head="\${_cur:0:_cut}"`)
+  lines.push(`    for ((_i = 0; _i < \${#COMPREPLY[@]}; _i++)); do`)
+  lines.push(`      COMPREPLY[_i]="\${COMPREPLY[_i]#"$_head"}"`)
+  lines.push(`    done`)
+  lines.push(`  fi`)
+  lines.push(`}`)
+  lines.push(``)
+}
+
+const choiceCompletion = (helperName: string, values: ReadonlyArray<string>): string =>
+  `${helperName} "$cur" ${values.map((value) => `'${escapeForBash(value)}'`).join(" ")}`
+
+const flagValueCompletion = (type: Completions.FlagType, helperName: string): string | undefined => {
   switch (type._tag) {
     case "Boolean":
       return undefined
     case "Choice":
-      return `COMPREPLY=( $(compgen -W '${type.values.join(" ")}' -- "$cur") )`
+      return choiceCompletion(helperName, type.values)
     case "Path":
       if (type.pathType === "directory") return `COMPREPLY=( $(compgen -d -- "$cur") )`
       return `COMPREPLY=( $(compgen -f -- "$cur") )`
@@ -74,10 +175,10 @@ const flagValueCompletion = (type: Completions.FlagType): string | undefined =>
   }
 }
 
-const argCompletion = (type: Completions.ArgumentType): string | undefined => {
+const argCompletion = (type: Completions.ArgumentType, helperName: string): string | undefined => {
   switch (type._tag) {
     case "Choice":
-      return `COMPREPLY=( $(compgen -W '${type.values.join(" ")}' -- "$cur") )`
+      return choiceCompletion(helperName, type.values)
     case "Path":
       if (type.pathType === "directory") return `COMPREPLY=( $(compgen -d -- "$cur") )`
       return `COMPREPLY=( $(compgen -f -- "$cur") )`
@@ -93,7 +194,8 @@ const argCompletion = (type: Completions.ArgumentType): string | undefined => {
 const generateFunction = (
   descriptor: Completions.CommandDescriptor,
   parentPath: ReadonlyArray<string>,
-  lines: Array<string>
+  lines: Array<string>,
+  helperName: string
 ): void => {
   const currentPath = [...parentPath, descriptor.name]
   const funcName = `_${currentPath.map(sanitizeFunctionName).join("_")}`
@@ -115,7 +217,7 @@ const generateFunction = (
       for (const alias of flag.aliases) {
         longNames.push(alias.length === 1 ? `-${alias}` : `--${alias}`)
       }
-      const completion = flagValueCompletion(flag.type)
+      const completion = flagValueCompletion(flag.type, helperName)
       if (completion) {
         lines.push(`    ${longNames.join("|")})`)
         lines.push(`      ${completion}`)
@@ -171,7 +273,7 @@ const generateFunction = (
 
   // Positional argument completion
   const argsWithCompletions = descriptor.arguments.flatMap((argument, index) => {
-    const completion = argCompletion(argument.type)
+    const completion = argCompletion(argument.type, helperName)
     return completion === undefined ? [] : [{ argument, completion, index }]
   })
   if (argsWithCompletions.length > 0) {
@@ -227,7 +329,7 @@ const generateFunction = (
 
   // Recurse into subcommands
   for (const sub of descriptor.subcommands) {
-    generateFunction(sub, currentPath, lines)
+    generateFunction(sub, currentPath, lines, helperName)
   }
 }
 
@@ -238,6 +340,9 @@ export const generate = (
 ): string => {
   const lines: Array<string> = []
   const safeName = sanitizeFunctionName(executableName)
+  const taken = emittedFunctionNames(descriptor, [], new Set())
+  let helperName = `_${safeName}__choices`
+  while (taken.has(helperName)) helperName += "_"
 
   lines.push(`###-begin-${escapeForBash(executableName)}-completions-###`)
   lines.push(`#`)
@@ -263,7 +368,8 @@ export const generate = (
   lines.push(`fi`)
   lines.push(``)
 
-  generateFunction(descriptor, [], lines)
+  choicesHelper(helperName, lines)
+  generateFunction(descriptor, [], lines, helperName)
 
   lines.push(`complete -F _${safeName} ${escapeForBash(executableName)}`)
   lines.push(`###-end-${escapeForBash(executableName)}-completions-###`)

packages/effect/src/unstable/cli/internal/completions/fish.ts

@@ -12,7 +12,15 @@ import type * as Completions from "../../Completions.ts"
 // Helpers
 // ---------------------------------------------------------------------------
 
-const escapeFishString = (s: string): string => s.replace(/'/g, "\\'")
+const escapeFishString = (s: string): string => s.replace(/\\/g, "\\\\").replace(/'/g, "\\'")
+
+/**
+ * Fish expands the list given to `complete -a` — command substitution included —
+ * so a value has to survive two rounds: an inner backslash escape, then string
+ * quoting. The inner pass denies by default; keep its character class identical
+ * to `escapeZshChoice`.
+ */
+const escapeFishChoice = (s: string): string => escapeFishString(s.replace(/[^A-Za-z0-9_.,/@%+-]/g, "\\$&"))
 
 /**
  * Build a Fish condition that checks the current subcommand context.
@@ -88,7 +96,7 @@ const flagValueArgs = (type: Completions.FlagType): string | undefined => {
     case "Boolean":
       return undefined
     case "Choice":
-      return `-r -f -a '${type.values.join(" ")}'`
+      return `-r -f -a '${type.values.map(escapeFishChoice).join(" ")}'`
     case "Path":
       if (type.pathType === "directory") return `-r -F`
       return `-r -F`
@@ -101,7 +109,7 @@ const flagValueArgs = (type: Completions.FlagType): string | undefined => {
 const argValueArgs = (type: Completions.ArgumentType): string | undefined => {
   switch (type._tag) {
     case "Choice":
-      return `-r -f -a '${type.values.join(" ")}'`
+      return `-r -f -a '${type.values.map(escapeFishChoice).join(" ")}'`
     case "Path":
       return `-r -F`
     default:

packages/effect/src/unstable/cli/internal/completions/zsh.ts

@@ -14,6 +14,13 @@ import type * as Completions from "../../Completions.ts"
 
 const escapeZsh = (s: string): string => s.replace(/\\/g, "\\\\").replace(/'/g, "'\\''").replace(/:/g, "\\:")
 
+/**
+ * Values in an `_arguments` action list are backslash-tokenized after the spec
+ * itself is unquoted, so a value has to survive two rounds: an inner backslash
+ * escape, then the surrounding single quotes.
+ */
+const escapeZshChoice = (s: string): string => s.replace(/[^A-Za-z0-9_.,/@%+-]/g, "\\$&").replace(/'/g, "'\\''")
+
 const sanitize = (s: string): string => s.replace(/[^a-zA-Z0-9_]/g, "_")
 
 /**
@@ -35,7 +42,7 @@ const valueAction = (type: Completions.FlagType): string => {
     case "Boolean":
       return ""
     case "Choice":
-      return `:value:(${type.values.join(" ")})`
+      return `:value:(${type.values.map(escapeZshChoice).join(" ")})`
     case "Path":
       return type.pathType === "directory" ? `:directory:_directories` : `:file:_files`
     case "Integer":
@@ -52,7 +59,7 @@ const valueAction = (type: Completions.FlagType): string => {
 const argAction = (type: Completions.ArgumentType): string => {
   switch (type._tag) {
     case "Choice":
-      return `(${type.values.join(" ")})`
+      return `(${type.values.map(escapeZshChoice).join(" ")})`
     case "Path":
       return type.pathType === "directory" ? `_directories` : `_files`
     default:

packages/effect/test/unstable/cli/completions/completions.test.ts

@@ -61,6 +61,15 @@ const withChoices = Command.make("deploy", {
   )
 }).pipe(Command.withDescription("Deploy application"))
 
+const withTrickyChoices = Command.make("deploy", {
+  mode: Flag.choice("mode", ["it's-fine", "node:20", "with space", "(whoami)", "#tag", "$HOME"]).pipe(
+    Flag.withDescription("Deploy mode")
+  ),
+  target: Argument.choice("target", ["o'clock", "a:b", "{x,y}"]).pipe(
+    Argument.withDescription("Deployment target")
+  )
+}).pipe(Command.withDescription("Deploy application"))
+
 const withPaths = Command.make("process", {
   input: Flag.file("input").pipe(Flag.withDescription("Input file")),
   outDir: Flag.directory("output-dir").pipe(Flag.withDescription("Output directory")),
@@ -145,8 +154,8 @@ describe("Bash completions", () => {
     assert.include(script, `--verbose|-v|--no-verbose) ;;`)
     assert.include(script, `--format|-f) _skip_next=1 ;;`)
     assert.include(script, `--format=*|-f=*) ;;`)
-    assert.include(script, `0)\n      COMPREPLY=( $(compgen -W 'one' -- "$cur") )`)
-    assert.include(script, `1)\n      COMPREPLY=( $(compgen -W 'two' -- "$cur") )`)
+    assert.include(script, `0)\n      _tool__choices "$cur" 'one'`)
+    assert.include(script, `1)\n      _tool__choices "$cur" 'two'`)
   })
 
   it("generates completion function for root command", () => {
@@ -215,7 +224,47 @@ describe("Bash completions", () => {
   it("inlines choice values for choice flags", () => {
     const desc = fromCommand(withChoices)
     const script = Bash.generate("deploy", desc)
-    assert.include(script, "dev staging prod")
+    assert.include(script, `_deploy__choices "$cur" 'dev' 'staging' 'prod'`)
+  })
+
+  it("quotes choice values instead of exposing them to compgen -W re-expansion", () => {
+    const desc = fromCommand(withTrickyChoices)
+    const script = Bash.generate("deploy", desc)
+    assert.include(script, `_deploy__choices "$cur" 'it'\\''s-fine' 'node:20' 'with space' '(whoami)' '#tag' '$HOME'`)
+    assert.include(script, `_deploy__choices "$cur" 'o'\\''clock' 'a:b' '{x,y}'`)
+    assert.notInclude(script, `compgen -W 'it`)
+  })
+
+  it("escapes matches for the quoting context of the word being completed", () => {
+    const desc = fromCommand(withTrickyChoices)
+    const script = Bash.generate("deploy", desc)
+    assert.include(script, `local _prefix="\${_cur#[\\"\\']}"; _prefix="\${_prefix//\\\\/}"`)
+    assert.include(script, `[[ "$_cur" == [\\"\\']* ]] && _open="\${_cur:0:1}"`)
+    // unquoted: %q. Double quotes: escape what the shell still expands there.
+    // Single quotes: splice, since a quote cannot be escaped inside them.
+    assert.include(script, `printf -v _match '%q' "$_choice"`)
+    assert.include(script, `_match="\${_match//\\$/\\\\$}"`)
+    assert.include(script, `_match=\${_choice//\\'/\\'\\\\\\'\\'}`)
+  })
+
+  it("treats only unescaped, unquoted word-break characters as the replacement boundary", () => {
+    const desc = fromCommand(withTrickyChoices)
+    const script = Bash.generate("deploy", desc)
+    assert.include(script, `if [[ -z "$_quote" && "$COMP_WORDBREAKS" == *"$_c"* ]]; then _cut=$((_i + 1)); fi`)
+    assert.include(script, `COMPREPLY[_i]="\${COMPREPLY[_i]#"$_head"}"`)
+  })
+
+  it("picks a choices-helper name that no generated command function can collide with", () => {
+    const collidingName = Command.make("deploy", {
+      mode: Flag.choice("mode", ["a"])
+    }).pipe(
+      Command.withSubcommands([Command.make("-choices", { m: Flag.choice("m", ["b"]) })])
+    )
+    const script = Bash.generate("deploy", fromCommand(collidingName))
+    assert.include(script, "_deploy__choices()")
+    assert.include(script, "_deploy__choices_()")
+    assert.include(script, `_deploy__choices_ "$cur" 'a'`)
+    assert.notInclude(script, `_deploy__choices "$cur"`)
   })
 
   it("generates separate functions for nested subcommands", () => {
@@ -352,6 +401,13 @@ describe("Zsh completions", () => {
     assert.include(script, "(us-east eu-west ap-south)")
   })
 
+  it("escapes quotes, colons and spaces in choice values", () => {
+    const desc = fromCommand(withTrickyChoices)
+    const script = Zsh.generate("deploy", desc)
+    assert.include(script, `(it\\'\\''s-fine node\\:20 with\\ space \\(whoami\\) \\#tag \\$HOME)`)
+    assert.include(script, `(o\\'\\''clock a\\:b \\{x,y\\})`)
+  })
+
   it("uses alternative argument sets for positional arguments and subcommands", () => {
     const desc = fromCommand(withOptionalDirectoryAndSubcommands)
     const script = Zsh.generate("example", desc)
@@ -502,6 +558,13 @@ describe("Fish completions", () => {
     assert.include(script, "-r -f -a 'dev staging prod'")
   })
 
+  it("escapes quotes and spaces in choice values for re-parsing", () => {
+    const desc = fromCommand(withTrickyChoices)
+    const script = Fish.generate("deploy", desc)
+    assert.include(script, "-r -f -a 'it\\\\\\'s-fine node\\\\:20 with\\\\ space \\\\(whoami\\\\) \\\\#tag \\\\$HOME'")
+    assert.include(script, "-r -f -a 'o\\\\\\'clock a\\\\:b \\\\{x,y\\\\}'")
+  })
+
   it("uses -n conditions for nested subcommand flags", () => {
     const desc = fromCommand(withSubcommands)
     const script = Fish.generate("server", desc)