Fix "offset might not exist" false-positives when offset is a expression

[staabm]

closes phpstan/phpstan#9494
closes phpstan/phpstan#12115
closes phpstan/phpstan#12805
closes phpstan/phpstan#12931
closes phpstan/phpstan#13214
closes phpstan/phpstan#13039
closes phpstan/phpstan#13538

requires

• Simplify ClassReflectionExtensions #4374
• Simplify *StatementWithoutImpurePointsRule #4375
• Improve ConstantArrayType inference #4376
• Cleanup instanceof Type checks #4380

[staabm]

reduced reproducer of the FileTypeMapper false-positive error we get right now:

<?php declare(strict_types = 1);

namespace PHPStan\Type;

use function is_callable;

final class NameScope {}

final class FileTypeMapper
{

	/** @var (true|callable(): NameScope|NameScope)[][] */
	private array $inProcess = [];

	public function getNameScope(
		string $fileName,
		?string $className,
		?string $traitName,
		?string $functionName,
	): NameScope
	{
		$nameScopeKey = $this->getNameScopeKey($fileName, $className, $traitName, $functionName);
		if (!isset($this->inProcess[$fileName][$nameScopeKey])) { // wrong $fileName due to traits
			throw new \RuntimeException();
		}

		if ($this->inProcess[$fileName][$nameScopeKey] === true) { // PHPDoc has cyclic dependency
			throw new \RuntimeException();
		}

		if (is_callable($this->inProcess[$fileName][$nameScopeKey])) {
			$resolveCallback = $this->inProcess[$fileName][$nameScopeKey];
			$this->inProcess[$fileName][$nameScopeKey] = true;
			$this->inProcess[$fileName][$nameScopeKey] = $resolveCallback();
		}

		return $this->inProcess[$fileName][$nameScopeKey];
	}

	private function getNameScopeKey(
		?string $file,
		?string $class,
		?string $trait,
		?string $function,
	): string
	{
		return '';
	}

}

leads to

------ --------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   test7.php                                                                                                                                    
 ------ --------------------------------------------------------------------------------------------------------------------------------------------- 
  37     Method PHPStan\Type\FileTypeMapper::getNameScope() should return PHPStan\Type\NameScope but returns (callable)|PHPStan\Type\NameScope|true.  
         🪪  return.type                                                                                                                              
         at test7.php:37                                                                                                                              
 ------ --------------------------------------------------------------------------------------------------------------------------------------------- 

needs investigation fixed in 72177cf

[staabm]

required change to not regress existing tests like

<?php

$anotherIndex = 0;
$postIncArray = [];
$postIncArray[$anotherIndex++] = $anotherIndex++;
\PHPStan\Testing\assertType('array{1}', $postIncArray);

[staabm]

required to prevent regression in

@ -0,0 +1,22 @@
<?php

namespace AssignNestedArrays;

use function PHPStan\debugScope;
use function PHPStan\Testing\assertType;

class Foo
{
	public function doBar(int $i, int $j)
	{
		$array = [];

		$array[$i][$j]['bar'] = 1;
		debugScope();
		$array[$i][$j]['baz'] = 2;
		debugScope();

		assertType('non-empty-array<int, non-empty-array<int, array{bar: 1, baz: 2}>>', $array);
	}

}

[staabm]

ok - I think after all required PRs (see description) have been merged, this one will be ready to land

[phpstan-bot]

This pull request has been marked as ready for review.

[staabm]

@ondrejmirtes why is the issue related to Plus? there is no + contained in the related code example?

[staabm]

ohh.. the Plus is related to the list-inference logic below.

so it was a bug lingering for longer already :)

[staabm]

looking at issue bot, the last fix also fixes more stuff.. adding regression tests now..

[ondrejmirtes]

I'm not a fan of by-ref parameters. I'd rather if the method returned array{Type, list<array{Expr, Type}>} 😊

[staabm]

done

[ondrejmirtes]

Awesome, thank you!

[ondrejmirtes]

This is really nice PR and very solid work, but I found a regression: https://phpstan.org/r/16197474-84cb-43ad-b983-ad78371fba02

My guess is that the bug will be somewhere in MutatingScope::generalizeWith().

[ondrejmirtes]

And another one: https://phpstan.org/r/80fa95b4-c7a0-4591-937d-efbf39561d95 (the nullCoalesce.offset ones)

I guess it's not really easy to reason about that. After the for loop iteration, we can be sure that $stats[$author] contains all three offsets commits/files/insertions, but when evaluating:

$stats[$author]['files'] = ($stats[$author]['files'] ?? 0) + $files;

We don't know whether $stats[$author] already has 'files' offset from the previous iteration, or if it's a new author that only just has 'commits' offset.

[staabm]

My guess is that the bug will be somewhere in MutatingScope::generalizeWith().

my gut feeling is we miss to generalize key/value types when generalizeType(ConstArray, GeneralArray) is compared
(so we miss to recurse into key/value types in this case)

[ondrejmirtes]

I have a different theory now and possible solution - it's a problem when merging scopes.

The $array variable type will be merged because it's in both scopes but then there's also $array[$k] which will only be in the inner scope but not the outer scope so it will remain with the same type in the final scope after merging and certainty Maybe.

So I think when we're merging scopes and the type of $array changes, we need to throw away all other expressions that contain $array and not merge them.

I think somewhere in MutatingScope we do or used to do in the past was to sort expressions by length so that the shorter ones are evaluated first. It might be useful in merging too.