Raise RedundantCast when using array_values on a list (fixes #6988)

[danog]

Fixes #6988

[orklah]

Interesting, it found a few cases in psalm itself. Can you remove them and see if anything breaks?

[danog]

src/Psalm/Type/Atomic/GenericTrait.php:121:28 is really strange, @psalm-trace reports a type of array{0: Union} exactly after unsetting $type_params[0], which leads to this false positive:

            //we remove the key for display
            unset($type_params[0]);
            /** @psalm-trace $type_params */;
            $type_params = array_values($type_params);

Psalm output:

ERROR: Trace - src/Psalm/Type/Atomic/GenericTrait.php:120:45 - $type_params: array{Psalm\Type\Union} (see https://psalm.dev/224)
            /** @psalm-trace $type_params */;


ERROR: RedundantCastGivenDocblockType - src/Psalm/Type/Atomic/GenericTrait.php:121:28 - The call to array_values is unnecessary given the list docblock type array{Psalm\Type\Union} (see https://psalm.dev/263)
            $type_params = array_values($type_params);

[orklah]

This may be a bug in Psalm, I'll look at that this evening

[orklah]

Ok, the issue is in Psalm.

we have a array{Psalm\Type\Union, Psalm\Type\Union} that becomes an array{Psalm\Type\Union} after a unset($type_params[0]). This is wrong, it should become a array{1: Psalm\Type\Union} or something like that.

I'll see how I can fix that

[orklah]

Can you rebase? It should be better

[orklah]

test with real project fails with strange error

Uncaught ParseError: syntax error, unexpected fully qualified name "\class_alias" in phar:///home/docker/project/build/psalm.phar/vendor/symfony/polyfill-php81/Resources/stubs/ReturnTypeWillChange.php:14
Stack trace:
#0 phar:///home/docker/project/build/psalm.phar/.box/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('phar:///home/do...')
#1 [internal function]: Composer\Autoload\ClassLoader->loadClass('_HumbugBoxe8d53...')
#2 phar:///home/docker/project/build/psalm.phar/vendor/autoload.php(32): spl_autoload_call('_HumbugBoxe8d53...')
#3 phar:///home/docker/project/build/psalm.phar/src/Psalm/Internal/CliUtils.php(50): require_once('phar:///home/do...')
#4 phar:///home/docker/project/build/psalm.phar/src/Psalm/Internal/Cli/Psalm.php(173): Psalm\Internal\CliUtils::requireAutoloaders('/tmp/testing-wi...', false, 'vendor')
#5 phar:///home/docker/project/build/psalm.phar/src/Psalm/Internal/IncludeCollector.php(31): Psalm\Internal\Cli\Psalm::Psalm\Internal\Cli\{closure}()
#6 phar:///home/docker/project/build/psalm.phar/src/Psalm/Internal/Cli/Psalm.php(174): Psalm\Internal\IncludeCollector->runAndCollect(Object(Closure))
#7 phar:///home/docker/project/build/psalm.phar/psalm(6): Psalm\Internal\Cli\Psalm::run(Array)
#8 /home/docker/project/build/psalm.phar(14): require('phar:///home/do...')
#9 {main}
(Psalm (unknown version) crashed due to an uncaught Throwable)

I'm also unsure what to think about adding polyfills into Psalm. We have to keep in mind that Psalm make use of global namespace for a lot of things and things like polyfill could introduce new weird bugs

[weirdan]

Interesting. Locally, the crash happens with PHP 8.0 only (neither 7.4 nor 8.1 are affected).

[weirdan]

I believe this is caused by humbug/php-scoper adding class attributes to class_alias() call it adds. It's syntactically invalid in PHP 8+, but the polyfill for ReturnTypeWillChange is not loaded in 8.1+. That's why it happens on PHP 8 only.

Rather than investigating and fixing it upstream (we can't use current php-scoper version on PHP 7.1 anyway) it's probably easier to snatch the function in its entirety and put it somewhere under Psalm\Internal namespace as a static method.

It'll also make sure we don't load global symbols (functions and classes) into Psalm runtime - that could cause false negatives analyzing the code on PHP 8 due to reflection fallback we have.

[danog]

This should work nicely!

[orklah]

@weirdan I think we can merge this, but we'll need to update baseline on phpunit

[weirdan]

Thanks!

[weirdan]

we'll need to update baseline on phpunit

Done.