Improve flag & option name quoting in completion generation for all 3 shells. (#767)

Improve zsh quoting.

Many quoting issues remain.

To fix them, there should first be enforced & documented limits on the acceptable characters in various values throughout SAP to avoid unnecessary quoting.

Signed-off-by: Ross Goldberg <[email protected]>

Author: rgoldberg

Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift

@@ -236,7 +236,7 @@ extension [ParsableCommand.Type] {
         if arg.isNullary { return nil }
 
         return """
-              \(arg.bashCompletionWords.joined(separator: "|")))
+              \(arg.bashCompletionWords.map { "'\($0.shellEscapeForSingleQuotedString())'" }.joined(separator: "|")))
           \(bashValueCompletion(arg).indentingEachLine(by: 8))\
                   return
                   ;;

Sources/ArgumentParser/Completions/FishCompletionsGenerator.swift

@@ -316,11 +316,11 @@ extension Name {
   fileprivate var asFishSuggestion: String {
     switch self {
     case .long(let longName):
-      return "-l \(longName)"
+      return "-l '\(longName.fishEscapeForSingleQuotedString())'"
     case .short(let shortName, _):
-      return "-s \(shortName)"
+      return "-s '\(String(shortName).fishEscapeForSingleQuotedString())'"
     case .longWithSingleDash(let dashedName):
-      return "-o \(dashedName)"
+      return "-o '\(dashedName.fishEscapeForSingleQuotedString())'"
     }
   }
 }

Sources/ArgumentParser/Completions/ZshCompletionsGenerator.swift

@@ -77,7 +77,7 @@ extension [ParsableCommand.Type] {
                 local -ar subcommands=(
         \(
           subcommands.map { """
-                        '\($0._commandName):\($0.configuration.abstract.zshEscapeForSingleQuotedExplanation())'
+                        '\($0._commandName.zshEscapeForSingleQuotedDescribeCompletion()):\($0.configuration.abstract.shellEscapeForSingleQuotedString())'
             """
           }
           .joined(separator: "\n")
@@ -146,10 +146,12 @@ extension [ParsableCommand.Type] {
       line = arg.help.options.contains(.isRepeating) ? "*" : ""
     case 1:
       line = """
-        \(arg.isRepeatingOption ? "*" : "")\(arg.names[0].synopsisString)\(arg.zshCompletionAbstract)
+        \(arg.isRepeatingOption ? "*" : "")\(arg.names[0].synopsisString.zshEscapeForSingleQuotedOptionSpec())\(arg.zshCompletionAbstract)
         """
     default:
-      let synopses = arg.names.map { $0.synopsisString }
+      let synopses = arg.names.map {
+        $0.synopsisString.zshEscapeForSingleQuotedOptionSpec()
+      }
       line = """
         \(arg.isRepeatingOption ? "*" : "(\(synopses.joined(separator: " ")))")'\
         {\(synopses.joined(separator: ","))}\
@@ -160,7 +162,10 @@ extension [ParsableCommand.Type] {
     switch arg.update {
     case .unary:
       let (argumentAction, setupScript) = argumentActionAndSetupScript(arg)
-      return ("'\(line):\(arg.valueName):\(argumentAction)'", setupScript)
+      return (
+        "'\(line):\(arg.valueName.zshEscapeForSingleQuotedOptionSpec()):\(argumentAction)'",
+        setupScript
+      )
     case .nullary:
       return ("'\(line)'", nil)
     }
@@ -179,7 +184,7 @@ extension [ParsableCommand.Type] {
         extensions.isEmpty
         ? ("_files", nil)
         : (
-          "_files -g '\\''\(extensions.map { "*.\($0.zshEscapeForSingleQuotedExplanation())" }.joined(separator: " "))'\\''",
+          "_files -g '\\''\(extensions.map { "*.\($0.shellEscapeForSingleQuotedString())" }.joined(separator: " "))'\\''",
           nil
         )
 
@@ -239,17 +244,6 @@ extension [ParsableCommand.Type] {
   }
 }
 
-extension String {
-  fileprivate func zshEscapeForSingleQuotedExplanation() -> String {
-    replacingOccurrences(
-      of: #"[\\\[\]]"#,
-      with: #"\\$0"#,
-      options: .regularExpression
-    )
-    .shellEscapeForSingleQuotedString()
-  }
-}
-
 extension ArgumentDefinition {
   /// - returns: `true` if `self` is a flag or an option and can be tab-completed multiple times in one command line.
   ///   For example, `ssh` allows the `-L` option to be given multiple times, to establish multiple port forwardings.
@@ -266,7 +260,27 @@ extension ArgumentDefinition {
   }
 
   fileprivate var zshCompletionAbstract: String {
-    guard !help.abstract.isEmpty else { return "" }
-    return "[\(help.abstract.zshEscapeForSingleQuotedExplanation())]"
+    help.abstract.isEmpty
+      ? ""
+      : "[\(help.abstract.zshEscapeForSingleQuotedOptionSpec())]"
+  }
+}
+
+extension String {
+  fileprivate func zshEscapeForSingleQuotedDescribeCompletion() -> String {
+    replacingOccurrences(
+      of: #"[:\\]"#,
+      with: #"\\$0"#,
+      options: .regularExpression
+    )
+    .shellEscapeForSingleQuotedString()
+  }
+  fileprivate func zshEscapeForSingleQuotedOptionSpec() -> String {
+    replacingOccurrences(
+      of: #"[:\\\[\]]"#,
+      with: #"\\$0"#,
+      options: .regularExpression
+    )
+    .shellEscapeForSingleQuotedString()
   }
 }

Tests/ArgumentParserExampleTests/Snapshots/testMathBashCompletionScript().bash

@@ -218,7 +218,7 @@ _math_stats_average() {
 
     # Offer option value completions
     case "${prev}" in
-    --kind)
+    '--kind')
         __math_add_completions -W 'mean'$'\n''median'$'\n''mode'
         return
         ;;
@@ -238,23 +238,23 @@ _math_stats_quantiles() {
 
     # Offer option value completions
     case "${prev}" in
-    --file)
+    '--file')
         __math_add_completions -o plusdirs -fX '!*.@(txt|md)'
         return
         ;;
