Skip to content

Commit 90ab8fb

Browse files
shahinataei-tsruudk
shahinataei-ts
authored and
ruudk
committed
Fix: Pass builtInOnly parameter correctly for list types
When processing ListOfType in mapGraphQLTypeToPHPType, the $builtInOnly parameter was incorrectly passed as the second argument ($nullable) instead of the third argument ($builtInOnly). This caused the $builtInOnly parameter to default to false when processing list items, resulting in the PHP mapped type being used instead of the primitive JSON type in $data parameter docblocks. Example of the bug: - GraphQL: currencies: [Currency!]! - Config: withScalar('Currency', Type::string(), Type::object(Currency::class)) - Generated docblock: 'currencies': list<Currency> (wrong) - Should be: 'currencies': list<string> (correct) This caused PHPStan errors like: "Parameter #1 $code of class Money\Currency constructor expects non-empty-string, Money\Currency given."
1 parent 279fc05 commit 90ab8fb

File tree

9 files changed

+261
-1
lines changed

9 files changed

+261
-1
lines changed

‎src/TypeMapper.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function mapGraphQLTypeToPHPType(Type $type, ?bool $nullable = null, bool
9393
}
9494

9595
if ($type instanceof ListOfType) {
96-
return SymfonyType::list($this->mapGraphQLTypeToPHPType($type->getWrappedType(), $builtInOnly));
96+
return SymfonyType::list($this->mapGraphQLTypeToPHPType($type->getWrappedType(), null, $builtInOnly));
9797
}
9898

