ReflectionParameter: try to get the correct default value for global constants

[voku]

For example, this function function foo(int $case = \CASE_LOWER);, ReflectionParameter->allowsNull() for $casewill currently return true.

---

This change is 

[Ocramius]

This is not really acceptable, since constant() leads to autoloading here:

https://3v4l.org/BeYeT

[voku]

@Ocramius Node\Expr\ConstFetch is only for global constants not for class constants, instead of Node\Expr\ClassConstFetch, or? If this is correct then there is no autoloading here.

[Ocramius]

Yeah, but then it will crash in case of an undefined constant: https://3v4l.org/ZZnmJ

Looking at our code, we do indeed use constant() elsewhere (in Roave\BetterReflection\NodeCompiler\CompileNodeToValue)

I'd say that this patch is good if following is true:

1. tests are added
2. CompileNodeToValue is used in here, instead of constant()

Having a constant() lookup there is already a bug: let's keep it isolated for now.

[voku]

@Ocramius constant() is replaced and a simple test case is added

But now I see Roave\BetterReflection\NodeCompiler\Exception\UnableToCompileNode : Could not locate constant "SOME_DEFINED_VALUE" ... in a different test.

[Ocramius]

The test that fails is this one:

    public function testIsDefaultValueConstantAndGetDefaultValueConstantName() : void
    {
        $classInfo = $this->reflector->reflect(Methods::class);
        $method    = $classInfo->getMethod('methodWithUpperCasedDefaults');

        $boolUpper = $method->getParameter('boolUpper');
        self::assertFalse($boolUpper->isDefaultValueConstant());

        $boolLower = $method->getParameter('boolLower');
        self::assertFalse($boolLower->isDefaultValueConstant());

        $nullUpper = $method->getParameter('nullUpper');
        self::assertFalse($nullUpper->isDefaultValueConstant());

        $method       = $classInfo->getMethod('methodWithConstAsDefault');
        $constDefault = $method->getParameter('constDefault');
        self::assertTrue($constDefault->isDefaultValueConstant());
        self::assertSame(Methods::class . '::SOME_CONST', $constDefault->getDefaultValueConstantName());

        $definedDefault = $method->getParameter('definedDefault');
        self::assertTrue($definedDefault->isDefaultValueConstant());
        self::assertSame('SOME_DEFINED_VALUE', $definedDefault->getDefaultValueConstantName());

        $intDefault = $method->getParameter('intDefault');
        self::assertFalse($intDefault->isDefaultValueConstant());

        $this->expectException(LogicException::class);
        $this->expectExceptionMessage('This parameter is not a constant default value, so cannot have a constant name');
        $intDefault->getDefaultValueConstantName();
    }

The source is Methods.php:

<?php

namespace Roave\BetterReflectionTest\Fixture;

\define('SOME_DEFINED_VALUE', 1);

abstract class Methods
{
    const SOME_CONST = 1;

    public function __construct()
    {
    }

    public function publicMethod()
    {
    }

    private function privateMethod()
    {
    }

    protected function protectedMethod()
    {
    }

    final public function finalPublicMethod()
    {
    }

    abstract public function abstractPublicMethod();

    public static function staticPublicMethod()
    {
    }

    function noVisibility()
    {
    }

    public function __destruct()
    {
    }

    /**
     * @param string $parameter1
     * @param int|float $parameter2
     */
    public function methodWithParameters($parameter1, $parameter2)
    {
    }

    public function methodWithOptionalParameters($parameter, $optionalParameter = null)
    {
    }

    public function methodWithExplicitTypedParameters(
        \stdClass $stdClassParameter,
        ClassForHinting $namespaceClassParameter,
        \Roave\BetterReflectionTest\Fixture\ClassForHinting $fullyQualifiedClassParameter,
        array $arrayParameter,
        callable $callableParameter
    ) {
    }

    public function methodWithVariadic($nonVariadicParameter, ...$variadicParameter)
    {
    }

    public function methodWithReference($nonRefParameter, &$refParameter)
    {
    }

    public function methodWithNonOptionalDefaultValue($firstParameter = 'someValue', $secondParameter)
    {
    }

    public function methodToCheckAllowsNull($allowsNull, \stdClass $hintDisallowNull, \stdClass $hintAllowNull = null)
    {
    }

    public function methodWithConstAsDefault($intDefault = 1, $constDefault = self::SOME_CONST, $definedDefault = SOME_DEFINED_VALUE)
    {
    }

    public function methodWithUpperCasedDefaults($boolUpper = TRUE, $boolLower = false, $nullUpper = NULL)
    {
    }
}

I wonder if we can make the default value "lazy", so that we only trip the exception if a read on the constant is happening. The test above is indeed correct: the Methods.php file should never be included, and therefore SOME_DEFINED_VALUE cannot be found by constant().

[Ocramius]

Are these AST caches necessary? I would suggest avoiding them, if they don't bring a massive advantage (the added complexity is noticeable)

[voku]

I remvoed the AST cache.

[Ocramius]

I think CS is needed (run vendor/bin/phpcbf), and maybe we should remove some conditionals that aren't strictly necessary for the patch.

[Ocramius]

LGTM! Some wonkiness in the tests, but I'm OK with ignoring CS for those couple rows :-)

Good job!