…er::getConnection($args)` params
As per doctrine#4637, `vimeo/psalm:4.6.4` cannot introduce templated parameters within a re-used imported
type: this is understandable, since imported types are copy-paste aided by the type checker, but do
not carry any template information with them (and it would be too complex to do so anyway).
Quoting the initial issue:
Minimal reproducer: https://psalm.dev/r/62a0a5854d
```php
<?php
class Connection {}
class MyConnection extends Connection{}
/**
* @psalm-type Params = array{
* charset?: string,
* wrapperClass?: class-string<Connection>,
* }
*/
final class DriverManager
{
/**
* @param array<string,mixed> $params
* @param Configuration|null $config The configuration to use.
* @param EventManager|null $eventManager The event manager to use.
*
* @phpstan-param array<string,mixed> $params
* @psalm-param Params $params
* @psalm-return ($params is array{wrapperClass:mixed} ? T : Connection)
* @template T of Connection
*/
public static function getConnection(
array $params
): Connection {
throw new \Exception('irrelevant');
}
}
function mkConnection(): MyConnection {
return DriverManager::getConnection(['wrapperClass' => MyConnection::class]);
}
```
Which produces:
```
Psalm output (using commit aa854ae):
INFO: MixedInferredReturnType - 31:26 - Could not verify return type 'MyConnection' for mkConnection
```
The issue is that the `Param` alias type is not templated: this is potentially a psalm limitation, but also an understandable one, since there is no syntax to specify template arguments for an imported type (AFAIK).
Here is a potential fix:
https://psalm.dev/r/8ef9909bb9
```php
<?php
class Connection {}
class MyConnection extends Connection{}
/**
* @psalm-type Params = array{
* charset?: string,
* wrapperClass?: class-string<Connection>,
* }
*/
final class DriverManager
{
/**
* @psalm-param array{
* charset?: string,
* wrapperClass?: class-string<T>,
* } $params
* @psalm-return ($params is array{wrapperClass: mixed} ? T : Connection)
* @template T of Connection
*/
public static function getConnection(
array $params
): Connection {
throw new \Exception('irrelevant');
}
}
function mkConnection(): MyConnection {
return DriverManager::getConnection(['wrapperClass' => MyConnection::class]);
}
```
It's a bit ugly, but the idea is that we replace `Params` with the concrete definition again, but this time including `T`, so that the type inference can correctly fill the type variable with the given input:
```
Psalm output (using commit aa854ae):
No issues!
```
The idea therefore is to re-introduce some duplication at the advantage of better type inference later on.
In order to verify that the change had an effect, we introduced a new `tests/Doctrine/StaticAnalysis` directory,
to be checked via `vendor/bin/psalm -c psalm-strict.xml`, which will tell us exactly whether there are obvious
type-checker issues around our code. Since `psalm.xml.dist` uses `errorLevel="2"`, it is not sufficient to integrate
this directory within the normal workflow.
Because this project uses a custom github action for running `vimeo/psalm`, it is not really possible to run this
in CI, and therefore that part of the patch has been omitted.
It is endorsed that maintainers of `doctrine/dbal` in future:
1. commit `composer.lock`
2. set up dependabot to track dependency upgrades
3. use the standard `vimeo/psalm` via `require-dev`, and run the CI static analysis checks from the same
environment that runs normal unit/integration tests