diff --git a/integration-tests/list_namespace_test.go b/integration-tests/list_namespace_test.go new file mode 100644 index 00000000..b2615eca --- /dev/null +++ b/integration-tests/list_namespace_test.go @@ -0,0 +1,173 @@ +package tests + +import ( + "os" + "path/filepath" + "regexp" + "slices" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +var commandNamePattern = regexp.MustCompile(`(?m)^\s{2}(project:[a-z0-9:-]+)`) + +// commandNames returns the command names listed in a command listing. +func commandNames(listing string) []string { + matches := commandNamePattern.FindAllStringSubmatch(listing, -1) + names := make([]string, 0, len(matches)) + for _, m := range matches { + names = append(names, m[1]) + } + sort.Strings(names) + return names +} + +// TestListNamespace checks that naming a namespace lists the same commands as +// "list ". Symfony lists the namespace itself when the name is not a +// command, but it builds its own descriptor, which describes lazily-loaded +// commands without resolving them: hidden and disabled commands were listed, +// and the formatting differed from the rest of the CLI. +func TestListNamespace(t *testing.T) { + f := newCommandFactory(t, "", "") + + viaList, _, err := f.RunCombinedOutput("list", "project") + require.NoError(t, err) + + // A namespace is not a command, so the CLI reports a failure, as before. + viaNamespace, viaNamespaceErr, _ := f.RunCombinedOutput("project") + // Symfony writes this listing to stderr. + namespaceListing := viaNamespace + viaNamespaceErr + + // The listing comes from the legacy CLI, which does not know the commands + // implemented in Go: the list command adds those to its own output. So the + // namespace listing is the list output minus the Go commands. + goCommands := []string{"project:init"} + var expected []string + for _, name := range commandNames(viaList) { + if !slices.Contains(goCommands, name) { + expected = append(expected, name) + } + } + assert.Equal(t, expected, commandNames(namespaceListing), + "naming a namespace must list the same commands as 'list '") + for _, name := range goCommands { + assert.Contains(t, commandNames(viaList), name, + "the list command must still add the commands implemented in Go") + } + + // Asking for help on a namespace lists it too, rather than reporting an + // ambiguous command name. Unlike the bare namespace, this was asked for + // explicitly, so it succeeds and writes to stdout. + viaHelp, _, err := f.RunCombinedOutput("help", "project") + require.NoError(t, err) + assert.Equal(t, commandNames(namespaceListing), commandNames(viaHelp), + "'help ' must list the same commands as the namespace itself") + assert.NotContains(t, viaHelp, "is ambiguous") + + // Hidden commands stay hidden: project:curl, and the deprecated + // project:variable:* commands, are only listed by 'list --all'. + for _, listing := range []string{namespaceListing, viaHelp} { + for _, hidden := range []string{"project:curl", "project:variable:get"} { + assert.NotContains(t, listing, hidden) + } + } + viaListAll, _, err := f.RunCombinedOutput("list", "project", "--all") + require.NoError(t, err) + assert.Contains(t, viaListAll, "project:curl", "--all must still show hidden commands") + + // The listing uses the CLI's own descriptor: aliases in parentheses, not + // Symfony's "[alias|alias]" form. + assert.Contains(t, namespaceListing, "(projects, pro)") + assert.NotContains(t, namespaceListing, "[projects|pro]") + + // An abbreviated namespace resolves as well. It used to be reported as + // ambiguous, because the hidden project:variable:* commands made + // "project:variable" count as a second namespace matching "proj". + viaAbbreviation, viaAbbreviationErr, _ := f.RunCombinedOutput("proj") + assert.Equal(t, commandNames(namespaceListing), commandNames(viaAbbreviation+viaAbbreviationErr)) + assert.NotContains(t, viaAbbreviation+viaAbbreviationErr, "is ambiguous") +} + +// TestCompletionHidesHiddenCommands checks that command name completion does not +// suggest hidden commands. Symfony skips them, but it read the hidden state from +// the LazyCommand wrapper, which does not have the one this CLI decides on. +func TestCompletionHidesHiddenCommands(t *testing.T) { + f := newCommandFactory(t, "", "") + + // The long options are what the completion scripts send. + suggestions, _, err := f.RunCombinedOutput("_complete", "--no-interaction", + "--shell=zsh", "--api-version=1", "--current=1", "--input=platform-test", "--input=") + require.NoError(t, err) + + // Each suggestion is a name and a description, separated by a tab. + lines := strings.Split(strings.TrimSpace(suggestions), "\n") + names := make([]string, 0, len(lines)) + for _, line := range lines { + names = append(names, strings.SplitN(line, "\t", 2)[0]) + } + require.Greater(t, len(names), 100, "the whole command list should be suggested") + + // Hidden commands: project:curl and its siblings, and the deprecated + // project:variable:* commands. + for _, hidden := range []string{"project:curl", "api:curl", "project:variable:get"} { + assert.NotContains(t, names, hidden, "a hidden command must not be suggested") + } + // Ordinary commands are still suggested. + for _, visible := range []string{"project:list", "environment:list"} { + assert.Contains(t, names, visible) + } +} + +// TestListNamespaceDisabledCommand checks that a command disabled by +// configuration, as a vendor distribution does, is not listed by any of the +// listing paths. +func TestListNamespaceDisabledCommand(t *testing.T) { + baseConfig, err := os.ReadFile("config.yaml") + require.NoError(t, err) + + // Disable project:create, the way a vendor configuration does. + var cnf map[string]any + require.NoError(t, yaml.Unmarshal(baseConfig, &cnf)) + application, ok := cnf["application"].(map[string]any) + require.True(t, ok, "the test config must have an application section") + application["disabled_commands"] = []string{"project:create"} + + out, err := yaml.Marshal(cnf) + require.NoError(t, err) + configPath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(configPath, out, 0o600)) + + f := newCommandFactory(t, "", "") + // A later CLI_CONFIG_FILE wins over the one testEnv sets. + f.extraEnv = []string{"CLI_CONFIG_FILE=" + configPath} + + viaList, _, err := f.RunCombinedOutput("list", "project") + require.NoError(t, err) + assert.NotContains(t, viaList, "project:create") + + viaNamespace, viaNamespaceErr, _ := f.RunCombinedOutput("project") + assert.NotContains(t, viaNamespace+viaNamespaceErr, "project:create", + "a disabled command must not be listed for its namespace") + + viaHelp, _, err := f.RunCombinedOutput("help", "project") + require.NoError(t, err) + assert.NotContains(t, viaHelp, "project:create", + "a disabled command must not be listed by 'help '") + + // It is not listed even with --all: it cannot be run at all. + viaListAll, _, err := f.RunCombinedOutput("list", "project", "--all") + require.NoError(t, err) + assert.NotContains(t, viaListAll, "project:create") + + // Sanity check: the same listing without the override does show it. + f.extraEnv = nil + plain, _, err := f.RunCombinedOutput("list", "project") + require.NoError(t, err) + assert.Contains(t, plain, "project:create") + assert.True(t, strings.Contains(plain, "project:list")) +} diff --git a/legacy/src/Application.php b/legacy/src/Application.php index 075ff5e8..68a5f100 100644 --- a/legacy/src/Application.php +++ b/legacy/src/Application.php @@ -5,6 +5,7 @@ namespace Platformsh\Cli; use Doctrine\Common\Cache\CacheProvider; +use Platformsh\Cli\Command\CommandBase; use Platformsh\Cli\Command\HelpCommand; use Platformsh\Cli\Command\ListCommand; use Platformsh\Cli\Command\WelcomeCommand; @@ -21,9 +22,13 @@ use Symfony\Component\Console\Command\Command as ConsoleCommand; use Symfony\Component\Console\Command\CompleteCommand; use Symfony\Component\Console\Command\DumpCompletionCommand; +use Symfony\Component\Console\Command\LazyCommand; use Symfony\Component\Console\CommandLoader\CommandLoaderInterface; use Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass; +use Symfony\Component\Console\Exception\CommandNotFoundException; +use Symfony\Component\Console\Exception\ExceptionInterface as ConsoleExceptionInterface; use Symfony\Component\Console\Input\ArgvInput; +use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputDefinition; use Symfony\Component\Console\Input\InputInterface; @@ -196,6 +201,95 @@ protected function getDefaultCommands(): array ]; } + /** + * @inheritdoc + * + * Resolves lazily-loaded commands, so that callers see each command's own + * hidden state. A LazyCommand reports the state of the AsCommand attribute + * it was built from, while this CLI decides on the command itself, from the + * configured hidden_commands and the command's stability. + * + * Without this, the parent lists hidden commands when describing a + * namespace, suggests them when completing a command name, and counts the + * namespaces they define. Resolving every command costs about 80ms. + * + * @see CommandBase::isHidden() + */ + public function all(?string $namespace = null): array + { + $commands = parent::all($namespace); + foreach ($commands as $name => $command) { + if ($command instanceof LazyCommand) { + $commands[$name] = $command->getCommand(); + } + } + + return $commands; + } + + /** + * @inheritdoc + * + * When the command name is a namespace rather than a command, the parent + * lists the namespace's commands using its own DescriptorHelper, which only + * knows the default descriptors. Those describe lazily-loaded commands + * without resolving them, so hidden commands (which this CLI decides on the + * command itself) and commands disabled by configuration are both listed. + * + * Run our own list command for the namespace instead, so that the output + * matches "list ". As in the parent, it is written to the error + * output and the exit code reports that no command was run. + * + * @see ListCommand + * @see \Platformsh\Cli\Console\DescriptorUtils::describeNamespaces() + */ + public function doRun(InputInterface $input, OutputInterface $output): int + { + if (($namespace = $this->getDescribableNamespace($input)) !== null) { + $listInput = new ArrayInput(['command' => 'list', 'namespace' => $namespace]); + $listInput->setInteractive(false); + $this->get('list')->run( + $listInput, + $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output, + ); + + return 1; + } + + return parent::doRun($input, $output); + } + + /** + * Returns the namespace to describe, if the input names one instead of a command. + */ + private function getDescribableNamespace(InputInterface $input): ?string + { + if ($input->hasParameterOption(['--version', '-V'], true)) { + return null; + } + + try { + // As in the parent method: this makes ArgvInput::getFirstArgument() + // able to tell an option from an argument. Errors are ignored + // because the command is not known yet. + $input->bind($this->getDefinition()); + } catch (ConsoleExceptionInterface) { + // Ignored. + } + + $name = $this->getCommandName($input); + if ($name === null || $name === '' || $this->has($name)) { + return null; + } + + try { + return $this->findNamespace($name); + } catch (CommandNotFoundException) { + // Not a namespace either: let the parent report the error. + return null; + } + } + /** * @inheritdoc */ diff --git a/legacy/src/Command/HelpCommand.php b/legacy/src/Command/HelpCommand.php index d77143aa..0756841f 100644 --- a/legacy/src/Command/HelpCommand.php +++ b/legacy/src/Command/HelpCommand.php @@ -13,8 +13,11 @@ use Platformsh\Cli\Service\Config; use Platformsh\Cli\Console\CustomMarkdownDescriptor; use Platformsh\Cli\Console\CustomTextDescriptor; +use Symfony\Component\Console\Application; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Exception\CommandNotFoundException; use Symfony\Component\Console\Exception\InvalidArgumentException; +use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; @@ -63,6 +66,11 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { + $application = $this->getApplication(); + if ($application !== null && ($namespace = $this->describableNamespace($application, $input)) !== null) { + return $this->listNamespace($application, $namespace, $input, $output); + } + $command = $this->command ?: $this->getApplication()->find($input->getArgument('command_name')); $format = $input->getOption('format'); @@ -91,4 +99,41 @@ protected function execute(InputInterface $input, OutputInterface $output): int throw new InvalidArgumentException(sprintf('Unsupported format "%s".', $format)); } + + /** + * Returns the namespace to describe, if help was asked for one instead of a command. + * + * Without this, asking for help on a namespace only reports that the name is + * ambiguous, and the suggestions it lists include hidden commands. + */ + private function describableNamespace(Application $application, InputInterface $input): ?string + { + $name = $input->getArgument('command_name'); + if ($this->command !== null || !is_string($name) || $name === '' || $application->has($name)) { + return null; + } + + try { + return $application->findNamespace($name); + } catch (CommandNotFoundException) { + // Neither a command nor a namespace: let the caller report the error. + return null; + } + } + + /** + * Lists the commands of a namespace, as the list command does. + */ + private function listNamespace(Application $application, string $namespace, InputInterface $input, OutputInterface $output): int + { + $listInput = new ArrayInput([ + 'command' => 'list', + 'namespace' => $namespace, + '--format' => $input->getOption('format'), + '--raw' => $input->getOption('raw'), + ]); + $listInput->setInteractive(false); + + return $application->get('list')->run($listInput, $output); + } }