tecnickcom / TCPDF

Official clone of PHP library to generate PDF documents and barcodes
https://tcpdf.org
Other
4.22k stars 1.52k forks source link

Fixes for PHP 8.2 in tcpdf_fonts.php - fixes #632 #633

Closed littlepackage closed 1 year ago

littlepackage commented 1 year ago

Fixes "use of "self" in callables is deprecated" warning is arising from tcpdf_fonts.php when using PHP >= 8.2

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

littlepackage commented 1 year ago

PHP 5.4... Wow. This should work down to PHP 5.5.

I guess I'd have to write some sort of phpversion() check. Would that be OK?

williamdes commented 1 year ago

PHP 5.4... Wow. This should work down to PHP 5.5.

I guess I'd have to write some sort of phpversion() check. Would that be OK?

I checked composer.json it's even worse: 5.3 I think that would be okay, keep your patch for all versions and do an if for old versions to have the old version of the code

littlepackage commented 1 year ago

5.3! Gasp! 😋 I was able to test and confirm another approach (committed to #633) works using PHP 5.3.29 through 8.2.

littlepackage commented 1 year ago

Spaces done, I think?

williamdes commented 1 year ago

Please add Fixes: #632 to your PR description so it links it and closes it on merge

littlepackage commented 1 year ago

Sorry - I've made a mistake. This fix doesn't work, and probably won't work without a phpversion() conditional and an include() to move code (self::class) which WILL break 5.3 to another file. PHP 5.5 is as low as the "easy fix" goes. Awkward! Hopefully other ideas roll in.

williamdes commented 1 year ago

Sorry - I've made a mistake. This fix doesn't work, and probably won't work without a phpversion() conditional and an include() to move code (self::class) which WILL break 5.3 to another file. PHP 5.5 is as low as the "easy fix" goes. Awkward! Hopefully other ideas roll in.

Is there only 5.3 that is broken?

littlepackage commented 1 year ago

This fix starts working with PHP 5.5.

It seems it'd be a bit of an ordeal to make it work 5.3-8.2 because of these callables. Do-able but not pretty!

littlepackage commented 1 year ago

Interestingly, PHP 5.3 EOL was almost exactly ten years ago: 14 Aug 2014 đŸ˜ŗ

Edit: Today felt like 2024.

williamdes commented 1 year ago

Interestingly, PHP 5.3 EOL was almost exactly ten years ago: 14 Aug 2014 đŸ˜ŗ

đŸ˜ŗ😨

Can it be a variable and replaced if version compare === old?

littlepackage commented 1 year ago

5.3 parses everything before run and when it sees self::class, it breaks. ☚ī¸ That's why I was mentioning using phpversion() to check for >8.2 and including that in a separate file, maybe.

williamdes commented 1 year ago

self::class

If I am not mistaken self::class is just a string in the end, can you hard code the string ? I would bet it's just \CLASS_NAME

littlepackage commented 1 year ago

self::class is a string but 5.3 sees that :: in the file and đŸ˜ĩ

Something like this might work:

$selfclass = get_called_class();
if ( $isunicode ) {
    return array_map($selfclass . '::unichrUnicode', $ta );
}
return array_map($selfclass . '::unichrASCII', $ta );

I'm testing now.

williamdes commented 1 year ago

self::class is a string but 5.3 sees that :: in the file and

I meant that is you var_dump(self::class) you will get it's final form ;)

littlepackage commented 1 year ago

Sure, TCPDF_Fonts. Have you tried what you're suggesting?

williamdes commented 1 year ago

Sure, TCPDF_Fonts. Have you tried what you're suggesting?

Hmm, sorry I got confused in my brain

So with a version compare it can be changed between TCPDF_FONTS and self. Should be okay for the linter and the code execution, right ?

littlepackage commented 1 year ago

Because writing self::class anywhere in a file called by TCPDF (such as tcpdf_fonts.php) will break PHP 5.3. Similar to how using array() notation [] in a PHP 5.3 file will break it. The other solution I've mentioned with include() is much more complicated, and involves checking for PHP > 8.2, and including a separate file conditionally which isn't seen by 5.3 at all.

if ( version_compare( phpversion(), '5.3', '>' ) ) {
    if ( ! in_array( 'nuts', [ 'nutty', 'array' ] ) ) {
        // Doesn't even get here due to syntax error involving use of []
    die( 'Well, nuts' );
    }
}

Using get_called_class() (like I did above) is the simplest solution I've found that works. I'll do some more testing here to make sure.

littlepackage commented 1 year ago

I think when I've begun coding about nuts I've lost my mind. đŸĨœđŸ§  Will test this a bit more and update the PR!

williamdes commented 1 year ago

Because writing self::class anywhere in a file called by TCPDF (such as tcpdf_fonts.php) will break PHP 5.3. Similar to how using array() notation [] in a PHP 5.3 file will break it. The other solution I've mentioned with include() is much more complicated, and involves checking for PHP > 8.2, and including a separate file conditionally which isn't seen by 5.3 at all.

if ( version_compare( phpversion(), '5.3', '>' ) ) {
    if ( in_array( 'nuts', [ 'nutty', 'array' ] ) ) {
        // Doesn't even get here due to syntax error
  die( 'Well, nuts' );
    }
}

Using get_called_class() (like I did above) is the simplest solution I've found that works. I'll do some more testing here to make sure.

I agree, that said you do not need to write self::class but only it's final form and it will work great too that said I like your get_called_class solution, it may solve this nicely without too much lines it it maybe works for all php versions

littlepackage commented 1 year ago

Using something like 'TCPDF_FONTS::uniord' would work in older versions of PHP but not newer versions 🤷‍♀ī¸ It stopped working with PHP 8. I'm glad you like the get_called_class() solution well enough 😊

littlepackage commented 1 year ago

You are welcome!

I've been using this library since well... PHP 5.3, so it's the least I can do to finally jump in on Github and help. đŸ‘ĩđŸŧ

williamdes commented 1 year ago

You are welcome!

I've been using this library since well... PHP 5.3, so it's the least I can do to finally jump in on Github and help. đŸ‘ĩđŸŧ

So cool! I will gladly welcome you on other open source projects I contribute to

jmv69800 commented 10 months ago

Hi I'm very new in TCPDF. Also very new in GitHub (first use, in fact). Please, accept my appolgies if I don't use GitHub correctly.

I tested the proposed fixe under PHP 8.2.9 x86 (in preparation to use TCPDF in PHP 8.3) ... and it seems not working.

I've reported the 3 new lines in tcpdf_fonts.php, and removed the old 3 existing ones, but I obtain that

_Deprecated: Optional parameter $isunicode declared before required parameter $currentfont is implicitly treated as a required parameter in .\tcpdf\include\tcpdf_fonts.php on line 2000, and 2027, 2043, etc..._

Also $setbom, $forcertl, $str, $default_css, $tagvs, etc... deprecated (perhaps due to the first deprecated)

Is there something I've done wrong ?