Fix false-positive primary key column length deprecation

UPGRADE.md

@@ -17,6 +17,7 @@ The following `Index` usage scenarios have been deprecated:
 3. Using qualified or otherwise invalid column names in index columns
 4. Using other values than positive integers as index column lengths
 5. Using nullable columns in a primary key index
+6. Extending the `Index` class has been deprecated. Use the `Index` class directly.
 
 ## Deprecated passing unquoted names containing dots for table introspection on platforms that don't support schemas
 

src/Schema/ForeignKeyConstraint.php

@@ -38,7 +38,7 @@ class ForeignKeyConstraint extends AbstractOptionallyNamedObject
      *
      * @deprecated
      *
-     * @var array<string, Identifier>
+     * @var non-empty-array<string, Identifier>
      */
     protected array $_localColumnNames;
 
@@ -54,7 +54,7 @@ class ForeignKeyConstraint extends AbstractOptionallyNamedObject
      *
      * @deprecated
      *
-     * @var array<string, Identifier>
+     * @var non-empty-array<string, Identifier>
      */
     protected array $_foreignColumnNames;
 
@@ -276,9 +276,9 @@ public function getDeferrability(): Deferrability
     }
 
     /**
-     * @param array<int, string> $names
+     * @param non-empty-array<int, string> $names
      *
-     * @return array<string, Identifier>
+     * @return non-empty-array<string, Identifier>
      */
     private function createIdentifierMap(array $names): array
     {
@@ -297,7 +297,7 @@ private function createIdentifierMap(array $names): array
      *
      * @deprecated Use {@see getReferencingColumnNames()} instead.
      *
-     * @return array<int, string>
+     * @return non-empty-list<string>
      */
     public function getLocalColumns(): array
     {
@@ -323,7 +323,7 @@ public function getLocalColumns(): array
      *
      * @param AbstractPlatform $platform The platform to use for quotation.
      *
-     * @return array<int, string>
+     * @return non-empty-array<int, string>
      */
     public function getQuotedLocalColumns(AbstractPlatform $platform): array
     {
@@ -348,7 +348,7 @@ public function getQuotedLocalColumns(AbstractPlatform $platform): array
      *
      * Returns unquoted representation of local table column names for comparison with other FK
      *
-     * @return array<int, string>
+     * @return non-empty-array<int, string>
      */
     public function getUnquotedLocalColumns(): array
     {
@@ -367,7 +367,7 @@ public function getUnquotedLocalColumns(): array
      *
      * Returns unquoted representation of foreign table column names for comparison with other FK
      *
-     * @return array<int, string>
+     * @return non-empty-array<int, string>
      */
     public function getUnquotedForeignColumns(): array
     {
@@ -457,7 +457,7 @@ public function getQuotedForeignTableName(AbstractPlatform $platform): string
      * Returns the names of the referenced table columns
      * the foreign key constraint is associated with.
      *
-     * @return array<int, string>
+     * @return non-empty-array<int, string>
      */
     public function getForeignColumns(): array
     {
@@ -483,7 +483,7 @@ public function getForeignColumns(): array
      *
      * @param AbstractPlatform $platform The platform to use for quotation.
      *
-     * @return array<int, string>
+     * @return non-empty-array<int, string>
      */
     public function getQuotedForeignColumns(AbstractPlatform $platform): array
     {

src/Schema/Index.php

@@ -24,8 +24,11 @@
 use function is_object;
 use function strtolower;
 
-/** @extends AbstractNamedObject<UnqualifiedName> */
-class Index extends AbstractNamedObject
+/**
+ * @final
+ * @extends AbstractNamedObject<UnqualifiedName>
+ */
+final class Index extends AbstractNamedObject
 {
     /**
      * Asset identifier instances of the column names the index is associated with.
@@ -55,9 +58,9 @@ class Index extends AbstractNamedObject
     private readonly array $columns;
 
     /**
-     * @param array<int, string>   $columns
-     * @param array<int, string>   $flags
-     * @param array<string, mixed> $options
+     * @param non-empty-list<string> $columns
+     * @param array<int, string>     $flags
+     * @param array<string, mixed>   $options
      */
     public function __construct(
         ?string $name,
@@ -80,16 +83,6 @@ public function __construct(
         $this->_isUnique  = $isUnique || $isPrimary;
         $this->_isPrimary = $isPrimary;
 
-        $lengths = $options['lengths'] ?? [];
-
-        if ($isPrimary && count($lengths) > 0) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/6787',
-                'Declaring column length for primary key indexes is deprecated.',
-            );
-        }
-
         foreach ($columns as $column) {
             $this->_addColumn($column);
         }
@@ -98,7 +91,7 @@ public function __construct(
             $this->addFlag($flag);
         }
 
-        $this->columns = $this->parseColumns($columns, $options['lengths'] ?? []);
+        $this->columns = $this->parseColumns($isPrimary, $columns, $options['lengths'] ?? []);
     }
 
     protected function getNameParser(): UnqualifiedNameParser
@@ -128,10 +121,11 @@ protected function _addColumn(string $column): void
     /**
      * Returns the names of the referencing table columns the constraint is associated with.
      *
-     * @return list<string>
+     * @return non-empty-list<string>
      */
     public function getColumns(): array
     {
+        /** @phpstan-ignore return.type */
         return array_keys($this->_columns);
     }
 
@@ -339,60 +333,71 @@ public function getOptions(): array
     }
 
     /**
-     * @param array<string> $columnNames
-     * @param array<int>    $lengths
+     * @param non-empty-array<int, string> $columnNames
+     * @param array<int>                   $lengths
      *
      * @return list<IndexedColumn>
      */
-    private function parseColumns(array $columnNames, array $lengths): array
+    private function parseColumns(bool $isPrimary, array $columnNames, array $lengths): array
     {
         $columns = [];
 
         $parser = Parsers::getUnqualifiedNameParser();
 
-        try {
-            foreach ($columnNames as $columnName) {
-                $name   = $parser->parse($columnName);
-                $length = array_shift($lengths);
-
-                if ($length !== null) {
-                    if (! is_int($length)) {
-                        Deprecation::trigger(
-                            'doctrine/dbal',
-                            'https://github.com/doctrine/dbal/pull/6787',
-                            'Indexed column length should be an integer, %s given.',
-                            is_object($length) ? $length::class : gettype($length),
-                        );
-
-                        $length = (int) $length;
-                    }
-
-                    if ($length < 1) {
-                        Deprecation::trigger(
-                            'doctrine/dbal',
-                            'https://github.com/doctrine/dbal/pull/6787',
-                            'Indexed column length should be a positive integer, %d given.',
-                            $length,
-                        );
-
-                        return [];
-                    }
+        foreach ($columnNames as $columnName) {
+            try {
+                $parsedName = $parser->parse($columnName);
+            } catch (Throwable $e) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/6787',
+                    'Unable to parse column name: %s.',
+                    $e->getMessage(),
+                );
+
+                return [];
+            }
+
+            $length = array_shift($lengths);
+
+            if ($length !== null) {
+                if ($isPrimary) {
+                    Deprecation::trigger(
+                        'doctrine/dbal',
+                        'https://github.com/doctrine/dbal/pull/6787',
+                        'Declaring column length for primary key indexes is deprecated.',
+                    );
+
+                    return [];
+                }
+
+                if (! is_int($length)) {
+                    Deprecation::trigger(
+                        'doctrine/dbal',
+                        'https://github.com/doctrine/dbal/pull/6787',
+                        'Indexed column length should be an integer, %s given.',
+                        is_object($length) ? $length::class : gettype($length),
+                    );
+
+                    $length = (int) $length;
                 }
 
-                $columns[] = new IndexedColumn($name, $length);
-            }
+                if ($length < 1) {
+                    Deprecation::trigger(
+                        'doctrine/dbal',
+                        'https://github.com/doctrine/dbal/pull/6787',
+                        'Indexed column length should be a positive integer, %d given.',
+                        $length,
+                    );
 
-            return $columns;
-        } catch (Throwable $e) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/6787',
-                'Unable to parse column name: %s.',
-                $e->getMessage(),
-            );
+                    return [];
+                }
+            }
 
-            return [];
+            $columns[] = new IndexedColumn($parsedName, $length);
         }
+
+        return $columns;
     }
 
     /**

src/Schema/Table.php

@@ -139,7 +139,7 @@ public function setSchemaConfig(SchemaConfig $schemaConfig): void
     /**
      * Sets the Primary Key.
      *
-     * @param array<int, string> $columnNames
+     * @param non-empty-list<string> $columnNames
      */
     public function setPrimaryKey(array $columnNames, ?string $indexName = null): self
     {
@@ -187,9 +187,9 @@ public function addUniqueConstraint(
     }
 
     /**
-     * @param array<int, string>   $columnNames
-     * @param array<int, string>   $flags
-     * @param array<string, mixed> $options
+     * @param non-empty-list<string> $columnNames
+     * @param array<int, string>     $flags
+     * @param array<string, mixed>   $options
      */
     public function addIndex(
         array $columnNames,
@@ -234,8 +234,8 @@ public function dropIndex(string $name): void
     }
 
     /**
-     * @param array<int, string>   $columnNames
-     * @param array<string, mixed> $options
+     * @param non-empty-list<string> $columnNames
+     * @param array<string, mixed>   $options
      */
     public function addUniqueIndex(array $columnNames, ?string $indexName = null, array $options = []): self
     {
@@ -915,9 +915,9 @@ private function _createUniqueConstraint(
     }
 
     /**
-     * @param array<int, string>   $columns
-     * @param array<int, string>   $flags
-     * @param array<string, mixed> $options
+     * @param non-empty-list<string> $columns
+     * @param array<int, string>     $flags
+     * @param array<string, mixed>   $options
      */
     private function _createIndex(
         array $columns,

src/Schema/UniqueConstraint.php

@@ -120,7 +120,7 @@ public function getColumnNames(): array
     /**
      * @deprecated Use {@see getColumnNames()} instead.
      *
-     * @return list<string>
+     * @return non-empty-list<string>
      */
     public function getColumns(): array
     {
@@ -131,6 +131,7 @@ public function getColumns(): array
             __METHOD__,
         );
 
+        /** @phpstan-ignore return.type */
         return array_keys($this->columns);
     }
 

tests/Schema/ForeignKeyConstraintTest.php

@@ -22,18 +22,13 @@ class ForeignKeyConstraintTest extends TestCase
 {
     use VerifyDeprecations;
 
-    /** @param string[] $indexColumns */
+    /** @param non-empty-list<string> $indexColumns */
     #[DataProvider('getIntersectsIndexColumnsData')]
     public function testIntersectsIndexColumns(array $indexColumns, bool $expectedResult): void
     {
         $foreignKey = new ForeignKeyConstraint(['foo', 'bar'], 'foreign_table', ['fk_foo', 'fk_bar']);
 
-        $index = $this->getMockBuilder(Index::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $index->expects(self::once())
-            ->method('getColumns')
-            ->willReturn($indexColumns);
+        $index = new Index('foo', $indexColumns);
 
         self::assertSame($expectedResult, $foreignKey->intersectsIndexColumns($index));
     }

tests/Schema/IndexTest.php

@@ -111,9 +111,9 @@ public function testOverrulesWithPartial(): void
     }
 
     /**
-     * @param string[]     $columns
-     * @param int[]|null[] $lengths1
-     * @param int[]|null[] $lengths2
+     * @param non-empty-list<string> $columns
+     * @param list<?int>             $lengths1
+     * @param list<?int>             $lengths2
      */
     #[DataProvider('indexLengthProvider')]
     public function testFulfilledWithLength(array $columns, array $lengths1, array $lengths2, bool $expected): void
@@ -204,6 +204,7 @@ public function testEmptyColumns(): void
     {
         $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
 
+        /** @phpstan-ignore argument.type */
         $index = new Index('idx_user_name', []);
 
         $this->expectException(InvalidState::class);
@@ -226,7 +227,18 @@ public function testPrimaryKeyWithColumnLength(): void
     {
         $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
 
-        new Index('primary', ['id'], false, true, [], ['lengths' => [32]]);
+        $index = new Index('primary', ['id'], false, true, [], ['lengths' => [32]]);
+
+        $this->expectException(InvalidState::class);
+
+        $index->getIndexedColumns();
+    }
+
+    public function testPrimaryKeyWithNullColumnLength(): void
+    {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
+
+        new Index('primary', ['id'], false, true, [], ['lengths' => [null]]);
     }
 
     public function testNonIntegerColumnLength(): void