diff --git a/commands/completion.go b/commands/completion.go index 7fa80223..61d79713 100644 --- a/commands/completion.go +++ b/commands/completion.go @@ -11,6 +11,19 @@ import ( "github.com/upsun/cli/internal/config" ) +// completeCommandName is the hidden legacy (Symfony Console) command that the +// generated completion scripts call to fetch suggestions. +const completeCommandName = "_complete" + +// shellOptionReplacer rewrites the shell option of the generated completion +// scripts from Symfony's glued short form (-szsh) to the long form +// (--shell=zsh), which no argument parser can misread as a flag bundle. +var shellOptionReplacer = strings.NewReplacer( + "-szsh", "--shell=zsh", + "-sbash", "--shell=bash", + "-sfish", "--shell=fish", +) + func newCompletionCommand(cnf *config.Config) *cobra.Command { return &cobra.Command{ Use: "completion", @@ -44,7 +57,45 @@ func newCompletionCommand(cnf *config.Config) *cobra.Command { filepath.Base(pharPath), cnf.Application.Executable, ) - fmt.Fprintln(cmd.OutOrStdout(), completions) + fmt.Fprintln(cmd.OutOrStdout(), shellOptionReplacer.Replace(completions)) }, } } + +// newCompleteCommand proxies the hidden _complete command of the legacy CLI, +// which the completion scripts call to fetch suggestions. +// +// It only exists to keep Cobra from parsing those arguments. The scripts pass +// the shell as a glued short option (-szsh, -sbash, -sfish), which Cobra +// splits into single-letter flags; as every supported shell name contains an +// "h", that always produced a -h flag and the CLI printed help instead of +// completions. +func newCompleteCommand(cnf *config.Config) *cobra.Command { + return &cobra.Command{ + Use: completeCommandName, + Short: "Internal command to provide shell completion suggestions", + Hidden: true, + Args: cobra.ArbitraryArgs, + DisableFlagParsing: true, + SilenceErrors: true, + Run: func(cmd *cobra.Command, args []string) { + c := makeLegacyCLIWrapper(cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin()) + if err := c.Exec(cmd.Context(), append([]string{completeCommandName}, args...)...); err != nil { + exitWithError(err) + } + }, + } +} + +// isCompletionRequest reports whether the command was run by a completion +// script rather than by a user. Those runs must stay silent: the bash script +// captures stderr along with stdout, so any extra message ends up in the +// suggestions. +func isCompletionRequest(cmd *cobra.Command) bool { + switch cmd.Name() { + case completeCommandName, cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd: + return true + } + + return false +} diff --git a/commands/completion_test.go b/commands/completion_test.go new file mode 100644 index 00000000..aef962d5 --- /dev/null +++ b/commands/completion_test.go @@ -0,0 +1,98 @@ +package commands + +import ( + "io" + "testing" + + "github.com/platformsh/platformify/vendorization" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCompletionScriptShellOption checks that the generated completion scripts +// request suggestions with the long --shell option, instead of the glued short +// option that Symfony's templates use. +func TestCompletionScriptShellOption(t *testing.T) { + cases := []struct { + name string + template string + expected string + }{ + { + name: "zsh", + template: `requestComp="${words[0]} ${words[1]} _complete --no-interaction -szsh -a1 -c$((CURRENT-1))" i=""`, + expected: `requestComp="${words[0]} ${words[1]} _complete --no-interaction --shell=zsh -a1 -c$((CURRENT-1))" i=""`, + }, + { + name: "bash", + template: `local completecmd=("$sf_cmd" "_complete" "--no-interaction" "-sbash" "-c$cword" "-a1")`, + expected: `local completecmd=("$sf_cmd" "_complete" "--no-interaction" "--shell=bash" "-c$cword" "-a1")`, + }, + { + name: "fish", + template: `set completecmd "$sf_cmd[1]" "_complete" "--no-interaction" "-sfish" "-a1"`, + expected: `set completecmd "$sf_cmd[1]" "_complete" "--no-interaction" "--shell=fish" "-a1"`, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.expected, shellOptionReplacer.Replace(c.template)) + }) + } +} + +// TestCompleteCommandPassesArgsThrough checks that the arguments of a +// completion request reach the legacy CLI unchanged. Cobra used to split the +// glued -s option into single-letter flags, and the bundled -h made it +// print the help page instead of any completions. +func TestCompleteCommandPassesArgsThrough(t *testing.T) { + cases := []struct { + name string + args []string + }{ + { + name: "zsh", + args: []string{"--no-interaction", "-szsh", "-a1", "-c1", "-itest-cli-executable", "-ienv"}, + }, + { + name: "bash", + args: []string{"--no-interaction", "-sbash", "-c1", "-a1", "-itest-cli-executable", "-ienv"}, + }, + { + name: "fish", + args: []string{"--no-interaction", "-sfish", "-a1", "-itest-cli-executable", "-ienv"}, + }, + { + name: "long options", + args: []string{"--no-interaction", "--shell=zsh", "--api-version=1", "--current=1", "--input=test-cli-executable"}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cnf := testConfig() + root := newRootCommand(cnf, &vendorization.VendorAssets{Binary: cnf.Application.Executable}) + root.SetOut(io.Discard) + root.SetErr(io.Discard) + + var helpCalled bool + root.SetHelpFunc(func(_ *cobra.Command, _ []string) { helpCalled = true }) + + // Stub out the command so that the legacy CLI is not executed. + completeCmd, _, err := root.Find([]string{completeCommandName}) + require.NoError(t, err) + require.Equal(t, completeCommandName, completeCmd.Name()) + + var got []string + completeCmd.Run = func(_ *cobra.Command, args []string) { got = args } + + root.SetArgs(append([]string{completeCommandName}, c.args...)) + require.NoError(t, root.Execute()) + + assert.False(t, helpCalled, "the help page was printed instead of completions") + assert.Equal(t, c.args, got) + }) + } +} diff --git a/commands/root.go b/commands/root.go index 49178829..31c3c7cf 100644 --- a/commands/root.go +++ b/commands/root.go @@ -53,6 +53,10 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob SilenceUsage: true, SilenceErrors: false, PersistentPreRun: func(cmd *cobra.Command, _ []string) { + if isCompletionRequest(cmd) { + // Completions must be fast and quiet. + return + } if viper.GetBool("quiet") && !viper.GetBool("debug") && !viper.GetBool("verbose") { viper.Set("no-interaction", true) cmd.SetErr(io.Discard) @@ -93,6 +97,9 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob } }, PersistentPostRun: func(cmd *cobra.Command, _ []string) { + if isCompletionRequest(cmd) { + return + } checkShellConfigLeftovers(cmd.ErrOrStderr(), cnf) select { case rel := <-updateMessageChan: @@ -143,6 +150,7 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob // Add subcommands. cmd.AddCommand( + newCompleteCommand(cnf), newConfigInstallCommand(), newCompletionCommand(cnf), newHelpCommand(cnf), diff --git a/integration-tests/completion_test.go b/integration-tests/completion_test.go new file mode 100644 index 00000000..19a925a8 --- /dev/null +++ b/integration-tests/completion_test.go @@ -0,0 +1,69 @@ +package tests + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestCompletionScript checks that the generated completion scripts request +// suggestions with the long --shell option, rather than the glued short option +// (-szsh) that Symfony's templates use. +func TestCompletionScript(t *testing.T) { + f := newCommandFactory(t, "", "") + + cases := []struct { + shell string + }{ + {shell: "zsh"}, + {shell: "bash"}, + {shell: "fish"}, + } + + for _, c := range cases { + t.Run(c.shell, func(t *testing.T) { + script := f.Run("completion", c.shell) + assert.Contains(t, script, "--shell="+c.shell) + assert.NotContains(t, script, "-s"+c.shell) + }) + } +} + +// TestComplete checks that a completion request returns suggestions. The glued +// short options of the completion scripts must reach the legacy CLI unparsed: +// the bundled -h of -szsh used to make the CLI print its help page instead. +func TestComplete(t *testing.T) { + f := newCommandFactory(t, "", "") + + cases := []struct { + name string + args []string + expected string + }{ + { + name: "short options", + args: []string{"_complete", "--no-interaction", "-szsh", "-a1", "-c1", "-iplatform-test", "-ienv"}, + expected: "environment:list", + }, + { + name: "long options", + args: []string{ + "_complete", "--no-interaction", "--shell=zsh", "--api-version=1", "--current=1", + "--input=platform-test", "--input=env", + }, + expected: "environment:list", + }, + { + // An input token can contain an "h" too, as in "platform-test ssh --pro". + name: "input containing h", + args: []string{"_complete", "--no-interaction", "-szsh", "-a1", "-c2", "-iplatform-test", "-issh", "-i--pro"}, + expected: "--project", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Contains(t, f.Run(c.args...), c.expected) + }) + } +}