Allow 8.5 as PHP version

[kubawerlos]

Since the update of nikic/PHP-Parser, PHPStan recognises PHP 8.5 constants (such as T_PIPE or T_VOID_CAST), so let's allow setting up 8.5 as the PHP version.

[ondrejmirtes]

Thank you. You also need to update the workflows to include 8.5 (everywhere the matrix includes 7.4...8.4).

[ondrejmirtes]

No you need to figure out why the build is failing 😊

[kubawerlos]

No you need to figure out why the build is failing 😊

I assume the typo in the first word 😊.

I have found out that Attribute::IS_REPEATABLE changed the value in PHP 8.5, and PHPStan\Reflection\ClassReflection takes the value from vendor/symfony/polyfill-php80/Resources/stubs/Attribute.php which may be a problem as of symfony/polyfill#538

[ondrejmirtes]

and PHPStan\Reflection\ClassReflection takes the value from vendor/symfony/polyfill-php80/Resources/stubs/Attribute.php

this reasoning is a bit suspicious to me. Why would code on PHP 8+ take values from a polyfill? Can you point to a specific line that causes this issue?

[kubawerlos]

this reasoning is a bit suspicious to me. Why would code on PHP 8+ take values from a polyfill? Can you point to a specific line that causes this issue?

I haven't found why (yet?). You can verify it by changing IS_REPEATABLE in vendor/symfony/polyfill-php80/Resources/stubs/Attribute.php to something else and running tests/PHPStan/Reflection/ClassReflectionTest.php - they will fail.

[ondrejmirtes]

I've thought about it more and the polyfill isn't the reason for this problem. We don't need the change in Symfony code.

The same problem would arise when analysing a project that supplies its own different polyfill with Composer.

The correct solution is to provide our own Attribute class stub and put it as own of the first locators in BetterReflectionSourceLocator factory.

Please know I debuggged this in my head while on vacation. I can verify these claims later.

[kubawerlos]

I've thought about it more and the polyfill isn't the reason for this problem. We don't need the change in Symfony code.

I agree we don't need it, but somehow removing it (see changes in composer.json fixed all the tests.

The correct solution is to provide our own Attribute class stub and put it as own of the first locators in BetterReflectionSourceLocator factory.

Wouldn't it be better if we ignore the polyfill class and rely on the built-in one?

[ondrejmirtes]

I said what needs to happen. The fact that PHPStan prioritizes Composer-provided vendor/ classes instead of PHP-built-in ones doesn't have an easy solution. Because sometimes, there's an obscure PHP extension represented in phpstorm-stubs and people have a class with the same name in their project which they actually use (like Event or HttpRequest).

That's why I'm proposing a one-off solution for Attribute.

I bet you can reproduce the same issue on PHP 8.4 if you create a project with its own Attribute class and different values for its constants.

[kubawerlos]

Can you give me a hint on how to do that? Was there any similar change made in the past?

I've tried to add a new stub in stubs directory and added it to conf/config.neon - no luck.

Also, I have found this: https://github.com/phpstan/phpstan-src/blob/2.1.21/stubs/runtime/Attribute.php, but adding/changing values there are ignored.

[ondrejmirtes]

Something like OptimizedSingleFileSourceLocator has to be put in the locators array somewhere early in BetterReflectionSourceLocatorFactory.

[ondrejmirtes]

I can't push to your fork so I'm trying my luck here #4192

[ondrejmirtes]

And we're green! Thank you.