Skip to content

Commit ad8dabe

Browse files
morozovmorozov
authored
Merge pull request #6799 from morozov/index-deprecations
Fix false-positive primary key column length deprecation
2 parents 113744f + deddf53 commit ad8dabe

File tree

7 files changed

+103
-89
lines changed

7 files changed

+103
-89
lines changed

‎UPGRADE.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ The following `Index` usage scenarios have been deprecated:
1717
3. Using qualified or otherwise invalid column names in index columns
1818
4. Using other values than positive integers as index column lengths
1919
5. Using nullable columns in a primary key index
20+
6. Extending the `Index` class has been deprecated. Use the `Index` class directly.
2021

2122
## Deprecated passing unquoted names containing dots for table introspection on platforms that don't support schemas
2223

‎src/Schema/ForeignKeyConstraint.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class ForeignKeyConstraint extends AbstractOptionallyNamedObject
3838
*
3939
* @deprecated
4040
*
41-
* @var array<string, Identifier>
41+
* @var non-empty-array<string, Identifier>
4242
*/
4343
protected array $_localColumnNames;
4444

@@ -54,7 +54,7 @@ class ForeignKeyConstraint extends AbstractOptionallyNamedObject
5454
*
5555
* @deprecated
5656
*
57-
* @var array<string, Identifier>
57+
* @var non-empty-array<string, Identifier>
5858
*/
5959
protected array $_foreignColumnNames;
6060

@@ -276,9 +276,9 @@ public function getDeferrability(): Deferrability
276276
}
277277

