Roave / BetterReflection

:crystal_ball: Better Reflection is a reflection API that aims to improve and provide more features than PHP's built-in reflection API.
MIT License
1.19k stars 131 forks source link

Default enum value for a parameter throw an exception #1376

Closed forrest79 closed 11 months ago

forrest79 commented 12 months ago

Hi, I figured out, that reflecting a class like this will fail on getting the default value for the $x parameter:

enum TestEnum: int {
    case X = 1;
}

class TestClass {
    public function text(TestEnum $x = TestEnum::X) {
    }
}

I have got Uncaught Roave\BetterReflection\NodeCompiler\Exception\UnableToCompileNode: Could not locate constant TestEnum::X while trying to evaluate constant expression in method TestClass::test() in file ....

As a quick workaround, I added something like this:

if ($classReflection instanceof ReflectionEnum) {
    $case = $classReflection->getCase($constantName);

    if ($case === null) {
        throw Exception\UnableToCompileNode::becauseOfNotFoundClassConstantReference($context, $classReflection, $node);
    }

    return $case->getValue();
}

Into src/NodeCompiler/CompileNodeToValue.php method getClassConstantValue(). I wanted to return the correct enum instance, but I got another error, the enum is not supported as return value.

Ocramius commented 12 months ago

This is correct, because BetterReflection would have to instantiate the object, therefore loading it.

Ocramius commented 12 months ago

BTW, the exception message is obviously confusing:

Could not locate constant TestEnum::X while trying to evaluate constant expression in method TestClass::test() in file

We could improve that :thinking:

forrest79 commented 11 months ago

Thanks for a rapid response. I solved this using getDefaultValueExpression (my bad :-()

Ocramius commented 11 months ago

@forrest79 are we OK with just raising a better exception message here?

forrest79 commented 11 months ago

Absolutely :-) It was my fault, I was expecting, that Enum could be simply returned as a returned value.

Ocramius commented 11 months ago

@forrest79 are we OK with just raising a better exception message here? Would you be able to help with that, perhaps?

forrest79 commented 11 months ago

Yes, I will be glad to help. My idea is just to detect, if the class is an Enum and throw a different exception?

Ocramius commented 11 months ago

I'd say so, yep 👍

forrest79 commented 11 months ago

PR #1380

Ocramius commented 11 months ago

Handled in #1380