Skip to content

Commit 8a25471

Browse files
committed
Deprecate TypedExpression in favor of ExpressionWithReturnType
`TypedExpression::getReturnType()` returns a `Doctrine\DBAL\Types\Type` instance, which forces implementers to perform a static, global type lookup (`Type::getType()`). `SqlWalker` then converts the instance back to a name via `TypeRegistry::lookupName()`, making the round-trip pointless and introducing two static `Type::*` calls that block moving the ORM to instance-based type registries. Introduce `Doctrine\ORM\Query\AST\ExpressionWithReturnType`, which exposes the DBAL type name directly (`getReturnTypeName(): string`). `SqlWalker` prefers the new interface, falls back to the legacy one with a deprecation notice, and finally to the `'string'` default. `CountFunction` and `LengthFunction` now implement both interfaces: `getReturnTypeName()` returns a plain `Types::*` constant, and the legacy `getReturnType()` delegates and is marked `@deprecated`.
1 parent e56aa3b commit 8a25471

7 files changed

Lines changed: 214 additions & 11 deletions

File tree

UPGRADE.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,47 @@ and directly start using native lazy objects.
2929

3030
# Upgrade to 3.7
3131

32+
## Deprecated `Doctrine\ORM\Query\AST\TypedExpression`
33+
34+
The `TypedExpression` interface, whose `getReturnType()` method returns a
35+
`Doctrine\DBAL\Types\Type` instance, forces implementers to resolve the type
36+
via a static, global lookup (`Type::getType()`). The result is then converted
37+
back to a name by `SqlWalker`, making the round-trip pointless.
38+
39+
Implement `Doctrine\ORM\Query\AST\ExpressionWithReturnType` instead, which exposes
40+
the DBAL type name directly.
41+
42+
Before:
43+
44+
```php
45+
use Doctrine\DBAL\Types\Type;
46+
use Doctrine\DBAL\Types\Types;
47+
use Doctrine\ORM\Query\AST\TypedExpression;
48+
49+
class MyFunction extends FunctionNode implements TypedExpression
50+
{
51+
public function getReturnType(): Type
52+
{
53+
return Type::getType(Types::INTEGER);
54+
}
55+
}
56+
```
57+
58+
After:
59+
60+
```php
61+
use Doctrine\DBAL\Types\Types;
62+
use Doctrine\ORM\Query\AST\ExpressionWithReturnType;
63+
64+
class MyFunction extends FunctionNode implements ExpressionWithReturnType
65+
{
66+
public function getReturnTypeName(): string
67+
{
68+
return Types::INTEGER;
69+
}
70+
}
71+
```
72+
3273
## Deprecated using strings or null as sort directions
3374

3475
PHP 8.6 provides a native `\SortDirection` enum that should be used instead of
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\ORM\Query\AST;
6+
7+
/**
8+
* Provides an API for resolving the DBAL type name of a Node.
9+
*
10+
* Replaces {@see TypedExpression}, which returns a Type instance and thus
11+
* forces implementers to perform a global static Type lookup.
12+
*/
13+
interface ExpressionWithReturnType
14+
{
15+
/**
16+
* Returns the DBAL type name (see {@see \Doctrine\DBAL\Types\Types}) of
17+
* the value produced by this expression.
18+
*/
19+
public function getReturnTypeName(): string;
20+
}

src/Query/AST/Functions/CountFunction.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@
77
use Doctrine\DBAL\Types\Type;
88
use Doctrine\DBAL\Types\Types;
99
use Doctrine\ORM\Query\AST\AggregateExpression;
10+
use Doctrine\ORM\Query\AST\ExpressionWithReturnType;
1011
use Doctrine\ORM\Query\AST\TypedExpression;
1112
use Doctrine\ORM\Query\Parser;
1213
use Doctrine\ORM\Query\SqlWalker;
1314

1415
/**
1516
* "COUNT" "(" ["DISTINCT"] StringPrimary ")"
1617
*/
17-
final class CountFunction extends FunctionNode implements TypedExpression
18+
final class CountFunction extends FunctionNode implements ExpressionWithReturnType, TypedExpression
1819
{
1920
private AggregateExpression $aggregateExpression;
2021

@@ -28,8 +29,14 @@ public function parse(Parser $parser): void
2829
$this->aggregateExpression = $parser->AggregateExpression();
2930
}
3031

