drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.33k stars 1.08k forks source link

HelpCLIFormatter crashes on "dirty" commands (commands that have a missing description) #6053

Open MakerTim opened 1 week ago

MakerTim commented 1 week ago

Describe the bug Tried to use --help on a few commands, but get an error about null and description

To Reproduce There are many drush commands out there that have this behaviour. proof of concept code:

  #[CLI\Command(name: 'periodical:types')]
  #[CLI\Topics(topics: ['periodical'])]
  #[CLI\Option(name: 'user', description: 'Run as user (uid)')]
  #[CLI\ValidateModulesEnabled(modules: ['periodical'])]
  public function listTypes(array $options = [
    'user' => 0,
  ]): AbstractListData {
    return new UnstructuredListData([]);
}

drush periodical:types --help I get the following warning/error

[warning] Undefined array key "description" HelpCLIFormatter.php:29
[error]  TypeError: Symfony\Component\Console\Output\Output::writeln(): Argument #1 ($messages) must be of type Traversable|array|string, null given, called in /var/www/html/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php on line 2
9 in Symfony\Component\Console\Output\Output->writeln() (line 109 of /var/www/html/vendor/symfony/console/Output/Output.php) #0 /var/www/html/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php(29): Symfony\Component\Console\Output\Output->writeln()

Expected behavior An help "page" about the given command.

Workaround Currently working with the following patch: What adds a default to description: now it is a empty line but its better than a crash.

diff --git a/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php b/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php
index 052d1351..e56f19d1 100644
--- a/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php
+++ b/vendor/drush/drush/src/Commands/help/HelpCLIFormatter.php
@@ -26,7 +26,7 @@ public function write(OutputInterface $output, $data, FormatterOptions $options)
     {
         $formatterManager = new FormatterManager();

-        $output->writeln($data['description']);
+        $output->writeln($data['description'] ?? '');
         if (array_key_exists('help', $data) && $data['help'] != $data['description']) {
             $output->writeln('');
             $output->writeln($data['help']);

System Configuration

Q A
Drush version? 12.5.1
Drupal version? 10
PHP version 8
OS? Mac/Linux/Windows

Additional information

weitzman commented 1 week ago

It's a warning not an error or a crash. I'm ambivalent about making description optional.

greg-1-anderson commented 1 week ago

Also ambivalent. Description should be required, but Drush should behave well if it isn't provided. We could emit a warning when the command is declared, and skip adding it to the application object. Not sure if we should be that strict.

MakerTim commented 1 week ago

Sorry, but it does crash.

Yes: first you run into a [warning] Undefined array key "description" HelpCLIFormatter.php:29 But after that it will call writeln with null -> that does crash

Argument #1 ($messages) must be of type Traversable|array|string, null given

I get that it's better to enforce a description, but crashing is not the way to do so if you ask me. Maybe an assert -> then development is affected but it wont on production

example where its crashing