Read parameter default values from php8-stubs

[staabm]

Added support for reading parameter default values from JetBrains/phpstorm-stubs files as requested in https://github.com/phpstan/phpstan-src/pull/750/files#r745440887

Just expanded existing singnature tests with expected-default-param-values and also added a few new cases which the existing data did not cover yet.

[staabm]

I added support for a lot of cases and also test coverage for them.

I am at a point where default values can contain class-constants or bitwise-or of class-constants... adding support for these cases I feel I am re-building the NodeScoperResolver now for the default-values.

any hint on which direction to take is welcome.

[staabm]

this thing is ready. the only open problem (which renders the last test-case failling) is reported as phpstan/phpstan#5985

[staabm]

just updated the phpstorm stubs which fixed phpstan/phpstan#5985

the remaining errors also fail on the master branch.. therefore I think this one is ready for feedback now.

[ondrejmirtes]

Hi, the title of this PR says that you read default values from JetBrains/phpstorm-stubs, but the implementation looks like you're reading only php8-stubs.

Also, I'd like to see more integration-like test that for example obtains FunctionReflection from ReflectionProvider and reads the default value on that. Because that test would be executed in more PHP version and environments, we'd be able to see if it also works on PHP 7.x (which assumes that JetBrains/phpstorm-stubs are really used here).

[ondrejmirtes]

Because there's a lot of ShouldNotHappenException, we need a test that iterates over all symbols and makes sure that none of them throw.

[staabm]

added a separte unit test

[staabm]

Hi, the title of this PR says that you read default values from JetBrains/phpstorm-stubs, but the implementation looks like you're reading only php8-stubs.

actuall I was not sure either whether the default are now coming from JetBrains/phpstorm-stubs or php8-stubs. I got the impression that atm they are read from JetBrains/phpstorm-stubs because the fix from JetBrains/phpstorm-stubs#1273 made the PR pass, in cases it previously failed.

Also, I'd like to see more integration-like test that for example obtains FunctionReflection from ReflectionProvider and reads the default value on that. Because that test would be executed in more PHP version and environments, we'd be able to see if it also works on PHP 7.x (which assumes that JetBrains/phpstorm-stubs are really used here).

I need to figure out now, how todo that. before we dive deeper into this PR I would suggest to finalize #756 first

[ondrejmirtes]

I'm gonna need to solve similar problems as you're trying to do here in resolveParameterDefaultType, so I'm gonna base this PR on my work after it's finished :)

[staabm]

I think this should be good as is. in case there are wholes in the type-mapping, we will see those while finishing the named parameter PR - for which this one is a prerequisite.

the test-suite shows using ParserNodeTypeToPHPStanTypeTest::testAllDefinedParameterDefaultsAreParseble at least that all default parameter types contained in the php-src runtime are resolvable without exceptions.

[staabm]

the cause of the remaining error is that phpstan cannot resolve the type of these 2 global constants:

• STREAM_SERVER_BIND
• STREAM_SERVER_LISTEN

these constants are defined in the phpstorm stubs
https://github.com/JetBrains/phpstorm-stubs/blob/ec04c00bcdf45a38f67ea6d9d12f22f9575f917d/standard/standard_defines.php#L577
https://github.com/JetBrains/phpstorm-stubs/blob/ec04c00bcdf45a38f67ea6d9d12f22f9575f917d/standard/standard_defines.php#L587

is there anything else required to make sure phpstan can resolve their types? atm these are resolved to MixedType

[ondrejmirtes]

Hi, I've identified I need a similar thing solved not just for parameter default values, but for other issues as well.

So I need ParserNodeTypeToPHPStanType::resolveParameterDefaultType implemented properly and use it in many places. The root issue I'm trying to solve here is the support for "new in initializers" through static reflection, to solve issues like: https://phpstan.org/r/63bfdfb7-fc6c-4cea-9d27-28f1b086d5c1

Right now the problem is that BetterReflection's CompileNodeToValue is essentially "Expr -> runtime value" which isn't great for New_ because we don't to instantiate objects: https://github.com/ondrejmirtes/BetterReflection/blob/5.0.x/src/NodeCompiler/CompileNodeToValue.php

We want "Expr -> Type" which is what ParserNodeTypeToPHPStanType::resolveParameterDefaultType tries to do, and use it in as many places as possible.

I think the following steps can be taken when implementing this:

1. Check out phpstan-src, composer install, and then cripple BetterReflection by making CompileNodeToValue always throw an exception.
2. Run make tests-statlc-reflection and fix all the tests. The failed tests are trying to read the runtime value from CompileNodeToValue but that path should not be taken at all. Instead we should read the AST and use our own "Expr -> Type" logic.
3. Part of MutatingScope::getType() should be delegated to the new "Expr -> Type" logic.
4. Once this is done, we can use the new "Expr -> Type" logic to read default parameter values.

The way this should be implemented is to replicate PhpParser\ConstExprEvaluator and PHPStan\BetterReflection\NodeCompiler\CompileNodeToValue to do "Expr -> Type" and not "Expr -> runtime value", and then use it in all the places instead of currently reading the runtime value...

[ondrejmirtes]

Superseded by #1300