Use a more specific type for getSqlStatements()

[greg0ire]

It is strictly beneficial for the Psalm baseline.

[mpdude]

orm/lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php

Lines 90 to 116 in 82533af

	$i = -1;
	foreach (array_reverse($classNames) as $className) {
	$affected = false;
	$class = $em->getClassMetadata($className);
	$updateSql = 'UPDATE ' . $quoteStrategy->getTableName($class, $platform) . ' SET ';
	foreach ($updateItems as $updateItem) {
	$field = $updateItem->pathExpression->field;
	if (
	(isset($class->fieldMappings[$field]) && ! isset($class->fieldMappings[$field]['inherited'])) ||
	(isset($class->associationMappings[$field]) && ! isset($class->associationMappings[$field]['inherited']))
	) {
	$newValue = $updateItem->newValue;
	if (! $affected) {
	$affected = true;
	++$i;
	} else {
	$updateSql .= ', ';
	}
	$updateSql .= $sqlWalker->walkUpdateItem($updateItem);
	if ($newValue instanceof AST\InputParameter) {
	$this->sqlParameters[$i][] = $newValue->name;

Not sure this is guaranteed to be a list... It might increment the counter in line 108, but not meet the condition in 116.

[mpdude]

Not sure, see above

[greg0ire]

I wish there was a way to express the fact that the keys don't matter other than mixed[], because then it seems that we neglected to specify the type of they keys, when what we really mean is: "the keys don't matter, don't rely on them".

Also, see how the returned property is currently annotated (although yes, it might very well be a lie depending on what you mean by list):

orm/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php

Lines 34 to 35 in 82533af

	/** @var list<string>|string */
	protected $sqlStatements;

[SenseException]

I interpreted list<> rather as "the keys have no meaning" than "doesn't matter" (or maybe you meant the same thing). Because a tool like PHPStan defines a list also as sequential integer keys I'd usually see gaps in a sequence as array<int, string>.

Even if the keys don't matter, they can be (re-)used by a dev. I wonder if list<string> could break someone's PHPStan workflow of custom code build on ORM.

Does array_is_list() return true when used on $this->sqlStatements in some cases? I'd expect that there can happen some false cases.

[mpdude]

To my understanding, the difference between array<T> and list<T> is that a list has keys that are the sequential integers 0, 1, 2, ...?

[greg0ire]

"the keys have no meaning" is indeed what I meant, I think. In this case, it's possible there are gaps, but we should not be careful to preserve them, I don't think they have a meaning.

To my understanding, the difference between array and list is that a list has keys that are the sequential integers 0, 1, 2, ...?

Yes, technically, but if we were using Python, the data structure we would be using here would be list. We wouldn't be using a dictionary, right? If it's not a list, that's an unwanted albeit unimportant side effect.

Anyway, I've amended my commit to make sure list is technically true, thus removing even more baseline blocks.