Merge pull request #5904 from derrabus/improvement/modern-pgsql

Modernize pgsql driver for PHP 8.1

phpcs.xml.dist

@@ -53,11 +53,6 @@
         <exclude-pattern>*/tests/*</exclude-pattern>
     </rule>
 
-    <rule ref="SlevomatCodingStandard.Classes.RequireConstructorPropertyPromotion.RequiredConstructorPropertyPromotion">
-        <!-- CPP breaks static analysis here. -->
-        <exclude-pattern>src/Driver/PgSQL/Result.php</exclude-pattern>
-    </rule>
-
     <rule ref="SlevomatCodingStandard.PHP.RequireExplicitAssertion.RequiredExplicitAssertion">
         <exclude-pattern>tests/*</exclude-pattern>
     </rule>

phpstan.neon.dist

@@ -91,23 +91,10 @@ parameters:
         # We don't need to declare the return type *that* fine-grained.
         - '~^Method Doctrine\\DBAL\\Driver\\PDO\\Statement\:\:convertParamType\(\) never returns \d+ so it can be removed from the return type\.$~'
 
-        # PgSql objects are represented as resources in PHP 7.4
-        - '~ expects PgSql\\Connection(:?\|(?:string|null))?, PgSql\\Connection\|resource given\.$~'
-        - '~ expects PgSql\\Result, PgSql\\Result\|resource given\.$~'
-
         # PHPStan does not understand the array shapes returned by pg_fetch_*() methods.
         - '~^Parameter #1 \$row of method Doctrine\\DBAL\\Driver\\PgSQL\\Result\:\:mapAssociativeRow\(\) expects array<string, string\|null>, array<int\|string, string\|null> given\.$~'
         - '~^Parameter #1 \$row of method Doctrine\\DBAL\\Driver\\PgSQL\\Result\:\:mapNumericRow\(\) expects array<int, string\|null>, array<int\|string, string\|null> given\.$~'
 
-        # Ignore isset() checks in destructors.
-        - '~^Property Doctrine\\DBAL\\Driver\\PgSQL\\Connection\:\:\$connection \(PgSql\\Connection\|resource\) in isset\(\) is not nullable\.$~'
-        - '~^Property Doctrine\\DBAL\\Driver\\PgSQL\\Statement\:\:\$connection \(PgSql\\Connection\|resource\) in isset\(\) is not nullable\.$~'
-
-        # On PHP 7.4, pg_fetch_all() might return false for empty result sets.
-        -
-            message: '~^Strict comparison using === between array<int, array> and false will always evaluate to false\.$~'
-            paths:
-                - src/Driver/PgSQL/Result.php
 includes:
     - vendor/phpstan/phpstan-phpunit/extension.neon
     - vendor/phpstan/phpstan-phpunit/rules.neon

psalm.xml.dist

@@ -118,32 +118,6 @@
                 <file name="src/Driver/AbstractSQLiteDriver.php"/>
             </errorLevel>
         </NullableReturnStatement>
-        <PossiblyInvalidArgument>
-            <errorLevel type="suppress">
-                <!-- PgSql objects are represented as resources in PHP 7.4 -->
-                <referencedFunction name="pg_affected_rows"/>
-                <referencedFunction name="pg_close"/>
-                <referencedFunction name="pg_escape_bytea"/>
-                <referencedFunction name="pg_escape_identifier"/>
-                <referencedFunction name="pg_escape_literal"/>
-                <referencedFunction name="pg_fetch_all"/>
-                <referencedFunction name="pg_fetch_all_columns"/>
-                <referencedFunction name="pg_fetch_assoc"/>
-                <referencedFunction name="pg_fetch_row"/>
-                <referencedFunction name="pg_field_name"/>
-                <referencedFunction name="pg_field_type"/>
-                <referencedFunction name="pg_free_result"/>
-                <referencedFunction name="pg_get_result"/>
-                <referencedFunction name="pg_last_error"/>
-                <referencedFunction name="pg_num_fields"/>
-                <referencedFunction name="pg_query"/>
-                <referencedFunction name="pg_result_error_field"/>
-                <referencedFunction name="pg_send_execute"/>
-                <referencedFunction name="pg_send_prepare"/>
-                <referencedFunction name="pg_send_query"/>
-                <referencedFunction name="pg_version"/>
-            </errorLevel>
-        </PossiblyInvalidArgument>
         <PossiblyInvalidArrayOffset>
             <errorLevel type="suppress">
                 <!-- $array[key($array)] is safe. -->
@@ -227,8 +201,9 @@
         </TypeDoesNotContainNull>
         <TypeDoesNotContainType>
             <errorLevel type="suppress">
-                <!-- On PHP 7.4, pg_fetch_all() might return false for empty result sets. -->
-                <file name="src/Driver/PgSQL/Result.php"/>
+                <!-- Ignore isset() checks in destructors. -->
+                <file name="src/Driver/PgSQL/Connection.php"/>
+                <file name="src/Driver/PgSQL/Statement.php"/>
             </errorLevel>
         </TypeDoesNotContainType>
         <UndefinedDocblockClass>

src/Driver/PgSQL/Connection.php

@@ -8,12 +8,8 @@
 use Doctrine\DBAL\Driver\Exception\NoIdentityValue;
 use Doctrine\DBAL\SQL\Parser;
 use PgSql\Connection as PgSqlConnection;
-use TypeError;
 
 use function assert;
-use function gettype;
-use function is_object;
-use function is_resource;
 use function pg_close;
 use function pg_escape_literal;
 use function pg_get_result;
@@ -22,24 +18,14 @@
 use function pg_send_prepare;
 use function pg_send_query;
 use function pg_version;
-use function sprintf;
 use function uniqid;
 
 final class Connection implements ConnectionInterface
 {
-    private Parser $parser;
+    private readonly Parser $parser;
 
-    /** @param PgSqlConnection|resource $connection */
-    public function __construct(private mixed $connection)
+    public function __construct(private readonly PgSqlConnection $connection)
     {
-        if (! is_resource($connection) && ! $connection instanceof PgSqlConnection) {
-            throw new TypeError(sprintf(
-                'Expected connection to be a resource or an instance of %s, got %s.',
-                PgSqlConnection::class,
-                is_object($connection) ? $connection::class : gettype($connection),
-            ));
-        }
-
         $this->parser = new Parser(false);
     }
 
