themehybrid / hybrid-core

Official repository for the Hybrid Core WordPress development framework.
GNU General Public License v2.0
689 stars 144 forks source link

PHP 8.0 compatibility #180

Closed brettsmason closed 3 years ago

brettsmason commented 3 years ago

Testing the 6.0 branch on my PHP 8 setup and am getting the following error:

 Deprecated: Method ReflectionParameter::getClass() is deprecated in /home/brettsmason/Sites/themedev/web/app/themes/luxe/vendor/justintadlock/hybrid-core/src/Container/Container.php on line 395

From reading this it looks like this should be updated to ReflectionParameter::getType().

justintadlock commented 3 years ago

getType() will require a min. PHP version bump to 7. Currently, we support down to 5.6, but I'm good with bumping it.

justintadlock commented 3 years ago

This might work for line 395. It needs testing on PHP 7 and 8:

// Replace this line:
} elseif ( ! is_null( $dependency->getClass() ) ) {

// With this:
} elseif ( ! is_null( $dependency->getType() ) ) {

// Alternate idea:
} elseif ( ! is_null( $dependency->getType() ) && class_exists( $dependency->getType()->getName() ) ) {
justintadlock commented 3 years ago

And, I forgot line 397:

// Replace this line:
$args[] = $this->resolve( $dependency->getClass()->getName() );

// With this:
$args[] = $this->resolve( $dependency->getType()->getName() );
brettsmason commented 3 years ago

Can confirm that removes the deprecated notice and continues to work fine.

justintadlock commented 3 years ago

It seems to be working for me. getType()->getName() is undocumented in PHP, so I'd like to at least get one more person to verify this is working.

Can anyone test down to 7.0? I'd like to make sure we have a minimum version.

saas786 commented 3 years ago

For v6, I am ok with PHP v5.6 compatibility. Can bump it to at least PHP 7.3 in v6.1.

Will test it out on PHP v5.6, v7.3, v7.4 & v8.

and update the code as well if things are working fine.