Getting parameter from method with new initializer throws
DanielBadura opened this issue · 11 comments
Again, coming originally from BackwardsCompatibilityCheck. Running it results right now to some Skipped checks due to an error in BetterReflection. Here the output from BC-Check:
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\Projectionist::boot() in file /src/Projection/Projectionist.php (line 11)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\Projectionist::run() in file /src/Projection/Projectionist.php (line 16)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\Projectionist::teardown() in file /src/Projection/Projectionist.php (line 18)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\Projectionist::remove() in file /src/Projection/Projectionist.php (line 20)
Error: Could not locate constant Patchlevel\EventSourcing\Projection\ProjectorStatus::New while trying to evaluate constant expression in method Patchlevel\EventSourcing\Projection\ProjectorStore\ProjectorState::__construct() in file /src/Projection/ProjectorStore/ProjectorState.php (line 14)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\MetadataProjectorResolver::__construct() in file /src/Projection/MetadataProjectorResolver.php (line 17)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\DefaultProjectionist::__construct() in file /src/Projection/DefaultProjectionist.php (line 23)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\DefaultProjectionist::boot() in file /src/Projection/DefaultProjectionist.php (line 28)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\DefaultProjectionist::run() in file /src/Projection/DefaultProjectionist.php (line 75)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\DefaultProjectionist::teardown() in file /src/Projection/DefaultProjectionist.php (line 117)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\DefaultProjectionist::remove() in file /src/Projection/DefaultProjectionist.php (line 148)
Error: Unable to compile initializer in method Patchlevel\EventSourcing\Projection\SyncProjectorListener::__construct() in file /src/Projection/SyncProjectorListener.php (line 14)
So I tried to get an reproducer for this and came up with this TestCase:
public function testClassWithNewInitializer(): void
{
$php = <<<'PHP'
<?php
class Foo {}
class Bar {
public function __construct(Foo $foo = new Foo()) {}
}
PHP;
$reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
$classReflection = $reflector->reflectClass('Bar');
foreach ($classReflection->getMethod('__construct')->getParameters() as $parameter) {
$parameter->getDefaultValue();
}
}Which leads to this error:
vendor/bin/phpunit --filter testClassWithNewInitializer
PHPUnit 9.5.25 #StandWithUkraine
Random Seed: 1669196177
E 1 / 1 (100%)
Time: 00:00.190, Memory: 56.00 MB
There was 1 error:
1) Roave\BetterReflectionTest\Reflection\ReflectionClassTest::testClassWithNewInitializer
Roave\BetterReflection\NodeCompiler\Exception\UnableToCompileNode: Unable to compile initializer in method Bar::__construct() in file "" (line 4)
/app/src/NodeCompiler/Exception/UnableToCompileNode.php:107
/app/src/NodeCompiler/CompileNodeToValue.php:62
/app/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php:146
/app/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php:101
/app/src/NodeCompiler/CompileNodeToValue.php:136
/app/src/Reflection/ReflectionParameter.php:282
/app/src/Reflection/ReflectionParameter.php:376
/app/test/unit/Reflection/ReflectionClassTest.php:2830
ERRORS!
Tests: 1, Assertions: 0, Errors: 1.
This is due to https://github.com/Roave/BetterReflection/blob/6.5.x/src/NodeCompiler/CompileNodeToValue.php#L61-L63.
I will try to provide a patch for it.
This is valid and expected in BetterReflection.
Yup, we don't allow getting values for initializer expressions, since they can trigger autoloading, and avoiding autoloading is the primary purpose of this library.
Should I open another issue then for BC-Check?
Also the one error in between the new initialize errors is about a default enum value. This should be possible to solve, right?
Yeah, I think the @throws in roave/backward-compatibility-check is not handled.
I suggest raising a PR directly, perhaps.
Also the one error in between the new initialize errors is about a default enum value. This should be possible to solve, right?
Fetching an enum still causes it to autoload, heh
Yeah, I think the
@throwsinroave/backward-compatibility-checkis not handled.I suggest raising a PR directly, perhaps.
If i would know exactly what to do, sure. Already looking into it..
Fetching an enum still causes it to autoload, heh
True...
Fetching an enum still causes it to autoload, heh
No, I think this should be resolved without autoload.
@DanielBadura Can you try different enum case? There may be problem with the name New.
Tried it already here patchlevel/event-sourcing@8384f5b
But there the "base" was still having the enum with New so could also possibly that why it was still failing.
My bad, @Ocramius is right. Enum::SOME_CASE returns instance, so it cannot be resolved. It looks like a constant access, but it's not.
Yeah, getting the string value might be possible but this would not be the real value passed to the method. So i guess this is not possible without autoloading and could also be handled somehow in roave/backward-compatibility-check so that it will not produce errors there.