9999
if ($type instanceof ScalarType) {

‎tests/ListScalar/Generated/Query/Test/Data.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test;
6+
7+
use Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test\Data\Wallet;
8+
use Ruudk\GraphQLCodeGenerator\ListScalar\ValueObjects\Currency;
9+
10+
// This file was automatically generated and should not be edited.
11+
12+
final class Data
13+
{
14+
/**
15+
* @var list<Currency>
16+
*/
17+
public array $supportedCurrencies {
18+
get => $this->supportedCurrencies ??= array_map(fn($item) => new Currency($item), $this->data['supportedCurrencies']);
19+
}
20+
21+
public Wallet $wallet {
22+
get => $this->wallet ??= new Wallet($this->data['wallet']);
23+
}
24+
25+
/**
26+
* @var list<Error>
27+
*/
28+
public readonly array $errors;
29+
30+
/**
31+
* @param array{
32+
* 'supportedCurrencies': list<string>,
33+
* 'wallet': array{
34+
* 'currencies': list<string>,
35+
* 'name': string,
36+
* },
37+
* } $data
38+
* @param list<array{
39+
* 'code': string,
40+
* 'debugMessage'?: string,
41+
* 'message': string,
42+
* }> $errors
43+
*/
44+
public function __construct(
45+
private readonly array $data,
46+
array $errors,
47+
) {
48+
$this->errors = array_map(fn(array $error) => new Error($error), $errors);
49+
}
50+
}

‎tests/ListScalar/Generated/Query/Test/Data/Wallet.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test\Data;
6+
7+
use Ruudk\GraphQLCodeGenerator\ListScalar\ValueObjects\Currency;
8+
9+
// This file was automatically generated and should not be edited.
10+
11+
final class Wallet
12+
{
13+
/**
14+
* @var list<Currency>
15+
*/
16+
public array $currencies {
17+
get => $this->currencies ??= array_map(fn($item) => new Currency($item), $this->data['currencies']);
18+
}
19+
20+
public string $name {
21+
get => $this->name ??= $this->data['name'];
22+
}
23+
24+
/**
25+
* @param array{
26+
* 'currencies': list<string>,
27+
* 'name': string,
28+
* } $data
29+
*/
30+
public function __construct(
31+
private readonly array $data,
32+
) {}
33+
}

‎tests/ListScalar/Generated/Query/Test/Error.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test;
6+
7+
// This file was automatically generated and should not be edited.
8+
9+
final readonly class Error
10+
{
11+
public string $message;
12+
13+
/**
14+
* @param array{
15+
* 'debugMessage'?: string,
16+
* 'message': string,
17+
* } $error
18+
*/
19+
public function __construct(array $error)
20+
{
21+
$this->message = $error['debugMessage'] ?? $error['message'];
22+
}
23+
}

‎tests/ListScalar/Generated/Query/Test/TestQuery.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test;
6+
7+
use Ruudk\GraphQLCodeGenerator\TestClient;
8+
9+
// This file was automatically generated and should not be edited.
10+
11+
final readonly class TestQuery {
12+
public const string OPERATION_NAME = 'Test';
13+
public const string OPERATION_DEFINITION = <<<'GRAPHQL'
14+
query Test {
15+
supportedCurrencies
16+
wallet {
17+
name
18+
currencies
19+
}
20+
}
21+
22+
GRAPHQL;
23+
24+
public function __construct(
25+
private TestClient $client,
26+
) {}
27+
28+
public function execute() : Data
29+
{
30+
$data = $this->client->graphql(
31+
self::OPERATION_DEFINITION,
32+
[
33+
],
34+
self::OPERATION_NAME,
35+
);
36+
37+
return new Data(
38+
$data['data'] ?? [], // @phpstan-ignore argument.type
39+
$data['errors'] ?? [] // @phpstan-ignore argument.type
40+
);
41+
}
42+
}

‎tests/ListScalar/ListScalarTest.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar;
6+
7+
use Override;
8+
use Ruudk\GraphQLCodeGenerator\Config\Config;
9+
use Ruudk\GraphQLCodeGenerator\GraphQLTestCase;
10+
use Ruudk\GraphQLCodeGenerator\ListScalar\Generated\Query\Test\TestQuery;
11+
use Ruudk\GraphQLCodeGenerator\ListScalar\ValueObjects\Currency;
12+
use Symfony\Component\TypeInfo\Type;
13+
14+
/**
15+
* Test that lists of custom scalars are correctly typed in the generated code.
16+
*
17+
* This tests the fix for a bug where the $builtInOnly parameter was not correctly
18+
* passed when processing ListOfType in TypeMapper::mapGraphQLTypeToPHPType().
19+
*
20+
* The bug caused the $data parameter docblock to incorrectly use the PHP mapped type
21+
* (e.g., Currency) instead of the primitive JSON type (e.g., string) for list items.
22+
*/
23+
final class ListScalarTest extends GraphQLTestCase
24+
{
25+
#[Override]
26+
public function getConfig() : Config
27+
{
28+
return parent::getConfig()
29+
->withScalar('Currency', Type::string(), Type::object(Currency::class));
30+
}
31+
32+
public function testGenerate() : void
33+
{
34+
$this->assertActualMatchesExpected();
35+
}
36+
37+
public function testQueryWithListOfScalars() : void
38+
{
39+
$result = new TestQuery($this->getClient([
40+
'data' => [
41+
'supportedCurrencies' => ['EUR', 'USD', 'GBP'],
42+
'wallet' => [
43+
'name' => 'My Wallet',
44+
'currencies' => ['EUR', 'USD'],
45+
],
46+
],
47+
]))->execute();
48+
49+
// Verify the list of currencies is correctly converted to Currency objects
50+
self::assertCount(3, $result->supportedCurrencies);
51+
self::assertSame('EUR', $result->supportedCurrencies[0]->code);
52+
self::assertSame('USD', $result->supportedCurrencies[1]->code);
53+
self::assertSame('GBP', $result->supportedCurrencies[2]->code);
54+
55+
// Verify nested list of currencies
56+
self::assertSame('My Wallet', $result->wallet->name);
57+
self::assertCount(2, $result->wallet->currencies);
58+
self::assertSame('EUR', $result->wallet->currencies[0]->code);
59+
self::assertSame('USD', $result->wallet->currencies[1]->code);
60+
}
61+
}

‎tests/ListScalar/Schema.graphql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
scalar Currency
2+
3+
type Query {
4+
supportedCurrencies: [Currency!]!
5+
wallet: Wallet!
6+
}
7+
8+
type Wallet {
9+
name: String!
10+
currencies: [Currency!]!
11+
}

‎tests/ListScalar/Test.graphql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
query Test {
2+
supportedCurrencies
3+
wallet {
4+
name
5+
currencies
6+
}
7+
}

‎tests/ListScalar/ValueObjects/Currency.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ruudk\GraphQLCodeGenerator\ListScalar\ValueObjects;
6+
7+
use InvalidArgumentException;
8+
use JsonSerializable;
9+
use Override;
10+
use Stringable;
11+
12+
final readonly class Currency implements JsonSerializable, Stringable
13+
{
14+
public function __construct(
15+
public string $code,
16+
) {
17+
if (strlen($code) !== 3) {
18+
throw new InvalidArgumentException('Currency code must be 3 characters');
19+
}
20+
}
21+
22+
#[Override]
23+
public function jsonSerialize() : string
24+
{
25+
return $this->code;
26+
}
27+
28+
#[Override]
29+
public function __toString() : string
30+
{
31+
return $this->code;
32+
}
33+
}

0 commit comments

Comments
 (0)