Skip to content

Commit 68aed51

Browse files
author
Marilena Ruffelaere
committed
fix: reconcile the config mapping callback type between BuilderStack and BuilderConfigurator
BuilderStack declared the callback as `array<array-key, string>|null` while BuilderConfigurator declared it as `(\Closure(mixed): mixed)|null`. Both describe the same array, dumped into the container by SensiolabsGotenbergExtension, so one of them had to be wrong — the Closure one was: BuilderStack stores array callables (`[$enumClass, 'from']`, `[Unit::class, 'parse']`), and the container cannot dump a closure at all (PhpDumper throws on any object argument). PHPStan never noticed because the two types only met at compile time, outside analysed code. Both sides now share a single `BuilderConfigurationMapping` phpstan-type declared on BuilderConfigurator, honest about what is stored: a static method target on a backed enum. The invocation destructures it so PHPStan can follow the static call instead of having to prove an array literal is callable — behaviour is identical, PHP resolved `[$class, $method]($value)` the same way. BuilderConfiguratorTest builds the mapping with BuilderStack and feeds it straight into BuilderConfigurator, so the two shapes now meet in analysed code and cannot silently drift apart again. Assisted-by: Claude Code:Opus-5
1 parent 8ceaa92 commit 68aed51

3 files changed

Lines changed: 85 additions & 5 deletions

File tree

src/Configurator/BuilderConfigurator.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,21 @@
44

55
use Sensiolabs\GotenbergBundle\Builder\BuilderInterface;
66

