fix(stubs): named arguments no longer go missing in silence

UPGRADE.md

@@ -22,6 +22,35 @@ vendor/bin/rector process src
 Le set est **cumulatif** : le passer une fois rattrape toutes les versions franchies d'un coup. Il
 ne contient que ce que Rector sait faire sans deviner ; tout le reste est écrit à la main ci-dessous.
 
+## Non publié
+
+### Les stubs refusent ce que PHP refuse
+
+**Qui est concerné** : toute application qui appelle un contrat d'activité, d'opération Nexus ou de
+workflow enfant par un stub. Rien à écrire ; des appels qui passaient en silence lèvent désormais,
+et c'est le but.
+
+Les trois stubs transforment les arguments reçus par `__call` en une charge nommée. Trois fautes
+d'appel y disparaissaient sans un mot, et voyageaient jusque dans le journal — où elles se rejouent
+à l'identique, passe après passe, loin de l'appel fautif :
+
+| L'appel | Avant | Maintenant | Ce que PHP fait sur l'appel ordinaire |
+|---|---|---|---|
+| argument nommé inconnu | ignoré | `BadMethodCallException` | `Error: Unknown named parameter` |
+| paramètre requis non fourni | vaut `null` | `BadMethodCallException` | `ArgumentCountError` |
+| paramètre servi en positionnel **et** en nommé | le positionnel gagne | `BadMethodCallException` | `Error: Named parameter overwrites previous argument` |
+
+Le type est `\BadMethodCallException` et non celui de PHP parce que l'appel passe par `__call` :
+c'est l'exception que la SPL réserve à une méthode appelée de travers, et elle reste rattrapable.
+
+**Si une de ces exceptions apparaît en production**, elle désigne un appel qui était déjà faux : un
+workflow enfant démarré avec un paramètre manquant partait avec `null` et attendait un message qui
+ne venait jamais. Rector ne peut rien ici — la correction est dans votre code d'appel, pas dans une
+forme mécanique.
+
+Ces exceptions sont déterministes : rejouées à l'identique à chaque redélivrance, elles brûlent les
+tentatives de Messenger jusqu'au transport d'échec. Configurez-en un.
+
 ## 0.1.0-alpha8
 
 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat

src/Durable/Activity/ActivityStub.php

@@ -5,6 +5,7 @@
 namespace Gplanchat\Durable\Activity;
 
 use Gplanchat\Durable\Awaitable\Awaitable;
+use Gplanchat\Durable\Stub\StubArguments;
 
 /**
  * Proxy de planification côté workflow.
@@ -55,14 +56,6 @@ public function __call(string $name, array $arguments): Awaitable
      */
     private function argumentsToPayload(string $methodName, array $arguments): array
     {
-        $reflection = new \ReflectionMethod($this->contractClass, $methodName);
-        $params = $reflection->getParameters();
-        $payload = [];
-        foreach ($params as $i => $param) {
-            $key = $param->getName();
-            $payload[$key] = $arguments[$i] ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null);
-        }
-
-        return $payload;
+        return StubArguments::toPayload(new \ReflectionMethod($this->contractClass, $methodName), $arguments);
     }
 }

src/Durable/Nexus/NexusStub.php

@@ -6,6 +6,7 @@
 
 use Gplanchat\Durable\Awaitable\Awaitable;
 use Gplanchat\Durable\Nexus\Serving\NexusContractResolver;
+use Gplanchat\Durable\Stub\StubArguments;
 
 /**
  * Proxy de planification côté appelant.
@@ -76,12 +77,6 @@ public function __call(string $name, array $arguments): Awaitable
      */
     private function argumentsToPayload(string $methodName, array $arguments): array
     {
-        $payload = [];
-        foreach ((new \ReflectionMethod($this->contractClass, $methodName))->getParameters() as $i => $param) {
-            $payload[$param->getName()] = $arguments[$i]
-                ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null);
-        }
-
-        return $payload;
+        return StubArguments::toPayload(new \ReflectionMethod($this->contractClass, $methodName), $arguments);
     }
 }

src/Durable/Stub/StubArguments.php

