[SCA] Restrict the possible values for the casing options under the `…

…Portability\` NS
doctrine · Feb 26, 2023 · 6f4f3c0 · 6f4f3c0
1 parent 2c8afad
commit 6f4f3c0
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 32 deletions.
diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -611,6 +611,9 @@
             <errorLevel type="suppress">
                 <!-- See https://github.com/vimeo/psalm/issues/5472 -->
                 <file name="src/Portability/Converter.php"/>
+
+                <!-- See https://github.com/vimeo/psalm/issues/3399 -->
+                <file name="tests/Functional/PortabilityTest.php"/>
             </errorLevel>
         </InvalidDocblock>
         <InvalidNullableReturnType>

diff --git a/src/Portability/Converter.php b/src/Portability/Converter.php
@@ -10,8 +10,14 @@
 use function is_string;
 use function rtrim;
 
+use const CASE_LOWER;
+use const CASE_UPPER;
+
 final class Converter
 {
+    public const CASE_LOWER = CASE_LOWER;
+    public const CASE_UPPER = CASE_UPPER;
+
     /** @var callable */
     private $convertNumeric;
 
@@ -31,10 +37,12 @@ final class Converter
     private $convertFirstColumn;
 
     /**
-     * @param bool     $convertEmptyStringToNull Whether each empty string should be converted to NULL
-     * @param bool     $rightTrimString          Whether each string should right-trimmed
-     * @param int|null $case                     Convert the case of the column names
-     *                                           (one of {@see CASE_LOWER} and {@see CASE_UPPER})
+     * @param bool                                   $convertEmptyStringToNull Whether each empty string should
+     *                                                                         be converted to NULL
+     * @param bool                                   $rightTrimString          Whether each string should right-trimmed
+     * @param self::CASE_LOWER|self::CASE_UPPER|null $case                     Convert the case of the column names
+     *                                                                         (one of {@see self::CASE_LOWER} and
+     *                                                                         {@see self::CASE_UPPER})
      */
     public function __construct(bool $convertEmptyStringToNull, bool $rightTrimString, ?int $case)
     {
@@ -182,8 +190,8 @@ private function createConvertValue(bool $convertEmptyStringToNull, bool $rightT
     /**
      * Creates a function that will convert each array-row retrieved from the database
      *
-     * @param callable|null $function The function that will convert each value
-     * @param int|null      $case     Column name case
+     * @param callable|null                          $function The function that will convert each value
+     * @param self::CASE_LOWER|self::CASE_UPPER|null $case     Column name case
      *
      * @return callable|null The resulting function or NULL if no conversion is needed
      */

diff --git a/src/Portability/Driver.php b/src/Portability/Driver.php
@@ -11,15 +11,19 @@
 
 use function method_exists;
 
-use const CASE_LOWER;
-use const CASE_UPPER;
-
 final class Driver extends AbstractDriverMiddleware
 {
     private int $mode;
 
+    /** @var 0|ColumnCase::LOWER|ColumnCase::UPPER */
     private int $case;
 
+    /**
+     * @param 0|ColumnCase::LOWER|ColumnCase::UPPER $case Determines how the column case will be treated.
+     *                                                    0: The case will be left as is in the database.
+     *                                                    {@see ColumnCase::LOWER}: The case will be lowercased.
+     *                                                    {@see ColumnCase::UPPER}: The case will be uppercased.
+     */
     public function __construct(DriverInterface $driver, int $mode, int $case)
     {
         parent::__construct($driver);
@@ -55,9 +59,12 @@ public function connect(
 
             if ($nativeConnection instanceof PDO) {
                 $portability &= ~Connection::PORTABILITY_FIX_CASE;
-                $nativeConnection->setAttribute(PDO::ATTR_CASE, $this->case);
+                $nativeConnection->setAttribute(
+                    PDO::ATTR_CASE,
+                    $this->case === ColumnCase::LOWER ? PDO::CASE_LOWER : PDO::CASE_UPPER
+                );
             } else {
-                $case = $this->case === ColumnCase::LOWER ? CASE_LOWER : CASE_UPPER;
+                $case = $this->case === ColumnCase::LOWER ? Converter::CASE_LOWER : Converter::CASE_UPPER;
             }
         }
 

diff --git a/src/Portability/Middleware.php b/src/Portability/Middleware.php
@@ -4,15 +4,18 @@
 
 namespace Doctrine\DBAL\Portability;
 
+use Doctrine\DBAL\ColumnCase;
 use Doctrine\DBAL\Driver as DriverInterface;
 use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
 
 final class Middleware implements MiddlewareInterface
 {
     private int $mode;
 
+    /** @var 0|ColumnCase::LOWER|ColumnCase::UPPER */
     private int $case;
 
+    /** @param 0|ColumnCase::LOWER|ColumnCase::UPPER $case */
     public function __construct(int $mode, int $case)
     {
         $this->mode = $mode;

diff --git a/src/Portability/OptimizeFlags.php b/src/Portability/OptimizeFlags.php
@@ -17,7 +17,7 @@ final class OptimizeFlags
      * Platform-specific portability flags that need to be excluded from the user-provided mode
      * since the platform already operates in this mode to avoid unnecessary conversion overhead.
      *
-     * @var array<string,int>
+     * @var array<class-string, int>
      */
     private static array $platforms = [
         DB2Platform::class        => 0,

diff --git a/tests/Functional/PortabilityTest.php b/tests/Functional/PortabilityTest.php
@@ -46,7 +46,8 @@ public function testFullFetchMode(): void
     }
 
     /**
-     * @param list<string> $expected
+     * @param 0|ColumnCase::LOWER|ColumnCase::UPPER $case
+     * @param list<string>                          $expected
      *
      * @dataProvider caseProvider
      */
@@ -61,7 +62,7 @@ public function testCaseConversion(int $case, array $expected): void
         self::assertSame($expected, array_keys($row));
     }
 
-    /** @return iterable<string, array{int, list<string>}> */
+    /** @return iterable<string, array{ColumnCase::LOWER|ColumnCase::UPPER, list<string>}> */
     public static function caseProvider(): iterable
     {
         yield 'lower' => [ColumnCase::LOWER, ['test_int', 'test_string', 'test_null']];
@@ -137,6 +138,7 @@ public function testGetDatabaseName(): void
         self::assertNotNull($this->connection->getDatabase());
     }
 
+     /** @param 0|ColumnCase::LOWER|ColumnCase::UPPER $case */
     private function connectWithPortability(int $mode, int $case): void
     {
         // closing the default connection prior to 4.0.0 to prevent connection leak

diff --git a/tests/Portability/ConnectionTest.php b/tests/Portability/ConnectionTest.php
@@ -18,7 +18,7 @@ public function testGetServerVersion(): void
             ->method('getServerVersion')
             ->willReturn('1.2.3');
 
-        $connection = new Connection($driverConnection, new Converter(false, false, 0));
+        $connection = new Connection($driverConnection, new Converter(false, false, Converter::CASE_LOWER));
 
         self::assertSame('1.2.3', $connection->getServerVersion());
     }
@@ -27,7 +27,7 @@ public function testGetServerVersionFailsWithLegacyConnection(): void
     {
         $connection = new Connection(
             $this->createMock(DriverConnection::class),
-            new Converter(false, false, 0),
+            new Converter(false, false, Converter::CASE_LOWER),
         );
 
         $this->expectException(LogicException::class);
@@ -43,7 +43,7 @@ public function testGetNativeConnection(): void
         $driverConnection->method('getNativeConnection')
             ->willReturn($nativeConnection);
 
-        $connection = new Connection($driverConnection, new Converter(false, false, 0));
+        $connection = new Connection($driverConnection, new Converter(false, false, Converter::CASE_LOWER));
 
         self::assertSame($nativeConnection, $connection->getNativeConnection());
     }
@@ -52,7 +52,7 @@ public function testGetNativeConnectionFailsWithLegacyConnection(): void
     {
         $connection = new Connection(
             $this->createMock(DriverConnection::class),
-            new Converter(false, false, 0),
+            new Converter(false, false, Converter::CASE_LOWER),
         );
 
         $this->expectException(LogicException::class);

diff --git a/tests/Portability/ConverterTest.php b/tests/Portability/ConverterTest.php
@@ -5,8 +5,6 @@
 use Doctrine\DBAL\Portability\Converter;
 use PHPUnit\Framework\TestCase;
 
-use const CASE_LOWER;
-
 class ConverterTest extends TestCase
 {
     /**
@@ -61,8 +59,9 @@ public static function convertNumericProvider(): iterable
     }
 
     /**
-     * @param array<string,mixed>|false $row
-     * @param array<string,mixed>|false $expected
+     * @param array<string,mixed>|false                        $row
+     * @param Converter::CASE_LOWER|Converter::CASE_UPPER|null $case
+     * @param array<string,mixed>|false                        $expected
      *
      * @dataProvider convertAssociativeProvider
      */
@@ -136,7 +135,7 @@ public static function convertAssociativeProvider(): iterable
             $row,
             false,
             false,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 'foo' => '',
                 'bar' => 'X ',
@@ -147,7 +146,7 @@ public static function convertAssociativeProvider(): iterable
             $row,
             false,
             true,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 'foo' => '',
                 'bar' => 'X',
@@ -158,7 +157,7 @@ public static function convertAssociativeProvider(): iterable
             $row,
             true,
             false,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 'foo' => null,
                 'bar' => 'X ',
@@ -169,7 +168,7 @@ public static function convertAssociativeProvider(): iterable
             $row,
             true,
             true,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 'foo' => null,
                 'bar' => 'X',
@@ -276,8 +275,9 @@ public static function convertAllNumericProvider(): iterable
     }
 
     /**
-     * @param list<array<string,mixed>> $row
-     * @param list<array<string,mixed>> $expected
+     * @param list<array<string,mixed>>                        $row
+     * @param Converter::CASE_LOWER|Converter::CASE_UPPER|null $case
+     * @param list<array<string,mixed>>                        $expected
      *
      * @dataProvider convertAllAssociativeProvider
      */
@@ -381,7 +381,7 @@ public static function convertAllAssociativeProvider(): iterable
             $data,
             false,
             false,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 [
                     'foo' => 'X ',
@@ -398,7 +398,7 @@ public static function convertAllAssociativeProvider(): iterable
             $data,
             false,
             true,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 [
                     'foo' => 'X',
@@ -415,7 +415,7 @@ public static function convertAllAssociativeProvider(): iterable
             $data,
             true,
             false,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 [
                     'foo' => 'X ',
@@ -432,7 +432,7 @@ public static function convertAllAssociativeProvider(): iterable
             $data,
             true,
             true,
-            CASE_LOWER,
+            Converter::CASE_LOWER,
             [
                 [
                     'foo' => 'X',
@@ -499,6 +499,7 @@ public static function convertFirstColumnProvider(): iterable
         ];
     }
 
+    /** @param Converter::CASE_LOWER|Converter::CASE_UPPER|null $case */
     private function createConverter(bool $convertEmptyStringToNull, bool $rightTrimString, ?int $case): Converter
     {
         return new Converter($convertEmptyStringToNull, $rightTrimString, $case);