Improve intersection of ConstantArray and AccessoryIsList

[VincentLanglet]

Proof of the gain in the next PR #5069

[VincentLanglet]

The main idea of this PR.

Before, we had sometime a ConstantArray(isList = true) and sometimes ConstantArray(isList=maybe)&AccessoryArrayListType ; now I merged them to always have ConstantArray(isList = true).

[staabm]

I wonder why these 2 types of "isList" exist in the first place. could this always be using AccessoryArrayListType?

[VincentLanglet]

ConstantArray need to implement isList, if you return Maybe for this method, you'll end up with too many test failure ; having to intersect with AccessoryArrayListType every time, even when doing a basic $a = [].

Also, there is some optimisation in the ConstantArray possible when we know it's a list. It would be harder to do such thing in the intersection type. And that's the whole purpose of this PR ; currently some optimisation are ignored because the array doesn't know it's a list. And some other optimisation are possible.

[VincentLanglet]

Since ConstantArray&AccessoryArrayIsList is simplified into ConstantArray, I always get a description like

array{...}

This is annoying when having too much optional keys.

I tried to find the formula to change as few description as possible:

• array like array{0: string, 1: string} without optional keys are untouched because they are always a list
• same for array like array{0: string, 1: string, 2?: string} where the optional keys is the last one
• list{0: string, 1?: string, 2: string} should stay a list (in the next PR I'll make the offset 1 required)
• list{0: string, 1?: string, 2?: string} with 2+ optional keys should stay a list.

So we cannot only rely on IntersectionType to add the list keyWord.

[VincentLanglet]

Now constant array is not always a array{...} and strlen of list and array are different so I need to check it.

[VincentLanglet]

This is technically not a valid intersection anymore as it will be simplified into a constant array with isList = Yes by TypeCombinator::intersect.

So the test was failing because

• this type was transformed into list{0: string, 1: string, 2?: string, 3?: string}.
• this string was parsed into a ConstantArray (isList = Yes) without the need of an intersectionType.

I updated the test to reproduce the "new real case".

[VincentLanglet]

Kinda related to #5029 ; this is an idea I already had in mind. Without this fix tests are failing because we don't have the AccessoryIsList anymore (already merged into the constant array).

This method is used in two different context:

• when unsetting the value
• during something like

if (array_key_exist(...)) {

} else {
     // Here we're trying to remove the array key, and calling unsetOffset
}

For the second example, even if the key is "unset", shouldn't touch the list certainty since we didn't really modified the array. That's why tryRemove will call with 'true'.

if ($typeToRemove instanceof HasOffsetType) {
			return $this->unsetOffset($typeToRemove->getOffsetType(), true);
		}

[staabm]

I think this needs a phpdoc for this parameter, because intuitively unsetting on a list will always destroy the list

[VincentLanglet]

I added

/**
	 * Removes or marks as optional the key(s) matching the given offset type from this constant array.
	 *
	 * By default, the method assumes an actual `unset()` call was made, which actively modifies the
	 * array and weakens its list certainty to "maybe". However, in some contexts, such as the else
	 * branch of an array_key_exists() check, the key is statically known to be absent without any
	 * modification, so list certainty should be preserved as-is.
	 */

[phpstan-bot]

This pull request has been marked as ready for review.

[VincentLanglet]

This solve phpstan/phpstan#11602 (comment) but not the whole issue.

[VincentLanglet]

This is some bonus of this PR.

Before a ConstantArray(isList = true) was described as array in every occasion.

Now I describe it as list in some conditions, which gives a meaningful type.

As an example here array_is_list would have a return true for array{0: 0, 1?: 1|2, 2?: 2} but this would not have been clear for the user why.
See https://phpstan.org/r/a82c93cb-e834-4fd9-8c01-6bd06446ec04

[staabm]

I think this makes sense. Ondrej should have another look though.