@@ -0,0 +1,122 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Gplanchat\Durable\Stub;
+
+/**
+ * Ce qu'un stub `__call` fait des arguments qu'on lui a passés.
+ *
+ * Les trois stubs — activité, opération Nexus, workflow enfant — appellent tous une méthode d'un
+ * contrat par `__call`, et doivent tous transformer les arguments reçus en une charge nommée, parce
+ * que c'est nommé que ça voyage dans le journal. Ils le faisaient chacun de leur côté, à
+ * l'identique, et avec le même défaut.
+ *
+ * **Le défaut : les arguments nommés étaient silencieusement perdus.** PHP passe les arguments
+ * nommés à `__call` dans un tableau à **clés de chaînes** ; l'appariement se faisait par indice
+ * (`$arguments[$i]`), aucun indice ne répondait, et *tous* les paramètres retombaient sur leur
+ * valeur par défaut. Sans exception, sans trace. Un workflow enfant démarré ainsi partait avec un
+ * prompt vide et attendait un message qui ne viendrait jamais.
+ *
+ * **Le second défaut, du même ordre : `??` confond « absent » et « null ».** Passer explicitement
+ * `null` à un paramètre nullable donnait sa valeur par défaut plutôt que `null` — c'est-à-dire
+ * l'inverse de ce qui était demandé. D'où `array_key_exists` plutôt que `??`.
+ *
+ * **Et ce que PHP refuse, le stub le refuse.** Sur un appel ordinaire, PHP lève sur un argument
+ * nommé inconnu (`Unknown named parameter`), sur un argument requis manquant (`ArgumentCountError`)
+ * et sur un paramètre servi deux fois, en positionnel puis en nommé (`Named parameter $x overwrites
+ * previous argument`). Un stub qui avale l'un des trois rend une faute indiscernable d'une valeur
+ * voulue — et la fait voyager jusque dans le journal, où elle sera rejouée à l'identique. Les trois
+ * lèvent donc ici aussi. Le type diffère de celui de PHP — `\BadMethodCallException` plutôt que
+ * `\Error` ou `\ArgumentCountError` — parce que l'appel passe par `__call` : c'est l'exception que
+ * la SPL réserve à une méthode appelée de travers, et elle reste rattrapable.
+ */
+final class StubArguments
+{
+    private function __construct() {}
+
+    /**
+     * @param array<int|string, mixed> $arguments tels que `__call` les a reçus : les positionnels
+     *                                            sous des indices, les nommés sous leur nom
+     *
+     * @return array<string, mixed> la charge nommée, un paramètre du contrat par clé
+     *
+     * @throws \BadMethodCallException si un argument nommé ne correspond à aucun paramètre, si un
+     *                                 paramètre requis n'est pas fourni, ou si un paramètre est
+     *                                 servi à la fois en positionnel et en nommé
+     */
+    public static function toPayload(\ReflectionFunctionAbstract $method, array $arguments): array
+    {
+        $payload = [];
+        $connus = [];
+
+        foreach ($method->getParameters() as $i => $param) {
+            $name = $param->getName();
+            $connus[$name] = true;
+
+            // Un variadique n'a ni valeur par défaut ni obligation : il ne peut pas manquer, et
+            // il n'a pas de place à lui dans une charge nommée.
+            if ($param->isVariadic()) {
+                continue;
+            }
+
+            $parPosition = \array_key_exists($i, $arguments);
+            $parNom = \array_key_exists($name, $arguments);
+
+            if ($parPosition && $parNom) {
+                throw new \BadMethodCallException(\sprintf(
+                    'Parameter $%s of %s() was given both positionally and by name.',
+                    $name,
+                    self::describe($method),
+                ));
+            }
+
+            if ($parPosition) {
+                $payload[$name] = $arguments[$i];
+
+                continue;
+            }
+
+            if ($parNom) {
+                $payload[$name] = $arguments[$name];
+
+                continue;
+            }
+
+            if ($param->isDefaultValueAvailable()) {
+                $payload[$name] = $param->getDefaultValue();
+
+                continue;
+            }
+
+            // Le retomber-sur-`null` d'ici traversait le journal sans un mot, et se rejouait à
+            // l'identique à chaque passe : le paramètre est requis, son absence est une faute
+            // d'appel, et un type non nullable l'aurait de toute façon refusée à l'arrivée.
+            throw new \BadMethodCallException(\sprintf(
+                'Missing required argument $%s for %s().',
+                $name,
+                self::describe($method),
+            ));
+        }
+
+        foreach ($arguments as $key => $_) {
+            if (\is_string($key) && !isset($connus[$key])) {
+                throw new \BadMethodCallException(\sprintf(
+                    'Unknown named parameter $%s for %s(); known parameters: %s.',
+                    $key,
+                    self::describe($method),
+                    implode(', ', array_map(static fn(string $n): string => '$' . $n, array_keys($connus))),
+                ));
+            }
+        }
+
+        return $payload;
+    }
+
+    private static function describe(\ReflectionFunctionAbstract $method): string
+    {
+        return $method instanceof \ReflectionMethod
+            ? $method->getDeclaringClass()->getName() . '::' . $method->getName()
+            : $method->getName();
+    }
+}

src/Durable/Workflow/ChildWorkflowStub.php

@@ -5,6 +5,7 @@
 namespace Gplanchat\Durable\Workflow;
 
 use Gplanchat\Durable\ChildWorkflowOptions;
+use Gplanchat\Durable\Stub\StubArguments;
 
 /**
  * Proxy de planification côté workflow pour exécuter un workflow enfant typé.
@@ -59,13 +60,6 @@ public function __call(string $name, array $arguments): \Gplanchat\Durable\Await
      */
     private function argumentsToInput(array $arguments): array
     {
-        $params = $this->workflowMethod->getParameters();
-        $input = [];
-        foreach ($params as $i => $param) {
-            $key = $param->getName();
-            $input[$key] = $arguments[$i] ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null);
-        }
-
-        return $input;
+        return StubArguments::toPayload($this->workflowMethod, $arguments);
     }
 }