278278
/**
279-
* @param array<int, string> $names
279+
* @param non-empty-array<int, string> $names
280280
*
281-
* @return array<string, Identifier>
281+
* @return non-empty-array<string, Identifier>
282282
*/
283283
private function createIdentifierMap(array $names): array
284284
{
@@ -297,7 +297,7 @@ private function createIdentifierMap(array $names): array
297297
*
298298
* @deprecated Use {@see getReferencingColumnNames()} instead.
299299
*
300-
* @return array<int, string>
300+
* @return non-empty-list<string>
301301
*/
302302
public function getLocalColumns(): array
303303
{
@@ -323,7 +323,7 @@ public function getLocalColumns(): array
323323
*
324324
* @param AbstractPlatform $platform The platform to use for quotation.
325325
*
326-
* @return array<int, string>
326+
* @return non-empty-array<int, string>
327327
*/
328328
public function getQuotedLocalColumns(AbstractPlatform $platform): array
329329
{
@@ -348,7 +348,7 @@ public function getQuotedLocalColumns(AbstractPlatform $platform): array
348348
*
349349
* Returns unquoted representation of local table column names for comparison with other FK
350350
*
351-
* @return array<int, string>
351+
* @return non-empty-array<int, string>
352352
*/
353353
public function getUnquotedLocalColumns(): array
354354
{
@@ -367,7 +367,7 @@ public function getUnquotedLocalColumns(): array
367367
*
368368
* Returns unquoted representation of foreign table column names for comparison with other FK
369369
*
370-
* @return array<int, string>
370+
* @return non-empty-array<int, string>
371371
*/
372372
public function getUnquotedForeignColumns(): array
373373
{
@@ -457,7 +457,7 @@ public function getQuotedForeignTableName(AbstractPlatform $platform): string
457457
* Returns the names of the referenced table columns
458458
* the foreign key constraint is associated with.
459459
*
460-
* @return array<int, string>
460+
* @return non-empty-array<int, string>
461461
*/
462462
public function getForeignColumns(): array
463463
{
@@ -483,7 +483,7 @@ public function getForeignColumns(): array
483483
*
484484
* @param AbstractPlatform $platform The platform to use for quotation.
485485
*
486-
* @return array<int, string>
486+
* @return non-empty-array<int, string>
487487
*/
488488
public function getQuotedForeignColumns(AbstractPlatform $platform): array
489489
{

‎src/Schema/Index.php‎

Lines changed: 63 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,11 @@
2424
use function is_object;
2525
use function strtolower;
2626

27-
/** @extends AbstractNamedObject<UnqualifiedName> */
28-
class Index extends AbstractNamedObject
27+
/**
28+
* @final
29+
* @extends AbstractNamedObject<UnqualifiedName>
30+
*/
31+
final class Index extends AbstractNamedObject
2932
{
3033
/**
3134
* Asset identifier instances of the column names the index is associated with.
@@ -55,9 +58,9 @@ class Index extends AbstractNamedObject
5558
private readonly array $columns;
5659

5760
/**
58-
* @param array<int, string> $columns
59-
* @param array<int, string> $flags
60-
* @param array<string, mixed> $options
61+
* @param non-empty-list<string> $columns
62+
* @param array<int, string> $flags
63+
* @param array<string, mixed> $options
6164
*/
6265
public function __construct(
6366
?string $name,
@@ -80,16 +83,6 @@ public function __construct(
8083
$this->_isUnique = $isUnique || $isPrimary;
8184
$this->_isPrimary = $isPrimary;
8285

83-
$lengths = $options['lengths'] ?? [];
84-
85-
if ($isPrimary && count($lengths) > 0) {
86-
Deprecation::trigger(
87-
'doctrine/dbal',
88-
'https://github.com/doctrine/dbal/pull/6787',
89-
'Declaring column length for primary key indexes is deprecated.',
90-
);
91-
}
92-
9386
foreach ($columns as $column) {
9487
$this->_addColumn($column);
9588
}
@@ -98,7 +91,7 @@ public function __construct(
9891
$this->addFlag($flag);
9992
}
10093

101-
$this->columns = $this->parseColumns($columns, $options['lengths'] ?? []);
94+
$this->columns = $this->parseColumns($isPrimary, $columns, $options['lengths'] ?? []);
10295
}
10396

10497
protected function getNameParser(): UnqualifiedNameParser
@@ -128,10 +121,11 @@ protected function _addColumn(string $column): void
128121
/**
129122
* Returns the names of the referencing table columns the constraint is associated with.
130123
*
131-
* @return list<string>
124+
* @return non-empty-list<string>
132125
*/
133126
public function getColumns(): array
134127
{
128+
/** @phpstan-ignore return.type */
135129
return array_keys($this->_columns);
136130
}
137131

@@ -339,60 +333,71 @@ public function getOptions(): array
339333
}
340334

341335
/**
342-
* @param array<string> $columnNames
343-
* @param array<int> $lengths
336+
* @param non-empty-array<int, string> $columnNames
337+
* @param array<int> $lengths
344338
*
345339
* @return list<IndexedColumn>
346340
*/
347-
private function parseColumns(array $columnNames, array $lengths): array
341+
private function parseColumns(bool $isPrimary, array $columnNames, array $lengths): array
348342
{
349343
$columns = [];
350344

351345
$parser = Parsers::getUnqualifiedNameParser();
352346

353-
try {
354-
foreach ($columnNames as $columnName) {
355-
$name = $parser->parse($columnName);
356-
$length = array_shift($lengths);
357-
358-
if ($length !== null) {
359-
if (! is_int($length)) {
360-
Deprecation::trigger(
361-
'doctrine/dbal',
362-
'https://github.com/doctrine/dbal/pull/6787',
363-
'Indexed column length should be an integer, %s given.',
364-
is_object($length) ? $length::class : gettype($length),
365-
);
366-
367-
$length = (int) $length;
368-
}
369-
370-
if ($length < 1) {
371-
Deprecation::trigger(
372-
'doctrine/dbal',
373-
'https://github.com/doctrine/dbal/pull/6787',
374-
'Indexed column length should be a positive integer, %d given.',
375-
$length,
376-
);
377-
378-
return [];
379-
}
347+
foreach ($columnNames as $columnName) {
348+
try {
349+
$parsedName = $parser->parse($columnName);
350+
} catch (Throwable $e) {
351+
Deprecation::trigger(
352+
'doctrine/dbal',
353+
'https://github.com/doctrine/dbal/pull/6787',
354+
'Unable to parse column name: %s.',
355+
$e->getMessage(),
356+
);
357+
358+
return [];
359+
}
360+
361+
$length = array_shift($lengths);
362+
363+
if ($length !== null) {
364+
if ($isPrimary) {
365+
Deprecation::trigger(
366+
'doctrine/dbal',
367+
'https://github.com/doctrine/dbal/pull/6787',
368+
'Declaring column length for primary key indexes is deprecated.',
369+
);
370+
371+
return [];
372+
}
373+
374+
if (! is_int($length)) {
375+
Deprecation::trigger(
376+
'doctrine/dbal',
377+
'https://github.com/doctrine/dbal/pull/6787',
378+
'Indexed column length should be an integer, %s given.',
379+
is_object($length) ? $length::class : gettype($length),
380+
);
381+
382+
$length = (int) $length;
380383
}
381384

382-
$columns[] = new IndexedColumn($name, $length);
383-
}
385+
if ($length < 1) {
386+
Deprecation::trigger(
387+
'doctrine/dbal',
388+
'https://github.com/doctrine/dbal/pull/6787',
389+
'Indexed column length should be a positive integer, %d given.',
390+
$length,
391+
);
384392

385-
return $columns;
386-
} catch (Throwable $e) {
387-
Deprecation::trigger(
388-
'doctrine/dbal',
389-
'https://github.com/doctrine/dbal/pull/6787',
390-
'Unable to parse column name: %s.',
391-
$e->getMessage(),
392-
);
393+
return [];
394+
}
395+
}
393396

394-
return [];
397+
$columns[] = new IndexedColumn($parsedName, $length);
395398
}
399+
400+
return $columns;
396401
}
397402

398403
/**

‎src/Schema/Table.php‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public function setSchemaConfig(SchemaConfig $schemaConfig): void
139139
/**
140140
* Sets the Primary Key.
141141
*
142-
* @param array<int, string> $columnNames
142+
* @param non-empty-list<string> $columnNames
143143
*/
144144
public function setPrimaryKey(array $columnNames, ?string $indexName = null): self
145145
{
@@ -187,9 +187,9 @@ public function addUniqueConstraint(
187187
}
188188

189189
/**
190-
* @param array<int, string> $columnNames
191-
* @param array<int, string> $flags
192-
* @param array<string, mixed> $options
190+
* @param non-empty-list<string> $columnNames
191+
* @param array<int, string> $flags
192+
* @param array<string, mixed> $options
193193
*/
194194
public function addIndex(
195195
array $columnNames,
@@ -234,8 +234,8 @@ public function dropIndex(string $name): void
234234
}
235235

236236
/**
237-
* @param array<int, string> $columnNames
238-
* @param array<string, mixed> $options
237+
* @param non-empty-list<string> $columnNames
238+
* @param array<string, mixed> $options
239239
*/
240240
public function addUniqueIndex(array $columnNames, ?string $indexName = null, array $options = []): self
241241
{
@@ -915,9 +915,9 @@ private function _createUniqueConstraint(
915915
}
916916

917917
/**
918-
* @param array<int, string> $columns
919-
* @param array<int, string> $flags
920-
* @param array<string, mixed> $options
918+
* @param non-empty-list<string> $columns
919+
* @param array<int, string> $flags
920+
* @param array<string, mixed> $options
921921
*/
922922
private function _createIndex(
923923
array $columns,

‎src/Schema/UniqueConstraint.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public function getColumnNames(): array
120120
/**
121121
* @deprecated Use {@see getColumnNames()} instead.
122122
*
123-
* @return list<string>
123+
* @return non-empty-list<string>
124124
*/
125125
public function getColumns(): array
126126
{
@@ -131,6 +131,7 @@ public function getColumns(): array
131131
__METHOD__,
132132
);
133133

134+
/** @phpstan-ignore return.type */
134135
return array_keys($this->columns);
135136
}
136137

‎tests/Schema/ForeignKeyConstraintTest.php‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,13 @@ class ForeignKeyConstraintTest extends TestCase
2222
{
2323
use VerifyDeprecations;
2424

25-
/** @param string[] $indexColumns */
25+
/** @param non-empty-list<string> $indexColumns */
2626
#[DataProvider('getIntersectsIndexColumnsData')]
2727
public function testIntersectsIndexColumns(array $indexColumns, bool $expectedResult): void
2828
{
2929
$foreignKey = new ForeignKeyConstraint(['foo', 'bar'], 'foreign_table', ['fk_foo', 'fk_bar']);
3030

31-
$index = $this->getMockBuilder(Index::class)
32-
->disableOriginalConstructor()
33-
->getMock();
34-
$index->expects(self::once())
35-
->method('getColumns')
36-
->willReturn($indexColumns);
31+
$index = new Index('foo', $indexColumns);
3732

3833
self::assertSame($expectedResult, $foreignKey->intersectsIndexColumns($index));
3934
}

‎tests/Schema/IndexTest.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ public function testOverrulesWithPartial(): void
111111
}
112112

113113
/**
114-
* @param string[] $columns
115-
* @param int[]|null[] $lengths1
116-
* @param int[]|null[] $lengths2
114+
* @param non-empty-list<string> $columns
115+
* @param list<?int> $lengths1
116+
* @param list<?int> $lengths2
117117
*/
118118
#[DataProvider('indexLengthProvider')]
119119
public function testFulfilledWithLength(array $columns, array $lengths1, array $lengths2, bool $expected): void
@@ -204,6 +204,7 @@ public function testEmptyColumns(): void
204204
{
205205
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
206206

207+
/** @phpstan-ignore argument.type */
207208
$index = new Index('idx_user_name', []);
208209

209210
$this->expectException(InvalidState::class);
@@ -226,7 +227,18 @@ public function testPrimaryKeyWithColumnLength(): void
226227
{
227228
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
228229

229-
new Index('primary', ['id'], false, true, [], ['lengths' => [32]]);
230+
$index = new Index('primary', ['id'], false, true, [], ['lengths' => [32]]);
231+
232+
$this->expectException(InvalidState::class);
233+
234+
$index->getIndexedColumns();
235+
}
236+
237+
public function testPrimaryKeyWithNullColumnLength(): void
238+
{
239+
$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6787');
240+
241+
new Index('primary', ['id'], false, true, [], ['lengths' => [null]]);
230242
}
231243

232244
public function testNonIntegerColumnLength(): void

0 commit comments

Comments
 (0)