32+
public function getReturnTypeName(): string
33+
{
34+
return Types::INTEGER;
35+
}
36+
37+
/** @deprecated Use {@see getReturnTypeName()} instead. */
3138
public function getReturnType(): Type
3239
{
33-
return Type::getType(Types::INTEGER);
40+
return Type::getType($this->getReturnTypeName());
3441
}
3542
}

src/Query/AST/Functions/LengthFunction.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Doctrine\DBAL\Types\Type;
88
use Doctrine\DBAL\Types\Types;
9+
use Doctrine\ORM\Query\AST\ExpressionWithReturnType;
910
use Doctrine\ORM\Query\AST\Node;
1011
use Doctrine\ORM\Query\AST\TypedExpression;
1112
use Doctrine\ORM\Query\Parser;
@@ -17,7 +18,7 @@
1718
*
1819
* @link www.doctrine-project.org
1920
*/
20-
class LengthFunction extends FunctionNode implements TypedExpression
21+
class LengthFunction extends FunctionNode implements ExpressionWithReturnType, TypedExpression
2122
{
2223
public Node $stringPrimary;
2324

@@ -38,8 +39,14 @@ public function parse(Parser $parser): void
3839
$parser->match(TokenType::T_CLOSE_PARENTHESIS);
3940
}
4041

42+
public function getReturnTypeName(): string
43+
{
44+
return Types::INTEGER;
45+
}
46+
47+
/** @deprecated Use {@see getReturnTypeName()} instead. */
4148
public function getReturnType(): Type
4249
{
43-
return Type::getType(Types::INTEGER);
50+
return Type::getType($this->getReturnTypeName());
4451
}
4552
}

src/Query/AST/TypedExpression.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
use Doctrine\DBAL\Types\Type;
88

99
/**
10-
* Provides an API for resolving the type of a Node
10+
* Provides an API for resolving the type of a Node.
11+
*
12+
* @deprecated Implement {@see ExpressionWithReturnType} instead, which returns
13+
* the type name as a string and avoids a static Type lookup.
1114
*/
1215
interface TypedExpression
1316
{

src/Query/SqlWalker.php

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Doctrine\DBAL\LockMode;
1010
use Doctrine\DBAL\Platforms\AbstractPlatform;
1111
use Doctrine\DBAL\Types\Type;
12+
use Doctrine\DBAL\Types\Types;
1213
use Doctrine\Deprecations\Deprecation;
1314
use Doctrine\ORM\EntityManagerInterface;
1415
use Doctrine\ORM\Mapping\ClassMetadata;
@@ -1337,15 +1338,29 @@ public function walkSelectExpression(AST\SelectExpression $selectExpression): st
13371338
break;
13381339
}
13391340