tests/unit/Durable/Stub/StubArgumentsTest.php

@@ -0,0 +1,112 @@
+<?php
+
+declare(strict_types=1);
+
+namespace unit\Gplanchat\Durable\Stub;
+
+use Gplanchat\Durable\Stub\StubArguments;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Un stub `__call` transforme des arguments PHP en charge nommée. Les trois façons de se tromper
+ * sont ici, et les trois étaient silencieuses.
+ */
+#[CoversClass(StubArguments::class)]
+final class StubArgumentsTest extends TestCase
+{
+    public function testPositionalArgumentsLandOnTheirParameter(): void
+    {
+        self::assertSame(['text' => 'bonjour', 'times' => 3, 'tag' => 'défaut'], $this->map(['bonjour', 3]));
+    }
+
+    /**
+     * Le défaut qui a coûté un après-midi : PHP passe les arguments nommés à `__call` dans un
+     * tableau à **clés de chaînes**. Appariés par indice, ils disparaissaient tous — et chaque
+     * paramètre retombait sur sa valeur par défaut, sans exception ni trace.
+     */
+    public function testNamedArgumentsLandOnTheirParameter(): void
+    {
+        self::assertSame(['text' => 'bonjour', 'times' => 1, 'tag' => 'perso'], $this->map(['tag' => 'perso', 'text' => 'bonjour']));
+    }
+
+    public function testPositionalAndNamedArgumentsMix(): void
+    {
+        self::assertSame(['text' => 'bonjour', 'times' => 1, 'tag' => 'perso'], $this->map(['bonjour', 'tag' => 'perso']));
+    }
+
+    /**
+     * `??` confondait « absent » et « null » : passer explicitement `null` rendait la valeur par
+     * défaut, c'est-à-dire l'inverse de ce qui était demandé.
+     */
+    public function testAnExplicitNullIsNotTheDefault(): void
+    {
+        self::assertNull($this->map(['text' => 'x', 'tag' => null])['tag']);
+        self::assertNull($this->map(['x', 1, null])['tag']);
+    }
+
+    /**
+     * Une faute de frappe dans un nom d'argument ne doit pas être indiscernable d'une valeur par
+     * défaut voulue. PHP lève sur un appel ordinaire ; le stub aussi.
+     */
+    public function testAnUnknownNamedArgumentIsRefused(): void
+    {
+        $this->expectException(\BadMethodCallException::class);
+        $this->expectExceptionMessageMatches('/Unknown named parameter \$tags/');
+
+        $this->map(['text' => 'x', 'tags' => 'perso']);
+    }
+
+    /**
+     * Un paramètre requis non fourni lève, comme PHP lèverait `ArgumentCountError` sur l'appel
+     * ordinaire correspondant.
+     *
+     * Le laisser valoir `null` faisait voyager la faute jusque dans le journal, où elle se rejoue
+     * à l'identique à chaque passe : `$text` est déclaré `string`, une charge portant `null` est
+     * donc de toute façon refusée à l'arrivée — mais une passe de rejeu plus tard, dans un worker,
+     * loin de l'appel fautif.
+     */
+    public function testUnParametreRequisNonFourniLeve(): void
+    {
+        $this->expectException(\BadMethodCallException::class);
+        $this->expectExceptionMessageMatches('/Missing required argument \$text/');
+
+        $this->map([]);
+    }
+
+    /**
+     * Servir le même paramètre en positionnel puis en nommé : PHP refuse (« Named parameter $x
+     * overwrites previous argument »), le stub choisissait le positionnel en silence.
+     */
+    public function testUnParametreServiDeuxFoisLeve(): void
+    {
+        $this->expectException(\BadMethodCallException::class);
+        $this->expectExceptionMessageMatches('/\$text.*both positionally and by name/');
+
+        $this->map([0 => 'positionnel', 'text' => 'nommé']);
+    }
+
+    /**
+     * Le cas voisin, qui doit continuer de passer : un paramètre optionnel non fourni prend sa
+     * valeur par défaut, et un `null` explicite reste `null`.
+     */
+    public function testUnParametreOptionnelGardeSonDefautEtAccepteNull(): void
+    {
+        self::assertSame(1, $this->map(['text' => 'x'])['times']);
+        self::assertNull($this->map(['text' => 'x', 'tag' => null])['tag']);
+    }
+
+    /**
+     * @param array<int|string, mixed> $arguments
+     *
+     * @return array<string, mixed>
+     */
+    private function map(array $arguments): array
+    {
+        $contract = new class {
+            public function greet(string $text, int $times = 1, ?string $tag = 'défaut'): void {}
+        };
+
+        return StubArguments::toPayload(new \ReflectionMethod($contract, 'greet'), $arguments);
+    }
+}