7+
/**
8+
* The mapping is built at compile time by {@see \Sensiolabs\GotenbergBundle\DependencyInjection\BuilderStack} and
9+
* dumped into the container, so a callback can only be a static method target — the container cannot dump a closure.
10+
*
11+
* @phpstan-type BuilderConfigurationMapping array<class-string<BuilderInterface>, array<string, array{
12+
* method: string,
13+
* mustUseVariadic: bool,
14+
* callback: array{class-string<\BackedEnum>, string}|null,
15+
* }>>
16+
*/
717
final class BuilderConfigurator
818
{
919
/**
10-
* @param array<class-string<BuilderInterface>, array<string, array{'method': string, 'mustUseVariadic': bool, 'callback': (\Closure(mixed): mixed)|null}>> $configurations
11-
* @param array<class-string<BuilderInterface>, array<string, mixed>> $values
20+
* @param BuilderConfigurationMapping $configurations
21+
* @param array<class-string<BuilderInterface>, array<string, mixed>> $values
1222
*/
1323
public function __construct(
1424
private readonly array $configurations,
@@ -28,7 +38,8 @@ public function __invoke(BuilderInterface $builder): void
2838
}
2939

3040
if (null !== $configurationMap['callback']) {
31-
$value = $configurationMap['callback']($value);
41+
[$callbackClass, $callbackMethod] = $configurationMap['callback'];
42+
$value = $callbackClass::$callbackMethod($value);
3243
}
3344

3445
if (\is_array($value) && true === $configurationMap['mustUseVariadic']) {

src/DependencyInjection/BuilderStack.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@
55
use Sensiolabs\GotenbergBundle\Builder\Attributes\WithBuilderConfiguration;
66
use Sensiolabs\GotenbergBundle\Builder\Attributes\WithConfigurationNode;
77
use Sensiolabs\GotenbergBundle\Builder\BuilderInterface;
8+
use Sensiolabs\GotenbergBundle\Configurator\BuilderConfigurator;
89
use Sensiolabs\GotenbergBundle\Enumeration\Unit;
910
use Sensiolabs\GotenbergBundle\NodeBuilder\ArrayNodeBuilder;
1011
use Sensiolabs\GotenbergBundle\NodeBuilder\NativeEnumNodeBuilder;
1112
use Sensiolabs\GotenbergBundle\NodeBuilder\NodeBuilderInterface;
1213
use Sensiolabs\GotenbergBundle\NodeBuilder\UnitNodeBuilder;
1314

1415
/**
16+
* @phpstan-import-type BuilderConfigurationMapping from BuilderConfigurator
17+
*
1518
* @internal
1619
*/
1720
final class BuilderStack
@@ -27,7 +30,7 @@ final class BuilderStack
2730
private array $typeReverseMapping = [];
2831

2932
/**
30-
* @var array<class-string<BuilderInterface>, array<string, array{'method': string, 'mustUseVariadic': bool, 'callback': array<array-key, string>|null}>>
33+
* @var BuilderConfigurationMapping
3134
*/
3235
private array $configMapping = [];
3336

@@ -112,7 +115,7 @@ public function getTypeReverseMapping(): array
112115
}
113116

114117
/**
115-
* @return array<class-string<BuilderInterface>, array<string, array{'method': string, 'mustUseVariadic': bool, 'callback': array<array-key, string>|null}>>
118+
* @return BuilderConfigurationMapping
116119
*/
117120
public function getConfigMapping(): array
118121
{
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
3+
namespace Sensiolabs\GotenbergBundle\Tests\Configurator;
4+
5+
use Sensiolabs\GotenbergBundle\Builder\Pdf\UrlPdfBuilder;
6+
use Sensiolabs\GotenbergBundle\Configurator\BuilderConfigurator;
7+
use Sensiolabs\GotenbergBundle\DependencyInjection\BuilderStack;
8+
use Sensiolabs\GotenbergBundle\Test\Builder\GotenbergBuilderTestCase;
9+
use Symfony\Component\Routing\Generator\UrlGenerator;
10+
use Symfony\Component\Routing\RequestContext;
11+
use Symfony\Component\Routing\RouteCollection;
12+
13+
/**
14+
* The mapping the extension dumps into the container is the very one the configurator consumes. Building it with
15+
* BuilderStack and feeding it straight into BuilderConfigurator makes both declared shapes meet in analysed code,
16+
* so they cannot drift apart again without PHPStan noticing.
17+
*
18+
* @extends GotenbergBuilderTestCase<UrlPdfBuilder>
19+
*/
20+
final class BuilderConfiguratorTest extends GotenbergBuilderTestCase
21+
{
22+
protected function createBuilder(): UrlPdfBuilder
23+
{
24+
$this->container->set('router', new UrlGenerator(new RouteCollection(), new RequestContext()));
25+
26+
return (new UrlPdfBuilder())->url('https://example.com');
27+
}
28+
29+
public function testEnumConfigurationValuesAreConvertedThroughTheirEnumClass(): void
30+
{
31+
$this->configure([
32+
'pdf_format' => 'PDF/A-1b',
33+
'emulated_media_type' => 'screen',
34+
]);
35+
36+
$this->getBuilder()->generate();
37+
38+
$this->assertGotenbergFormData('pdfa', 'PDF/A-1b');
39+
$this->assertGotenbergFormData('emulatedMediaType', 'screen');
40+
}
41+
42+
public function testUnitConfigurationValuesAreParsedAndSpreadOverTheMethodArguments(): void
43+
{
44+
$this->configure([
45+
'paper_width' => '21cm',
46+
'margin_top' => 4.5,
47+
]);
48+
49+
$this->getBuilder()->generate();
50+
51+
$this->assertGotenbergFormData('paperWidth', '21cm');
52+
$this->assertGotenbergFormData('marginTop', '4.5in');
53+
}
54+
55+
/**
56+
* @param array<string, mixed> $values
57+
*/
58+
private function configure(array $values): void
59+
{
60+
$builderStack = new BuilderStack();
61+
$builderStack->push(UrlPdfBuilder::class);
62+
63+
$configurator = new BuilderConfigurator($builderStack->getConfigMapping(), [UrlPdfBuilder::class => $values]);
64+
$configurator($this->getBuilder());
65+
}
66+
}

0 commit comments

Comments
 (0)