Declare internal properties of TableDiff as private #5753
Conversation
src/Schema/Comparator.php
Outdated
| @@ -309,17 +307,17 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff | |||
|
|
|||
| return new TableDiff( | |||
| $fromTable, | |||
| $addedColumns, | |||
| array_values($addedColumns), | |||
There was a problem hiding this comment.
Why is this call to array_values needed now that this type has been relaxed?
There was a problem hiding this comment.
I still didn't want the keys of the associative arrays to leak into the diff, and some tests relied on the numeric keys for assertions. But this can be fixed.
| * @param array<Index> $modifiedIndexes | ||
| * @param array<Index> $droppedIndexes | ||
| * @param array<string, Index> $renamedIndexes | ||
| * @param array<ForeignKeyConstraint> $addedForeignKeys |
There was a problem hiding this comment.
I wish there was a way to express that this is a set explicitely (list is great at that but does have an extra constraint we don't need). Now it looks like we forgot to document the keys.
There was a problem hiding this comment.
We will likely introduce a dedicated data structure for these collections to address #4772. Currently, table constraints and indexes are indexed by the lower-case name but this doesn't work for case-sensitive platforms.
morozov
force-pushed
the
table-diff-internal-properties-private
branch
from
a433d60 to
26ece99
Compare
Change summary:
TableDiffconstructor parameters reordered by asset type: columns, indexes, foreign keys.getRenamed*()have been relaxed fromlist<T>toarray<T>. The former implies that the elements of the list can be accessed by index and their order matters while this is not intended (those are just sets).