-
Notifications
You must be signed in to change notification settings - Fork 97
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Precise return type for
Result::rowCount()
based on detected driver
- Loading branch information
1 parent
ba9563e
commit 66c248d
Showing
10 changed files
with
292 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
src/Type/Doctrine/DBAL/RowCountMethodDynamicReturnTypeExtension.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Type\Doctrine\DBAL; | ||
|
||
use Doctrine\DBAL\Driver\Result as DriverResult; | ||
use Doctrine\ORM\EntityManagerInterface; | ||
use PhpParser\Node\Expr\MethodCall; | ||
use PHPStan\Analyser\Scope; | ||
use PHPStan\Doctrine\Driver\DriverDetector; | ||
use PHPStan\Reflection\MethodReflection; | ||
use PHPStan\Reflection\ParametersAcceptorSelector; | ||
use PHPStan\Reflection\ReflectionProvider; | ||
use PHPStan\Type\Doctrine\ObjectMetadataResolver; | ||
use PHPStan\Type\DynamicMethodReturnTypeExtension; | ||
use PHPStan\Type\Type; | ||
|
||
class RowCountMethodDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension | ||
{ | ||
|
||
/** @var string */ | ||
private $class; | ||
|
||
/** @var ObjectMetadataResolver */ | ||
private $objectMetadataResolver; | ||
|
||
/** @var DriverDetector */ | ||
private $driverDetector; | ||
|
||
/** @var ReflectionProvider */ | ||
private $reflectionProvider; | ||
|
||
public function __construct( | ||
string $class, | ||
ObjectMetadataResolver $objectMetadataResolver, | ||
DriverDetector $driverDetector, | ||
ReflectionProvider $reflectionProvider | ||
) | ||
{ | ||
$this->class = $class; | ||
$this->objectMetadataResolver = $objectMetadataResolver; | ||
$this->driverDetector = $driverDetector; | ||
$this->reflectionProvider = $reflectionProvider; | ||
} | ||
|
||
public function getClass(): string | ||
{ | ||
return $this->class; | ||
} | ||
|
||
public function isMethodSupported(MethodReflection $methodReflection): bool | ||
{ | ||
return $methodReflection->getName() === 'rowCount'; | ||
} | ||
|
||
public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type | ||
{ | ||
$objectManager = $this->objectMetadataResolver->getObjectManager(); | ||
if (!$objectManager instanceof EntityManagerInterface) { | ||
return null; | ||
} | ||
|
||
$connection = $objectManager->getConnection(); | ||
$driver = $this->driverDetector->detect($connection); | ||
if ($driver === null) { | ||
return null; | ||
} | ||
|
||
$resultClass = $this->getResultClass($driver); | ||
if ($resultClass === null) { | ||
return null; | ||
} | ||
|
||
if (!$this->reflectionProvider->hasClass($resultClass)) { | ||
return null; | ||
} | ||
|
||
$resultReflection = $this->reflectionProvider->getClass($resultClass); | ||
if (!$resultReflection->hasNativeMethod('rowCount')) { | ||
return null; | ||
} | ||
|
||
$rowCountMethod = $resultReflection->getNativeMethod('rowCount'); | ||
$variant = ParametersAcceptorSelector::selectSingle($rowCountMethod->getVariants()); | ||
|
||
return $variant->getReturnType(); | ||
} | ||
|
||
/** | ||
* @param DriverDetector::* $driver | ||
* @return class-string<DriverResult>|null | ||
*/ | ||
private function getResultClass(string $driver): ?string | ||
{ | ||
switch ($driver) { | ||
case DriverDetector::IBM_DB2: | ||
return 'Doctrine\DBAL\Driver\IBMDB2\Result'; | ||
case DriverDetector::MYSQLI: | ||
return 'Doctrine\DBAL\Driver\Mysqli\Result'; | ||
case DriverDetector::OCI8: | ||
return 'Doctrine\DBAL\Driver\OCI8\Result'; | ||
case DriverDetector::PDO_MYSQL: | ||
case DriverDetector::PDO_OCI: | ||
case DriverDetector::PDO_PGSQL: | ||
case DriverDetector::PDO_SQLITE: | ||
case DriverDetector::PDO_SQLSRV: | ||
return 'Doctrine\DBAL\Driver\PDO\Result'; | ||
case DriverDetector::PGSQL: | ||
return 'Doctrine\DBAL\Driver\PgSQL\Result'; | ||
Check failure on line 108 in src/Type/Doctrine/DBAL/RowCountMethodDynamicReturnTypeExtension.php GitHub Actions / PHPStan (7.3)
|
||
case DriverDetector::SQLITE3: | ||
return 'Doctrine\DBAL\Driver\SQLite3\Result'; | ||
Check failure on line 110 in src/Type/Doctrine/DBAL/RowCountMethodDynamicReturnTypeExtension.php GitHub Actions / PHPStan (7.3)
|
||
case DriverDetector::SQLSRV: | ||
return 'Doctrine\DBAL\Driver\SQLSrv\Result'; | ||
} | ||
|
||
return null; | ||
} | ||
|
||
} |
35 changes: 35 additions & 0 deletions
35
tests/Type/Doctrine/DBAL/MysqliResultRowCountReturnTypeTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Type\Doctrine\DBAL; | ||
|
||
use PHPStan\Testing\TypeInferenceTestCase; | ||
|
||
class MysqliResultRowCountReturnTypeTest extends TypeInferenceTestCase | ||
{ | ||
|
||
/** @return iterable<mixed> */ | ||
public function dataFileAsserts(): iterable | ||
{ | ||
yield from $this->gatherAssertTypes(__DIR__ . '/data/mysqli-result-row-count.php'); | ||
} | ||
|
||
/** | ||
* @dataProvider dataFileAsserts | ||
* @param mixed ...$args | ||
*/ | ||
public function testFileAsserts( | ||
string $assertType, | ||
string $file, | ||
...$args | ||
): void | ||
{ | ||
$this->assertFileAsserts($assertType, $file, ...$args); | ||
} | ||
|
||
/** @return string[] */ | ||
public static function getAdditionalConfigFiles(): array | ||
{ | ||
return [__DIR__ . '/mysqli.neon']; | ||
} | ||
|
||
} |
35 changes: 35 additions & 0 deletions
35
tests/Type/Doctrine/DBAL/PDOResultRowCountReturnTypeTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Type\Doctrine\DBAL; | ||
|
||
use PHPStan\Testing\TypeInferenceTestCase; | ||
|
||
class PDOResultRowCountReturnTypeTest extends TypeInferenceTestCase | ||
{ | ||
|
||
/** @return iterable<mixed> */ | ||
public function dataFileAsserts(): iterable | ||
{ | ||
yield from $this->gatherAssertTypes(__DIR__ . '/data/pdo-result-row-count.php'); | ||
} | ||
|
||
/** | ||
* @dataProvider dataFileAsserts | ||
* @param mixed ...$args | ||
*/ | ||
public function testFileAsserts( | ||
string $assertType, | ||
string $file, | ||
...$args | ||
): void | ||
{ | ||
$this->assertFileAsserts($assertType, $file, ...$args); | ||
} | ||
|
||
/** @return string[] */ | ||
public static function getAdditionalConfigFiles(): array | ||
{ | ||
return [__DIR__ . '/pdo.neon']; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
namespace MysqliResultRowCount; | ||
|
||
use Doctrine\DBAL\Result; | ||
use Doctrine\DBAL\Driver\Result as DriverResult; | ||
use function PHPStan\Testing\assertType; | ||
|
||
function (Result $r): void { | ||
assertType('int|numeric-string', $r->rowCount()); | ||
}; | ||
|
||
function (DriverResult $r): void { | ||
assertType('int|numeric-string', $r->rowCount()); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
namespace PDOResultRowCount; | ||
|
||
use Doctrine\DBAL\Result; | ||
use Doctrine\DBAL\Driver\Result as DriverResult; | ||
use function PHPStan\Testing\assertType; | ||
|
||
function (Result $r): void { | ||
assertType('int', $r->rowCount()); | ||
}; | ||
|
||
function (DriverResult $r): void { | ||
assertType('int', $r->rowCount()); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
includes: | ||
- ../../../../extension.neon | ||
|
||
parameters: | ||
doctrine: | ||
objectManagerLoader: mysqli.php |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
use Cache\Adapter\PHPArray\ArrayCachePool; | ||
use Doctrine\Common\Annotations\AnnotationReader; | ||
use Doctrine\DBAL\DriverManager; | ||
use Doctrine\ORM\Configuration; | ||
use Doctrine\ORM\EntityManager; | ||
use Doctrine\ORM\Mapping\Driver\AnnotationDriver; | ||
|
||
$config = new Configuration(); | ||
$config->setProxyDir(__DIR__); | ||
$config->setProxyNamespace('App\GeneratedProxy'); | ||
$config->setMetadataCache(new ArrayCachePool()); | ||
$config->setMetadataDriverImpl(new AnnotationDriver( | ||
new AnnotationReader(), | ||
[__DIR__ . '/data'] | ||
)); | ||
|
||
return new EntityManager( | ||
DriverManager::getConnection([ | ||
'driver' => 'mysqli', | ||
'memory' => true, | ||
]), | ||
$config | ||
); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
includes: | ||
- ../../../../extension.neon | ||
|
||
parameters: | ||
doctrine: | ||
objectManagerLoader: pdo.php |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
use Cache\Adapter\PHPArray\ArrayCachePool; | ||
use Doctrine\Common\Annotations\AnnotationReader; | ||
use Doctrine\DBAL\DriverManager; | ||
use Doctrine\ORM\Configuration; | ||
use Doctrine\ORM\EntityManager; | ||
use Doctrine\ORM\Mapping\Driver\AnnotationDriver; | ||
|
||
$config = new Configuration(); | ||
$config->setProxyDir(__DIR__); | ||
$config->setProxyNamespace('App\GeneratedProxy'); | ||
$config->setMetadataCache(new ArrayCachePool()); | ||
$config->setMetadataDriverImpl(new AnnotationDriver( | ||
new AnnotationReader(), | ||
[__DIR__ . '/data'] | ||
)); | ||
|
||
return new EntityManager( | ||
DriverManager::getConnection([ | ||
'driver' => 'pdo_pgsql', | ||
'memory' => true, | ||
]), | ||
$config | ||
); |
66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @derrabus I'm narrowing the type of
Doctrine\DBAL\Result::rowCount()
andDoctrine\DBAL\Driver\Result::rowCount()
(int|numeric-string
) based onDoctrine\DBAL\Driver\*\Result::rowCount()
return type with the detected driver class. Most of them are justint
.Do you have some suggestions what else in Doctrine could benefit from the same approach? Thanks :)
66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware that you can detect the driver statically. For instance, in a typical Symfony project, I could switch from SQLite to MySQL by only changing an environment variable.
Also please don't ignore the middleware layer. In theory I could implement a driver middleware that wraps results like this:
Not that it would make much sense, but it would be correct according to the contract. 😓
66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not exactly "statically", it depends on objectManagerLoader.
Can we access these middlewares and read their class names from Configuration?
66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the detection only works in projects that use the ORM and it assumes that the same driver is used in all environments.
Doctrine\DBAL\Configuration::getMiddlewares()
returns the configured stack.66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@derrabus Yes, this feature depends on that: https://x.com/janedbal/status/1807693081459745065
I'm going to look into the middlewares for the extension I added in this commit.
66c248d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could add some configuration option to the phpstan-doctrine plugin that allows to specify the driver(s) that the project should be analyzed for? All this type narrowing is pure DBAL logic. It would be a pity if it wouldn't work on projects that don't use the ORM or those that support multiple DBMS.