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

PHP 8 Deprecation #192

Open
hallleron opened this issue Nov 27, 2020 · 11 comments
Open

PHP 8 Deprecation #192

hallleron opened this issue Nov 27, 2020 · 11 comments

Comments

@hallleron
Copy link

hallleron commented Nov 27, 2020

I checked an application that uses Dice with PHP 8 and immediately I get the error message "Method ReflectionParameter::getClass() is deprecated", which terminates the whole process. The error is thrown at the function call in line 216.

Will there be a PHP 8 compatible upgrade in the near future?

@StuTheBearded
Copy link

Here is my fix for that error message, replace line 216 with the below:

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

@cseufert
Copy link

cseufert commented Dec 6, 2020

Although this patch will work for single class parameters, any Union types will cause an error on this line.

@hallleron
Copy link
Author

Although this patch will work for single class parameters, any Union types will cause an error on this line.

Yes exactly, I haven't yet been able to figure out a workaround.

@jtojnar
Copy link

jtojnar commented Dec 14, 2020

It needs to be

if (PHP_VERSION_ID < 70100) {
    // getClass is deprecated in PHP 8.0 but getType() does not support getName() before 7.1.
    $class = $param->getClass() ? $param->getClass()->name : null;
} else {
    $class = $param->getType() && !$param->getType()->isBuiltin() && $param->getType() instanceof \ReflectionNamedType ? $param->getType()->getName() : null;
}

Works in PHP 7.0 as well as 8.0: https://3v4l.org/qRavH

@vaclavgreif
Copy link

@TRPB Any chance to add this to core? Using PHP8 on some projects already.

@cseufert
Copy link

If anyone is interested, I am now maintaining a fork here with PHP 8 support:
https://packagist.org/packages/moddengine/dice#v4.1.0

@jpirnat
Copy link

jpirnat commented Mar 24, 2021

@cseufert Could you apply the fix in #183 to your fork?

@cseufert
Copy link

@jpirnat
I have added that fix, and also added tests for that case

https://github.com/moddengine/Dice/releases/tag/v4.1.1

@jpirnat
Copy link

jpirnat commented Mar 25, 2021

@cseufert Your fork breaks Dice named instances from v4.1.0 onward, at least for PDO.

Test script:

<?php
declare(strict_types=1);

require __DIR__ . '/vendor/autoload.php';

$dice = new \Dice\Dice();

$host = '127.0.0.1';
$port = '3306';
$name = 'schema_name';
$user = 'username_here';
$pass = 'password_here';
$rule = [
	'constructParams' => [
		"mysql:host=$host;port=$port;dbname=$name;charset=utf8mb4",
		$user,
		$pass,
		[
			PDO::ATTR_EMULATE_PREPARES => false,
			PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
		]
	],
	'shared' => true,
];
$dice = $dice->addRule(PDO::class, $rule);

$rule = [
	'instanceOf' => PDO::class,
	'constructParams' => [
		"mysql:host=$host;port=$port;dbname=$name;charset=utf8mb4",
		$user,
		$pass,
		[
			PDO::ATTR_EMULATE_PREPARES => false,
			PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
		]
	],
	'shared' => true,
];
$dice = $dice->addRule('$database2', $rule);

$database1 = $dice->create(PDO::class);
$database2 = $dice->create('$database2');

var_dump($database1);
var_dump($database2);

Tests run with PHP v7.4.2 on a Mac with MAMP v5.7

Output with level-2/dice v4.0.2 and moddengine/dice v4.0.2:

object(PDO)#12 (0) {
}
object(PDO)#22 (0) {
}

Output with moddengine/dice v4.1.0


Notice: Undefined index: $database2 in /Users/jesse/dicetest/vendor/moddengine/dice/Dice.php on line 140
object(PDO)#12 (0) {
}
NULL

Output with moddengine/dice v4.1.1:


Notice: Undefined index: $database2 in /Users/jesse/dicetest/vendor/moddengine/dice/Dice.php on line 216
object(PDO)#12 (0) {
}
NULL

I would leave this as an issue directly on your fork, but uhh, that's not an option, so here it is instead.

@cseufert
Copy link

@jpirnat This is now fixed in 4.1.3

@jtojnar
Copy link

jtojnar commented May 1, 2021

This has been fixed in 4.0.3: https://github.com/Level-2/Dice/releases/tag/4.0.3

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

No branches or pull requests

6 participants