Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReflectionParameter::getClass is deprecated in PHP 8 #727

Closed
timfennis opened this issue Aug 5, 2020 · 9 comments · Fixed by #737
Closed

ReflectionParameter::getClass is deprecated in PHP 8 #727

timfennis opened this issue Aug 5, 2020 · 9 comments · Fixed by #737
Labels

Comments

@timfennis
Copy link

I'm testing out PHP-DI on PHP 8 and noticed that it's throwing some deprecation warnings. If this issue is not wanted feel free to delete it.

Deprecated: Method ReflectionParameter::getClass() is deprecated in /var/www/vendor/php-di/php-di/src/Definition/Source/ReflectionBasedAutowiring.php on line 65

and

Deprecated: Method ReflectionParameter::getClass() is deprecated in /var/www/vendor/php-di/php-di/src/Invoker/FactoryParameterResolver.php on line 45
@timfennis
Copy link
Author

In ReflectionBaseduaowiring.php chaning getparamtersDefinition to something like

    private function getParametersDefinition(\ReflectionFunctionAbstract $constructor) : array
    {
        $parameters = [];

        foreach ($constructor->getParameters() as $index => $parameter) {
            // Skip optional parameters
            if ($parameter->isOptional()) {
                continue;
            }

            $type = $parameter->getType();
            $class = null !== $type && !$type->isBuiltin() ? $type->getName() : null;

            if ($class) {
                $parameters[$index] = new Reference($class);
            }
        }

        return $parameters;
    }

seems to work. (Solution based on pull-requests seen elsewhere).

For FactoryParamterResolver.php the following solution appears to work:

    public function getParameters(
        ReflectionFunctionAbstract $reflection,
        array $providedParameters,
        array $resolvedParameters
    ) : array {
        $parameters = $reflection->getParameters();

        // Skip parameters already resolved
        if (! empty($resolvedParameters)) {
            $parameters = array_diff_key($parameters, $resolvedParameters);
        }

        foreach ($parameters as $index => $parameter) {
            $type = $parameter->getType();
            $class = null !== $type && !$type->isBuiltin() ? $type->getName() : null;

            if (!$class) {
                continue;
            }

            if ($class === 'Psr\Container\ContainerInterface') {
                $resolvedParameters[$index] = $this->container;
            } elseif ($class === 'DI\Factory\RequestedEntry') {
                // By convention the second parameter is the definition
                $resolvedParameters[$index] = $providedParameters[1];
            } elseif ($this->container->has($class)) {
                $resolvedParameters[$index] = $this->container->get($parameterClass->name);
            }
        }

        return $resolvedParameters;
    }

I'm not very familiar with php-di and I can't tell if these are actual solutions which is why I've neglected to make a pull-request.

@mnapoli mnapoli added the bug label Aug 5, 2020
@mnapoli
Copy link
Member

mnapoli commented Aug 5, 2020

In this pull request I fixed a few things for PHP 8 support: https://github.com/PHP-DI/PHP-DI/pull/726/commits

I think we'll have to support PHP 8 in PHP-DI 6 anyway, so it might be worth applying these "PHP 8" commits on master too.

@Findus23
Copy link

@mnapoli Just to be clear: Would it be possible to include the PHP 8 support (9218e54) also in PHP-DI 6 (which support PHP 7.2+)? That would be really great for Matomo.

@mnapoli
Copy link
Member

mnapoli commented Aug 14, 2020

@Findus23 yes, PHP-DI 6 should support PHP 8.

@Findus23
Copy link

Findus23 commented Sep 4, 2020

@mnapoli As the last PHP8 beta was released, it would be amazing if you could publish a new PHP-DI 6 version including the PHP 8 compatibility.

@mnapoli mnapoli mentioned this issue Sep 28, 2020
@mnapoli
Copy link
Member

mnapoli commented Sep 28, 2020

After spending a good amount of time into this, PHP 8 support for PHP-DI 6 is harder than I anticipated because of PHP-DI/PhpDocReader#19.

Since there is already more work on PHP-DI to do, my current plan is to support PHP 8 in PHP-DI 7 (which drops the need for PhpDocReader, i.e. it removes the problem).

If anyone wants PHP 8 support for PHP-DI 6, please work on PhpDocReader to support PHP 8. If you can't, you can also hire me to work on that (with enough time I know I can), but at the moment I cannot spend a whole day working on this on my own time.

@tsteur
Copy link
Contributor

tsteur commented Oct 5, 2020

Hi @mnapoli we at Matomo would sponsor it. I get Matt to reach out to you. We'd require PHP 7.2.5+ support though and not PHP 7.3+. Currently we're using PHP DI 6.2.1 in Matomo AFAIK https://github.com/matomo-org/matomo/blob/4.x-dev/composer.lock#L1219

@mnapoli
Copy link
Member

mnapoli commented Oct 7, 2020

@tsteur thanks, email received!

@mnapoli
Copy link
Member

mnapoli commented Oct 12, 2020

PHP-DI 6.3 is released with PHP 8 support 🎉 .

Thanks a lot to the InnoCraft team, the makers of Matomo, for sponsoring this release!

Let me know of any compatibility you may see, all dependencies have been upgraded and tested as well. For those curious, all tests are now running on GitHub Actions because Travis CI does not support PHP 8 yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants