Skip to content

PHP 8.1: Fix implicit conversion from float to int#387

Merged
nicolaasuni merged 1 commit intotecnickcom:mainfrom
MauricioFauth:implicit-float-to-int
Jul 20, 2021
Merged

PHP 8.1: Fix implicit conversion from float to int#387
nicolaasuni merged 1 commit intotecnickcom:mainfrom
MauricioFauth:implicit-float-to-int

Conversation

@MauricioFauth
Copy link
Copy Markdown
Contributor

@MauricioFauth MauricioFauth commented Jun 11, 2021

Fixes PHP 8.1 deprecation notice: Implicit conversion from float to int loses precision.

PHP Deprecated:  Implicit conversion from float 1.1768666666666663 to int loses precision in /home/runner/work/phpmyadmin/phpmyadmin/vendor/tecnickcom/tcpdf/tcpdf.php on line 4179
PHP Deprecated:  Implicit conversion from float 1.1768666666666663 to int loses precision in /home/runner/work/phpmyadmin/phpmyadmin/vendor/tecnickcom/tcpdf/tcpdf.php on line 4180

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 11, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@MauricioFauth MauricioFauth mentioned this pull request Jun 11, 2021
Closed
10 tasks
williamdes
williamdes previously approved these changes Jun 11, 2021
View reviewed changes
@mvorisek
Copy link
Copy Markdown
Contributor

mvorisek commented Jun 20, 2021

I think the fix is done at a wrong place, the char should be always an integer.

@MauricioFauth
Copy link
Copy Markdown
Contributor Author

MauricioFauth commented Jun 21, 2021 •
edited
Loading

I think the fix is done at a wrong place, the char should be always an integer.

This is done that way in other places too. See:

TCPDF/tcpdf.php

Line 4685 in cf8a66c

return (isset($this->CurrentFont['cw'][intval($char)]));

@mvorisek
Copy link
Copy Markdown
Contributor

mvorisek commented Jun 21, 2021

Hmm, what $char should represent? I still belive it is wrong somewhere else, even the phpdoc at method level say it should be integer.

@MauricioFauth
Copy link
Copy Markdown
Contributor Author

MauricioFauth commented Jun 21, 2021

This stack trace may help find the issue.

vendor/tecnickcom/tcpdf/tcpdf.php:4179
vendor/tecnickcom/tcpdf/tcpdf.php:6382
vendor/tecnickcom/tcpdf/tcpdf.php:5956
vendor/tecnickcom/tcpdf/tcpdf.php:3498
vendor/tecnickcom/tcpdf/tcpdf.php:3602
vendor/tecnickcom/tcpdf/tcpdf.php:3269
vendor/tecnickcom/tcpdf/tcpdf.php:3181

@MauricioFauth
Copy link
Copy Markdown
Contributor Author

MauricioFauth commented Jun 21, 2021

Anyway, this pull request is a quick fix to remove the deprecation notice, while keeping the current behavior.

nicolaasuni
nicolaasuni previously approved these changes Jul 10, 2021
View reviewed changes
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 10, 2021 •
edited
Loading

Codecov Report

Merging #387 (6c36556) into main (cf8a66c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #387   +/-   ##
=======================================
  Coverage   72.80%   72.80%           
=======================================
  Files         125      125           
  Lines       22959    22959           
=======================================
  Hits        16716    16716           
  Misses       6243     6243
Flag Coverage Δ
php-5.3-ubuntu-latest 72.28% <100.00%> (ø)
php-5.6-ubuntu-latest 45.14% <100.00%> (-0.01%) ⬇️
php-7.1-ubuntu-latest 45.69% <100.00%> (+<0.01%) ⬆️
php-7.2-ubuntu-latest 45.63% <100.00%> (-0.01%) ⬇️
php-7.3-ubuntu-latest 45.63% <100.00%> (+<0.01%) ⬆️
php-7.4-ubuntu-latest 45.63% <100.00%> (+<0.01%) ⬆️
php-8.0-ubuntu-latest 65.19% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tcpdf.php 70.93% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf8a66c...6c36556. Read the comment docs.

@nicolaasuni
Copy link
Copy Markdown
Member

nicolaasuni commented Jul 10, 2021

Please use intval instead of the int casting.

@MauricioFauth
Fix implicit conversion from float to int
456a714 
Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
@MauricioFauth MauricioFauth dismissed stale reviews from nicolaasuni and williamdes via 456a714 July 14, 2021 17:31
@MauricioFauth MauricioFauth force-pushed the implicit-float-to-int branch from 6c36556 to 456a714 Compare July 14, 2021 17:31
@MauricioFauth
Copy link
Copy Markdown
Contributor Author

MauricioFauth commented Jul 14, 2021

Please use intval instead of the int casting.

Done!

@nicolaasuni nicolaasuni merged commit ea9e1c4 into tecnickcom:main Jul 20, 2021
@MauricioFauth MauricioFauth deleted the implicit-float-to-int branch July 20, 2021 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicolaasuni nicolaasuni nicolaasuni approved these changes

+1 more reviewer

@williamdes williamdes williamdes left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@MauricioFauth @CLAassistant @mvorisek @codecov-commenter @nicolaasuni @williamdes