Fixes after review

Author: maelanleborgne

src/Maker/MakeWebhook.php

@@ -13,6 +13,7 @@
 
 use Symfony\Bundle\MakerBundle\ConsoleStyle;
 use Symfony\Bundle\MakerBundle\DependencyBuilder;
+use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
 use Symfony\Bundle\MakerBundle\FileManager;
 use Symfony\Bundle\MakerBundle\Generator;
 use Symfony\Bundle\MakerBundle\InputAwareMakerInterface;
@@ -51,11 +52,16 @@
 
 /**
  * @author Maelan LE BORGNE <[email protected]>
+ *
+ * @internal
  */
 final class MakeWebhook extends AbstractMaker implements InputAwareMakerInterface
 {
+    /** @see https://regex101.com/r/S3BWkx/1 */
+    public const WEBHOOK_NAME_PATTERN = '/^[a-zA-Z_.\-\x80-\xff][a-zA-Z0-9_.\-\x80-\xff]*$/u';
     private const WEBHOOK_CONFIG_PATH = 'config/packages/webhook.yaml';
     private YamlSourceManipulator $ysm;
+    private ?string $name;
 
     public function __construct(
         private FileManager $fileManager,
@@ -83,7 +89,7 @@ public function configureCommand(Command $command, InputConfiguration $inputConf
         $inputConfig->setArgumentAsNonInteractive('name');
     }
 
-    public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null)
+    public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null): void
     {
         $dependencies->addClassDependency(
             AbstractRequestParser::class,
@@ -101,9 +107,9 @@ public function configureDependencies(DependencyBuilder $dependencies, ?InputInt
 
     public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void
     {
-        if ($input->getArgument('name')) {
-            if (!$this->verifyWebhookName($input->getArgument('name'))) {
-                throw new \InvalidArgumentException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.');
+        if ($this->name = $input->getArgument('name')) {
+            if (!$this->verifyWebhookName($this->name)) {
+                throw new RuntimeCommandException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.');
             }
 
             return;
@@ -112,58 +118,56 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
         $argument = $command->getDefinition()->getArgument('name');
         $question = new Question($argument->getDescription());
         $question->setValidator(Validator::notBlank(...));
-        $webhookName = $io->askQuestion($question);
 
-        while (!$this->verifyWebhookName($webhookName)) {
+        $this->name = $io->askQuestion($question);
+        while (!$this->verifyWebhookName($this->name)) {
             $io->error('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.');
-            $webhookName = $io->askQuestion($question);
+            $this->name = $io->askQuestion($question);
         }
-
-        $input->setArgument('name', $webhookName);
-    }
-
-    private function verifyWebhookName(string $entityName): bool
-    {
-        return preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_.\-\x7f-\xff]*$/', $entityName);
     }
 
     public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void
     {
-        $webhookName = $input->getArgument('name');
         $requestParserDetails = $this->generator->createClassNameDetails(
-            Str::asClassName($webhookName.'RequestParser'),
+            Str::asClassName($this->name.'RequestParser'),
             'Webhook\\'
         );
         $remoteEventHandlerDetails = $this->generator->createClassNameDetails(
-            Str::asClassName($webhookName.'WebhookHandler'),
+            Str::asClassName($this->name.'WebhookHandler'),
             'RemoteEvent\\'
         );
 
-        $this->addToYamlConfig($webhookName, $requestParserDetails);
+        $this->addToYamlConfig($this->name, $requestParserDetails);
 
         $this->generateRequestParser($io, $requestParserDetails);
 
         $this->generator->generateClass(
             $remoteEventHandlerDetails->getFullName(),
             'webhook/WebhookHandler.tpl.php',
             [
-                'webhook_name' => $webhookName,
+                'webhook_name' => $this->name,
             ]
         );
 
         $this->generator->writeChanges();
         $this->fileManager->dumpFile(self::WEBHOOK_CONFIG_PATH, $this->ysm->getContents());
     }
 
+    private function verifyWebhookName(string $entityName): bool
+    {
+        return preg_match(self::WEBHOOK_NAME_PATTERN, $entityName);
+    }
+
     private function addToYamlConfig(string $webhookName, ClassNameDetails $requestParserDetails): void
     {
-        if (!$this->fileManager->fileExists(self::WEBHOOK_CONFIG_PATH)) {
-            $yamlConfig = Yaml::dump(['framework' => ['webhook' => ['routing' => []]]], 4, 2);
-        } else {
+        $yamlConfig = Yaml::dump(['framework' => ['webhook' => ['routing' => []]]], 4, 2);
+        if ($this->fileManager->fileExists(self::WEBHOOK_CONFIG_PATH)) {
             $yamlConfig = $this->fileManager->getFileContents(self::WEBHOOK_CONFIG_PATH);
         }
+
         $this->ysm = new YamlSourceManipulator($yamlConfig);
         $arrayConfig = $this->ysm->getData();
+
         if (\array_key_exists($webhookName, $arrayConfig['framework']['webhook']['routing'] ?? [])) {
             throw new \InvalidArgumentException('A webhook with this name already exists');
         }
@@ -180,7 +184,7 @@ private function addToYamlConfig(string $webhookName, ClassNameDetails $requestP
     /**
      * @throws \Exception
      */
-    public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void
+    private function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void
     {
         $useStatements = new UseStatementGenerator([
             JsonException::class,
@@ -191,6 +195,7 @@ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reques
             RejectWebhookException::class,
             RequestMatcherInterface::class,
         ]);
+
         $requestMatchers = [];
         while (true) {
             $newRequestMatcher = $this->askForNextRequestMatcher($io, $requestMatchers, $requestParserDetails->getFullName(), empty($requestMatchers));
@@ -199,12 +204,14 @@ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reques
             }
             $requestMatchers[] = $newRequestMatcher;
         }
-        $useChainRequestsMatcher = false;
+
         // Use a ChainRequestMatcher if multiple matchers have been added OR if none (will be printed with an empty array)
+        $useChainRequestsMatcher = false;
         if (1 !== \count($requestMatchers)) {
             $useChainRequestsMatcher = true;
             $useStatements->addUseStatement(ChainRequestMatcher::class);
         }
+
         $requestMatcherArguments = [];
         foreach ($requestMatchers as $requestMatcherClass) {
             $useStatements->addUseStatement($requestMatcherClass);
@@ -245,6 +252,7 @@ private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers
             $choices = array_diff($availableMatchers, $addedMatchers);
             $question = new ChoiceQuestion($questionText, array_values(['<skip>'] + $choices), 0);
             $matcherName = $io->askQuestion($question);
+
             if ('<skip>' === $matcherName) {
                 return null;
             }

tests/Maker/MakeWebhookTest.php

@@ -24,7 +24,7 @@ protected function getMakerClass(): string
 
     public function getTestDetails(): \Generator
     {
-        yield 'it_makes_webhook_whit_no_prior_config_file' => [$this->createMakerTest()
+        yield 'it_makes_webhook_with_no_prior_config_file' => [$this->createMakerTest()
             ->run(function (MakerTestRunner $runner) {
                 $output = $runner->runMaker([
                     // webhook name