-    --directory)
+    '--directory')
         __math_add_completions -d
         return
         ;;
-    --shell)
+    '--shell')
         __math_add_completions -W "$(eval 'head -100 /usr/share/dict/words | tail -50')"
         return
         ;;
-    --custom)
+    '--custom')
         __math_add_completions -W "$(__math_custom_complete ---completion stats quantiles -- --custom "${COMP_CWORD}" "$(__math_cursor_index_in_current_word)")"
         return
         ;;
-    --custom-deprecated)
+    '--custom-deprecated')
         __math_add_completions -W "$(__math_custom_complete ---completion stats quantiles -- --custom-deprecated)"
         return
         ;;

Tests/ArgumentParserExampleTests/Snapshots/testMathFishCompletionScript().fish

@@ -78,36 +78,36 @@ function __math_custom_completion
 end
 
 complete -c 'math' -f
-complete -c 'math' -n '__math_should_offer_completions_for "math"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math"' -s h -l help -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math"' -s 'h' -l 'help' -d 'Show help information.'
 complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'add' -d 'Print the sum of the values.'
 complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'multiply' -d 'Print the product of the values.'
 complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'stats' -d 'Calculate descriptive statistics.'
 complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'help' -d 'Show subcommand help information.'
-complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l hex-output -s x -d 'Use hexadecimal notation for the result.'
-complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math add"' -s h -l help -d 'Show help information.'
-complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l hex-output -s x -d 'Use hexadecimal notation for the result.'
-complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -s h -l help -d 'Show help information.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -s h -l help -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l 'hex-output' -s 'x' -d 'Use hexadecimal notation for the result.'
+complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math add"' -s 'h' -l 'help' -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l 'hex-output' -s 'x' -d 'Use hexadecimal notation for the result.'
+complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -s 'h' -l 'help' -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -s 'h' -l 'help' -d 'Show help information.'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'average' -d 'Print the average of the values.'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'stdev' -d 'Print the standard deviation of the values.'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'quantiles' -d 'Print the quantiles of the values (TBD).'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l kind -d 'The kind of average to provide.' -rfka 'mean median mode'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -s h -l help -d 'Show help information.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -s h -l help -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l 'kind' -d 'The kind of average to provide.' -rfka 'mean median mode'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -s 'h' -l 'help' -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -s 'h' -l 'help' -d 'Show help information.'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 1' -fka 'alphabet alligator branch braggart'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 2' -fka '(__math_custom_completion ---completion stats quantiles -- customArg (count (__math_tokens -pc)) (__math_tokens -tC))'
 complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 3' -fka '(__math_custom_completion ---completion stats quantiles -- customDeprecatedArg)'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l file -rfa '(set -l exts \'txt\' \'md\';for p in (string match -e -- \'*/\' (commandline -t);or printf \n)*.{$exts};printf %s\n $p;end;__fish_complete_directories (commandline -t) \'\')'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l directory -rfa '(__math_complete_directories)'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l shell -rfka '(head -100 /usr/share/dict/words | tail -50)'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l custom -rfka '(__math_custom_completion ---completion stats quantiles -- --custom (count (__math_tokens -pc)) (__math_tokens -tC))'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l custom-deprecated -rfka '(__math_custom_completion ---completion stats quantiles -- --custom-deprecated)'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l version -d 'Show the version.'
-complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -s h -l help -d 'Show help information.'
-complete -c 'math' -n '__math_should_offer_completions_for "math help"' -l version -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'file' -rfa '(set -l exts \'txt\' \'md\';for p in (string match -e -- \'*/\' (commandline -t);or printf \n)*.{$exts};printf %s\n $p;end;__fish_complete_directories (commandline -t) \'\')'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'directory' -rfa '(__math_complete_directories)'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'shell' -rfka '(head -100 /usr/share/dict/words | tail -50)'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'custom' -rfka '(__math_custom_completion ---completion stats quantiles -- --custom (count (__math_tokens -pc)) (__math_tokens -tC))'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'custom-deprecated' -rfka '(__math_custom_completion ---completion stats quantiles -- --custom-deprecated)'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'version' -d 'Show the version.'
+complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -s 'h' -l 'help' -d 'Show help information.'
+complete -c 'math' -n '__math_should_offer_completions_for "math help"' -l 'version' -d 'Show the version.'

Tests/ArgumentParserUnitTests/CompletionScriptTests.swift

@@ -88,7 +88,12 @@ extension CompletionScriptTests {
     }
 
     struct EscapedCommand: ParsableCommand {
-      @Option(help: #"Escaped chars: '[]\."#)
+      @Option(
+        name: .customLong("o:n[e"),
+        help: ArgumentHelp(
+          #"Escaped chars: '[]\."#, valueName: "path[:options]"
+        )
+      )
       var one: String
 
       @Argument(completion: .custom { _, _, _ in candidates(prefix: "i") })

Tests/ArgumentParserUnitTests/Snapshots/testBase_Bash().bash

@@ -164,33 +164,33 @@ _base_test() {
 
     # Offer option value completions
     case "${prev}" in
-    --name)
+    '--name')
         return
         ;;
-    --kind)
+    '--kind')
         __base_test_add_completions -W 'one'$'\n''two'$'\n''custom-three'
         return
         ;;
