Skip to content

Commit

Permalink
Precise return type for Result::rowCount() based on detected driver
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Sep 1, 2024
1 parent ba9563e commit 66c248d
Show file tree
Hide file tree
Showing 10 changed files with 292 additions and 0 deletions.
12 changes: 12 additions & 0 deletions extension.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,18 @@ services:
class: PHPStan\Type\Doctrine\DBAL\QueryBuilder\QueryBuilderExecuteMethodExtension
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
class: PHPStan\Type\Doctrine\DBAL\RowCountMethodDynamicReturnTypeExtension
arguments:
class: Doctrine\DBAL\Result
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
class: PHPStan\Type\Doctrine\DBAL\RowCountMethodDynamicReturnTypeExtension
arguments:
class: Doctrine\DBAL\Driver\Result
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension

# Type descriptors
-
Expand Down
118 changes: 118 additions & 0 deletions src/Type/Doctrine/DBAL/RowCountMethodDynamicReturnTypeExtension.php
View file
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

View workflow job for this annotation

GitHub Actions / PHPStan (7.3) 

Method PHPStan\Type\Doctrine\DBAL\RowCountMethodDynamicReturnTypeExtension::getResultClass() should return class-string<Doctrine\DBAL\Driver\Result>|null but returns string.
case DriverDetector::SQLITE3:
return 'Doctrine\DBAL\Driver\SQLite3\Result';

Check failure on line 110 in src/Type/Doctrine/DBAL/RowCountMethodDynamicReturnTypeExtension.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3) 

Method PHPStan\Type\Doctrine\DBAL\RowCountMethodDynamicReturnTypeExtension::getResultClass() should return class-string<Doctrine\DBAL\Driver\Result>|null but returns string.
case DriverDetector::SQLSRV:
return 'Doctrine\DBAL\Driver\SQLSrv\Result';
}

return null;
}

}
35 changes: 35 additions & 0 deletions tests/Type/Doctrine/DBAL/MysqliResultRowCountReturnTypeTest.php
View file
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 tests/Type/Doctrine/DBAL/PDOResultRowCountReturnTypeTest.php
View file
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'];
}

}
15 changes: 15 additions & 0 deletions tests/Type/Doctrine/DBAL/data/mysqli-result-row-count.php
View file
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());
};
15 changes: 15 additions & 0 deletions tests/Type/Doctrine/DBAL/data/pdo-result-row-count.php
View file
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());
};
6 changes: 6 additions & 0 deletions tests/Type/Doctrine/DBAL/mysqli.neon
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
includes:
- ../../../../extension.neon

parameters:
doctrine:
objectManagerLoader: mysqli.php
25 changes: 25 additions & 0 deletions tests/Type/Doctrine/DBAL/mysqli.php
View file
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
);
6 changes: 6 additions & 0 deletions tests/Type/Doctrine/DBAL/pdo.neon
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
includes:
- ../../../../extension.neon

parameters:
doctrine:
objectManagerLoader: pdo.php
25 changes: 25 additions & 0 deletions tests/Type/Doctrine/DBAL/pdo.php
View file
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
);

6 comments on commit 66c248d

@ondrejmirtes
Copy link
Member Author

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() and Doctrine\DBAL\Driver\Result::rowCount() (int|numeric-string) based on Doctrine\DBAL\Driver\*\Result::rowCount() return type with the detected driver class. Most of them are just int.

Do you have some suggestions what else in Doctrine could benefit from the same approach? Thanks :)

@derrabus
Copy link

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:

/** @return numeric-string */
function rowCount(): string
{
    return (string) parent::rowCount();
}

Not that it would make much sense, but it would be correct according to the contract. 😓

@ondrejmirtes
Copy link
Member Author

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?

@derrabus
Copy link

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.

So, the detection only works in projects that use the ORM and it assumes that the same driver is used in all environments.

Can we access these middlewares and read their class names from Configuration?

Doctrine\DBAL\Configuration::getMiddlewares() returns the configured stack.

@ondrejmirtes
Copy link
Member Author

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.

@derrabus
Copy link

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.

Please sign in to comment.