-
Notifications
You must be signed in to change notification settings - Fork 532
PHP 8 compatibility. #843
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 compatibility. #843
Conversation
The error is thrown in the HumHub project: PHP Deprecated Warning 'yii\\base\\ErrorException' with message 'Implicit conversion from float-string "42.95454545454545" to int loses precision' in /www/wwwroot/yta.yaro.info/protected/vendor/imagine/imagine/src/Imagick/Image.php:164 Stack trace: #0 [internal function]: yii\\base\\ErrorHandler->handleError() php-imagine#1 /www/wwwroot/yta.yaro.info/protected/vendor/imagine/imagine/src/Imagick/Image.php(164): Imagick->cropImage() php-imagine#2 /www/wwwroot/yta.yaro.info/protected/humhub/libs/ProfileImage.p...",
How can this error be reproduced? |
I just had it with PHP version 8. |
I was not able to reproduce that. |
I found this issue in HumHub project: https://www.humhub.com/. You can also try to pass a string as a number '8.123' or just float the number '8.123'. |
I think I see the problem now: $box = new Box('123.45', '123.45');
$point = new Point('123.45', '123.45');
var_dump($box->getWidth()); // int(123)
var_dump($point->getX()); // string(6) "123.45" So I think this needs to be fixed in the Lines 41 to 46 in c10ea7f
|
Point issue with non Integer values. Link: php-imagine#843
*/ | ||
public function __construct($x, $y) | ||
{ | ||
if (!\is_int($x)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we pass wrong arguments, they will be silently converted to 0.
I'd instead do something like
if (!is_numeric($x)) {
throw new InvalidArgumentException('x must be numeric');
}
$x = (int) $x;
if (!is_numeric($y)) {
throw new InvalidArgumentException('y must be numeric');
}
$y = (int) $y;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for better consistency, this should be handled the same way as Box
class does:
Lines 41 to 46 in c10ea7f
if (!\is_int($width)) { | |
$width = (int) round($width); | |
} | |
if (!\is_int($height)) { | |
$height = (int) round($height); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the Box class it's a bit easier, since it can't accept a width or a height with a value of 0 (so, an exception will be thrown if we pass a non-numeric value to it).
But I do agree that we should use round
.
So, what about something like this?
if (!is_numeric($x)) {
throw new InvalidArgumentException('x must be numeric');
}
$x = (int) round((float) $x));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn’t think of the 0
case.
So +1 for you latest code suggestion 👍
Superseeded by #847 |
Fix for
The error is thrown in the HumHub project:
PHP Deprecated Warning 'yii\base\ErrorException' with message 'Implicit conversion from float-string "42.95454545454545" to int loses precision'
in /www/wwwroot/yta.yaro.info/protected/vendor/imagine/imagine/src/Imagick/Image.php:164
Stack trace:
#0 [internal function]: yii\base\ErrorHandler->handleError() #1 /www/wwwroot/yta.yaro.info/protected/vendor/imagine/imagine/src/Imagick/Image.php(164): Imagick->cropImage() #2 /www/wwwroot/yta.yaro.info/protected/humhub/libs/ProfileImage.p...",