-    --other-kind)
+    '--other-kind')
         __base_test_add_completions -W 'b1_bash'$'\n''b2_bash'$'\n''b3_bash'
         return
         ;;
-    --path1)
+    '--path1')
         __base_test_add_completions -f
         return
         ;;
-    --path2)
+    '--path2')
         __base_test_add_completions -f
         return
         ;;
-    --path3)
+    '--path3')
         __base_test_add_completions -W 'c1_bash'$'\n''c2_bash'$'\n''c3_bash'
         return
         ;;
-    --rep1)
+    '--rep1')
         return
         ;;
-    -r|--rep2)
+    '-r'|'--rep2')
         return
         ;;
     esac
@@ -231,12 +231,12 @@ _base_test_sub_command() {
 
 _base_test_escaped_command() {
     flags=(-h --help)
-    options=(--one)
+    options=(--o:n[e)
     __base_test_offer_flags_options 1
 
     # Offer option value completions
     case "${prev}" in
-    --one)
+    '--o:n[e')
         return
         ;;
     esac

Tests/ArgumentParserUnitTests/Snapshots/testBase_Fish().fish

@@ -10,7 +10,7 @@ function __base-test_should_offer_completions_for -a expected_commands -a expect
         case 'sub-command'
             __base-test_parse_subcommand 0 'h/help'
         case 'escaped-command'
-            __base-test_parse_subcommand 1 'one=' 'h/help'
+            __base-test_parse_subcommand 1 'o:n[e=' 'h/help'
         case 'help'
             __base-test_parse_subcommand -r 1 
         end
@@ -68,25 +68,25 @@ function __base-test_custom_completion
 end
 
 complete -c 'base-test' -f
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l name -d 'The user\'s name.' -rfka ''
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l kind -rfka 'one two custom-three'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l other-kind -rfka 'b1_fish b2_fish b3_fish'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l path1 -rF
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l path2 -rF
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l path3 -rfka 'c1_fish c2_fish c3_fish'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l one
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l two
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l three
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l kind-counter
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l rep1 -rfka ''
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -s r -l rep2 -rfka ''
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'name' -d 'The user\'s name.' -rfka ''
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'kind' -rfka 'one two custom-three'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'other-kind' -rfka 'b1_fish b2_fish b3_fish'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'path1' -rF
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'path2' -rF
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'path3' -rfka 'c1_fish c2_fish c3_fish'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'one'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'two'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'three'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'kind-counter'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -l 'rep1' -rfka ''
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -s 'r' -l 'rep2' -rfka ''
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test" 1' -fka '(__base-test_custom_completion ---completion -- argument (count (__base-test_tokens -pc)) (__base-test_tokens -tC))'
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test" 2' -fka '(__base-test_custom_completion ---completion -- nested.nestedArgument (count (__base-test_tokens -pc)) (__base-test_tokens -tC))'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -s h -l help -d 'Show help information.'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test"' -s 'h' -l 'help' -d 'Show help information.'
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test" 3' -fa 'sub-command' -d ''
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test" 3' -fa 'escaped-command' -d ''
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test" 3' -fa 'help' -d 'Show subcommand help information.'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test sub-command"' -s h -l help -d 'Show help information.'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test escaped-command"' -l one -d 'Escaped chars: \'[]\\.' -rfka ''
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test sub-command"' -s 'h' -l 'help' -d 'Show help information.'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test escaped-command"' -l 'o:n[e' -d 'Escaped chars: \'[]\\.' -rfka ''
 complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test escaped-command" 1' -fka '(__base-test_custom_completion ---completion escaped-command -- two (count (__base-test_tokens -pc)) (__base-test_tokens -tC))'
-complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test escaped-command"' -s h -l help -d 'Show help information.'
+complete -c 'base-test' -n '__base-test_should_offer_completions_for "base-test escaped-command"' -s 'h' -l 'help' -d 'Show help information.'