@@ -136,8 +122,7 @@ public function getServerVersion(): string
         return (string) pg_version($this->connection)['server'];
     }
 
-    /** @return PgSqlConnection|resource */
-    public function getNativeConnection()
+    public function getNativeConnection(): PgSqlConnection
     {
         return $this->connection;
     }

src/Driver/PgSQL/Driver.php

@@ -75,11 +75,11 @@ private function constructConnectionString(
                 'password' => $params['password'] ?? null,
                 'sslmode' => $params['sslmode'] ?? null,
             ],
-            static fn ($value) => $value !== '' && $value !== null,
+            static fn (int|string|null $value) => $value !== '' && $value !== null,
         );
 
         return implode(' ', array_map(
-            static fn ($value, string $key) => sprintf("%s='%s'", $key, addslashes($value)),
+            static fn (int|string $value, string $key) => sprintf("%s='%s'", $key, addslashes((string) $value)),
             array_values($components),
             array_keys($components),
         ));

src/Driver/PgSQL/Exception.php

@@ -19,8 +19,7 @@
  */
 final class Exception extends AbstractException
 {
-    /** @param PgSqlResult|resource $result */
-    public static function fromResult($result): self
+    public static function fromResult(PgSqlResult $result): self
     {
         $sqlstate = pg_result_error_field($result, PGSQL_DIAG_SQLSTATE);
         if ($sqlstate === false) {

src/Driver/PgSQL/Result.php

@@ -8,15 +8,11 @@
 use Doctrine\DBAL\Driver\PgSQL\Exception\UnexpectedValue;
 use Doctrine\DBAL\Driver\Result as ResultInterface;
 use PgSql\Result as PgSqlResult;
-use TypeError;
 
 use function array_keys;
 use function array_map;
 use function assert;
-use function gettype;
 use function hex2bin;
-use function is_object;
-use function is_resource;
 use function pg_affected_rows;
 use function pg_fetch_all;
 use function pg_fetch_all_columns;
@@ -26,7 +22,6 @@
 use function pg_field_type;
 use function pg_free_result;
 use function pg_num_fields;
-use function sprintf;
 use function substr;
 
 use const PGSQL_ASSOC;
@@ -35,20 +30,10 @@
 
 final class Result implements ResultInterface
 {
-    /** @var PgSqlResult|resource|null */
-    private mixed $result;
+    private ?PgSqlResult $result;
 
-    /** @param PgSqlResult|resource $result */
-    public function __construct(mixed $result)
+    public function __construct(PgSqlResult $result)
     {
-        if (! is_resource($result) && ! $result instanceof PgSqlResult) {
-            throw new TypeError(sprintf(
-                'Expected result to be a resource or an instance of %s, got %s.',
-                PgSqlResult::class,
-                is_object($result) ? $result::class : gettype($result),
-            ));
-        }
-
         $this->result = $result;
     }
 
@@ -95,17 +80,11 @@ public function fetchAllNumeric(): array
             return [];
         }
 
-        $resultSet = pg_fetch_all($this->result, PGSQL_NUM);
-        // On PHP 7.4, pg_fetch_all() might return false for empty result sets.
-        if ($resultSet === false) {
-            return [];
-        }
-
         $types = $this->fetchNumericColumnTypes();
 
         return array_map(
             fn (array $row) => $this->mapNumericRow($row, $types),
-            $resultSet,
+            pg_fetch_all($this->result, PGSQL_NUM),
         );
     }
 
@@ -116,17 +95,11 @@ public function fetchAllAssociative(): array
             return [];
         }
 
-        $resultSet = pg_fetch_all($this->result, PGSQL_ASSOC);
-        // On PHP 7.4, pg_fetch_all() might return false for empty result sets.
-        if ($resultSet === false) {
-            return [];
-        }
-
         $types = $this->fetchAssociativeColumnTypes();
 
         return array_map(
             fn (array $row) => $this->mapAssociativeRow($row, $types),
-            $resultSet,
+            pg_fetch_all($this->result, PGSQL_ASSOC),
         );
     }
 
