Incorrect type simplification when combining mixed[] and iterators

[cs278]

Bug report

Type declaration \Iterator<array-key,mixed>|mixed[] is incorrectly simplified to iterable&Iterator<(int|string), mixed> instead of iterable<(int|string), mixed>. You can see in the below example that iterable<array-key,mixed> instead of mixed[] works around the problem.

https://phpstan.org/r/ff2f9b51-429c-46de-8bea-fe9728bca69b

Code snippet that reproduces the problem

class HelloWorld
{
	/**
	 * @param \Iterator<array-key,mixed>|array<array-key,mixed> $params1
	 * @param \Iterator<array-key,mixed>|mixed[] $params2
	 */
	public function query($params1, $params2): bool
	{
		return true;
	}
}

$p = [
	'foo' => 123,
	'bar' => 'yes',
];
$o = new HelloWorld();
$o->query($p, $p);

Parameter #2 $params2 of method HelloWorld::query() expects iterable&Iterator<(int|string), mixed>, array<string, int|string> given.

https://phpstan.org/r/f682ea7b-5cb7-4e91-a1c9-33e9daa0ca50

Expected output

Expected no error.

[ondrejmirtes]

Hi, this is a complicated heuristic mainly for legacy code that uses Iterator|Foo[] to express Iterator<Foo>. When you're writing your own code, simply use @param Iterator<array-key,mixed>. If this is third-party code we're talking about, use stub files to fix wrong PHPDocs.

[cs278]

Thanks for providing context, however could you consider reopening this issue?

This heuristic results in the following unpredictable behaviour:

• (\Iterator)|(string[]) is parsed as iterable<string>&Iterator which is clearly incorrect.
• \Iterator<array-key,string>|string[] and \Iterator<array_key,string>|bool[] result in very different types.

The reason I raised this in the first place is because of a Doctrine changing a bunch of array annotations to mixed[] causing PHPStan to fail on our code base: https://github.com/doctrine/orm/blame/ebae57eb9637acd8252b398df3121b120688ed5c/lib/Doctrine/ORM/AbstractQuery.php#L1033 Ironic given Doctrine also uses PHPStan to examine their code.

IMO this behaviour should be removed, even if users can opt-out of it right now that would be an improvement. As you say in your reply people can fix incorrect vendor code using stubs, but what you're telling me to do is fix non-broken code.

FWIW Psalm appears to have an option for this: https://psalm.dev/docs/running_psalm/configuration/#allowphpstormgenerics

Right now it seems with PHPStan using the type[] syntax should be avoided.

[ondrejmirtes]

Can you show the specific error you're getting when working with Doctrine reproduced on phpstan.org? ArrayCollection|mixed[]|null is a bit simpler than what you're showing above.

[cs278]

This seems to be as simple as I can make it without copying loads of code from Doctrine: https://phpstan.org/r/dcfed621-3978-4d3f-a4b9-73647ecf7c18

[ondrejmirtes]

Doctrine's PHPDoc is simply wrong. This is sufficient to make it right: https://phpstan.org/r/e85c863a-2c4f-4e55-814c-d3eb1be52bed

Please send Doctrine a PR, or override it in your codebase with a stub file.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.