1340-
if (! $expr instanceof Query\AST\TypedExpression) {
1341-
// Conceptually we could resolve field type here by traverse through AST to retrieve field type,
1342-
// but this is not a feasible solution; assume 'string'.
1343-
$this->rsm->addScalarResult($columnAlias, $resultAlias, 'string');
1341+
if ($expr instanceof Query\AST\ExpressionWithReturnType) {
1342+
$this->rsm->addScalarResult($columnAlias, $resultAlias, $expr->getReturnTypeName());
13441343

13451344
break;
13461345
}
13471346

1348-
$this->rsm->addScalarResult($columnAlias, $resultAlias, Type::getTypeRegistry()->lookupName($expr->getReturnType()));
1347+
if ($expr instanceof Query\AST\TypedExpression) {
1348+
Deprecation::trigger(
1349+
'doctrine/orm',
1350+
'https://github.com/doctrine/orm/pull/12543',
1351+
'Implementing %s is deprecated, implement %s instead.',
1352+
Query\AST\TypedExpression::class,
1353+
Query\AST\ExpressionWithReturnType::class,
1354+
);
1355+
1356+
$this->rsm->addScalarResult($columnAlias, $resultAlias, Type::getTypeRegistry()->lookupName($expr->getReturnType()));
1357+
1358+
break;
1359+
}
1360+
1361+
// Conceptually we could resolve field type here by traverse through AST to retrieve field type,
1362+
// but this is not a feasible solution; assume 'string'.
1363+
$this->rsm->addScalarResult($columnAlias, $resultAlias, 'string');
13491364

13501365
break;
13511366

@@ -1359,7 +1374,7 @@ public function walkSelectExpression(AST\SelectExpression $selectExpression): st
13591374

13601375
if (! $hidden) {
13611376
// We cannot resolve field type here; assume 'string'.
1362-
$this->rsm->addScalarResult($columnAlias, $resultAlias, 'string');
1377+
$this->rsm->addScalarResult($columnAlias, $resultAlias, Types::STRING);
13631378
}
13641379

13651380
break;
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Query;
6+
7+
use Doctrine\DBAL\Types\Type;
8+
use Doctrine\DBAL\Types\Types;
9+
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
10+
use Doctrine\ORM\EntityManagerInterface;
11+
use Doctrine\ORM\Query\AST\ExpressionWithReturnType;
12+
use Doctrine\ORM\Query\AST\Functions\FunctionNode;
13+
use Doctrine\ORM\Query\AST\Node;
14+
use Doctrine\ORM\Query\AST\TypedExpression;
15+
use Doctrine\ORM\Query\Parser;
16+
use Doctrine\ORM\Query\SqlWalker;
17+
use Doctrine\ORM\Query\TokenType;
18+
use Doctrine\Tests\Models\CMS\CmsPhonenumber;
19+
use Doctrine\Tests\OrmTestCase;
20+
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
21+
22+
class TypedExpressionDeprecationTest extends OrmTestCase
23+
{
24+
use VerifyDeprecations;
25+
26+
private EntityManagerInterface $entityManager;
27+
28+
protected function setUp(): void
29+
{
30+
$this->entityManager = $this->getTestEntityManager();
31+
}
32+
33+
#[IgnoreDeprecations]
34+
public function testImplementingLegacyTypedExpressionTriggersDeprecation(): void
35+
{
36+
$this->entityManager
37+
->getConfiguration()
38+
->addCustomNumericFunction('LEGACY_TYPED', LegacyTypedFunctionStub::class);
39+
40+
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/12543');
41+
42+
$this->entityManager
43+
->createQuery('SELECT LEGACY_TYPED(p.phonenumber) FROM ' . CmsPhonenumber::class . ' p')
44+
->getSQL();
45+
}
46+
47+
public function testImplementingExpressionWithReturnTypeDoesNotTriggerDeprecation(): void
48+
{
49+
$this->entityManager
50+
->getConfiguration()
51+
->addCustomNumericFunction('MODERN_TYPED', ModernTypedFunctionStub::class);
52+
53+
$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/12543');
54+
55+
$this->entityManager
56+
->createQuery('SELECT MODERN_TYPED(p.phonenumber) FROM ' . CmsPhonenumber::class . ' p')
57+
->getSQL();
58+
}
59+
}
60+
61+
final class LegacyTypedFunctionStub extends FunctionNode implements TypedExpression
62+
{
63+
private Node $arithmeticExpression;
64+
65+
public function getSql(SqlWalker $sqlWalker): string
66+
{
67+
return 'ABS(' . $sqlWalker->walkSimpleArithmeticExpression($this->arithmeticExpression) . ')';
68+
}
69+
70+
public function parse(Parser $parser): void
71+
{
72+
$parser->match(TokenType::T_IDENTIFIER);
73+
$parser->match(TokenType::T_OPEN_PARENTHESIS);
74+
$this->arithmeticExpression = $parser->SimpleArithmeticExpression();
75+
$parser->match(TokenType::T_CLOSE_PARENTHESIS);
76+
}
77+
78+
public function getReturnType(): Type
79+
{
80+
return Type::getType(Types::INTEGER);
81+
}
82+
}
83+
84+
final class ModernTypedFunctionStub extends FunctionNode implements ExpressionWithReturnType, TypedExpression
85+
{
86+
private Node $arithmeticExpression;
87+
88+
public function getSql(SqlWalker $sqlWalker): string
89+
{
90+
return 'ABS(' . $sqlWalker->walkSimpleArithmeticExpression($this->arithmeticExpression) . ')';
91+
}
92+
93+
public function parse(Parser $parser): void
94+
{
95+
$parser->match(TokenType::T_IDENTIFIER);
96+
$parser->match(TokenType::T_OPEN_PARENTHESIS);
97+
$this->arithmeticExpression = $parser->SimpleArithmeticExpression();
98+
$parser->match(TokenType::T_CLOSE_PARENTHESIS);
99+
}
100+
101+
public function getReturnTypeName(): string
102+
{
103+
return Types::INTEGER;
104+
}
105+
106+
public function getReturnType(): Type
107+
{
108+
return Type::getType($this->getReturnTypeName());
109+
}
110+
}

0 commit comments

Comments
 (0)