fix makeOffsetRequired with UnionType offset

[rajyan]

fixes phpstan/phpstan#7000

$offsetType with UnionType should be handled separately like in hasOffsetValueType

phpstan-src/src/Type/Constant/ConstantArrayType.php

Lines 412 to 419 in 5066420

	if ($offsetType instanceof UnionType) {
	$results = [];
	foreach ($offsetType->getTypes() as $innerType) {
	$results[] = $this->hasOffsetValueType($innerType);
	}
	return TrinaryLogic::extremeIdentity(...$results);
	}

[ondrejmirtes]

Hi, this fix isn't right. When you have a union type, the value is one of the inner types. So checking isset of 'require'|'require-dev' offset doesn't mean that both offsets will surely exist after the check.

In fact this is really hard to fix in typesystem alone. The closest thing we can get after the isset is something like:

ConstantArrayType(both keys still optional)&NonEmptyArrayType

But that still doesn't help us to get rid of the error when asking about $array['require'|'require-dev'] because we don't know which one is set (both are optional).

What we can do to fix this is to let TypeSpecifier specify $composer[$linkType] (ArrayDimFetch) based on the isset so that Scope::isSpecified(ArrayDimFetch) returns true. (This might already be happening, I'm not sure.)

And the right rule needs to pick up this information and not report the error. You also need to write a regression test for the rule instead of NodeScopeResolverTest.

[rajyan]

Hi, this fix isn't right. When you have a union type, the value is one of the inner types.

Oh, you're right.

Thank you for the pointers! I'll continue looking into it.

[ondrejmirtes]

Feel free to finish the other PR related to Coalesce/Isset_ first, that might have a bigger impact 😊

[rajyan]

Feel free to finish the other PR related to Coalesce/Isset_ first

I’ll finish it today!

[rajyan]

I believe this is now an improvement for markOffsetRequired, but cannot fix the problems you mentioned in
#1187 (comment)
phpstan/phpstan#7000 (comment)
it is hard to solve them by the type system alone (as you mentioned).

The problem (I think) is that type in isset scope should be if ($linkType === 'require') array{require: array<string, string>, require-dev?: array<string, string>}; if ($linkType === 'require-dev') array{require?: array<string, string>, require-dev: array<string, string>}, which is hard to represent in types.
so the type specified here

phpstan-src/src/Analyser/TypeSpecifier.php

Lines 785 to 790 in 036056b

	$type = $this->create(
	$var->var,
	new HasOffsetType($scope->getType($var->dim)),
	$context,
	false,
	$scope,

should be an conditional type if $var->dim is a UnionType

[rajyan]

I'll read this #1187 (comment) again and look for a fix if possible

[rajyan]

improving type to
ConstantArrayType(both keys still optional)&NonEmptyArrayType
by b8902c3
does fix the phpstan/phpstan#7000
while providing an false negative

phpstan-src/tests/PHPStan/Rules/Arrays/data/bug-7000.php

Line 16 in 0df7ef1

foreach ($composer[$foo] as $x) {} // should report error. It can be $linkType = 'require', $foo = 'require-dev'

[rajyan]

I'd close this for now. I think it's not easy enough to solve cleanly, and there are other related issues to solve before this.

phpstan/phpstan#7143
phpstan/phpstan#7144