@@ -242,32 +215,17 @@ private function mapType(string $postgresType, ?string $value): string|int|float
             return null;
         }
 
-        switch ($postgresType) {
-            case 'bool':
-                switch ($value) {
-                    case 't':
-                        return true;
-                    case 'f':
-                        return false;
-                }
-
-                throw UnexpectedValue::new($value, $postgresType);
-
-            case 'bytea':
-                return hex2bin(substr($value, 2));
-
-            case 'float4':
-            case 'float8':
-                return (float) $value;
-
-            case 'int2':
-            case 'int4':
-                return (int) $value;
-
-            case 'int8':
-                return PHP_INT_SIZE >= 8 ? (int) $value : $value;
-        }
-
-        return $value;
+        return match ($postgresType) {
+            'bool' => match ($value) {
+                't' => true,
+                'f' => false,
+                default => throw UnexpectedValue::new($value, $postgresType),
+            },
+            'bytea' => hex2bin(substr($value, 2)),
+            'float4', 'float8' => (float) $value,
+            'int2', 'int4' => (int) $value,
+            'int8' => PHP_INT_SIZE >= 8 ? (int) $value : $value,
+            default => $value,
+        };
     }
 }

src/Driver/PgSQL/Statement.php

@@ -8,12 +8,8 @@
 use Doctrine\DBAL\Driver\Statement as StatementInterface;
 use Doctrine\DBAL\ParameterType;
 use PgSql\Connection as PgSqlConnection;
-use TypeError;
 
 use function assert;
-use function gettype;
-use function is_object;
-use function is_resource;
 use function ksort;
 use function pg_escape_bytea;
 use function pg_escape_identifier;
@@ -22,7 +18,6 @@
 use function pg_query;
 use function pg_result_error;
 use function pg_send_execute;
-use function sprintf;
 
 final class Statement implements StatementInterface
 {
@@ -32,19 +27,12 @@ final class Statement implements StatementInterface
     /** @psalm-var array<int, ParameterType> */
     private array $parameterTypes = [];
 
-    /**
-     * @param PgSqlConnection|resource $connection
-     * @param array<array-key, int>    $parameterMap
-     */
-    public function __construct(private mixed $connection, private string $name, private array $parameterMap)
-    {
-        if (! is_resource($connection) && ! $connection instanceof PgSqlConnection) {
-            throw new TypeError(sprintf(
-                'Expected connection to be a resource or an instance of %s, got %s.',
-                PgSqlConnection::class,
-                is_object($connection) ? $connection::class : gettype($connection),
-            ));
-        }
+    /** @param array<array-key, int> $parameterMap */
+    public function __construct(
+        private readonly PgSqlConnection $connection,
+        private readonly string $name,
+        private readonly array $parameterMap,
+    ) {
     }
 
     public function __destruct()
@@ -77,14 +65,12 @@ public function execute(): Result
 
         $escapedParameters = [];
         foreach ($this->parameters as $parameter => $value) {
-            switch ($this->parameterTypes[$parameter]) {
-                case ParameterType::BINARY:
-                case ParameterType::LARGE_OBJECT:
-                    $escapedParameters[] = $value === null ? null : pg_escape_bytea($this->connection, $value);
-                    break;
-                default:
-                    $escapedParameters[] = $value;
-            }
+            $escapedParameters[] = match ($this->parameterTypes[$parameter]) {
+                ParameterType::BINARY, ParameterType::LARGE_OBJECT => $value === null
+                    ? null
+                    : pg_escape_bytea($this->connection, $value),
+                default => $value,
+            };
         }
 
         if (@pg_send_execute($this->connection, $this->name, $escapedParameters) !== true) {