-
Notifications
You must be signed in to change notification settings - Fork 714
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
Smarty is not compatible with PHP 8.0 #605
Comments
Can you please provide a patch as well? Use |
Planned availability of PHP8 is Nov 26 2020. We have some work to do here. |
Can we start by setting up Travis to run PHP8 tests? Also, https://travis-ci.org/github/smarty-php/smarty/jobs/713094965 while we're at it. |
I've set-up a vagrant box with debian 10 and compiled php8beta3 with mbstring. After removing the $errcontext variable everywhere (see #604, but completely removed it, and removed from unit tests as well) the remaining fails from the unit tests seem to be limited:
Mostly changes to the unit tests themselves. |
Yes, many of the "undefined array key ..." errors are easily fixed: "A number of notices have been converted into warnings". We should just up the setErrorReporting to ~E_WARNING. |
Can we set the Travis first, so pull requests could start flowing? |
Yes, but to re-enable it (I disabled php nightly recently) we'll have to fix the unit tests first, as they all fail now. I'll take a shot at it. |
See #608 |
@wisskid I wonder if the For nested cases, an equivalent of the null-coalescing operator ( {if $object?.attribute?.nested}
{/if} (which would not cause any warning if any of the hierarchy is not set/null) |
Just switched over our test site running on Smarty 3.1.36 to PHP 8. The content is shown, no white screen. So application doesn't bail out. However the logs are filled with: Undefined array key "XXX". The vote for lifting that from notice to warning in PHP RFC Reclassifying engine warnings was 42 to 21... Any hope on fixing this besides changing errorreporting? |
Find where Smarty trap internal errors and see if you can amend for that. Though that would be a workaround at most. |
Well, a slightly quick & dirty fix is the following line of code when initialising smarty: I have close to zero warnings today when running site on PHP 7.4 with Smarty, so besides muting out other relevant PHP 8 warnings, it feels a bit safe to do so. I also assume this function only affects error levels within Smarty, but I'll check. |
Seems so at first glance. |
as the other issue is closed: I recommend that the default modifier is changed to: $output = '(($tmp = ' . $output . ' ?? null)===null||$tmp==='' ? (' . $param . ' ?? null) : $tmp)'; // edit by 2020.12.1 -- fixed PHP 8 (#617) has support for more default options and those also can be unset. So it is the same fix the topic starter of #617 suggest only this is also working for the params: {$array.layer1.DT_KVERVAL|default: $array.DT_KVERVAL: ''} we currently extend the template_plugin_modifiercompiler_default so we are not raising any errors on airbrake as it is also ignoring @. It is NOT pre 7.0 supported as the ??(Null coalescing operator) is 7.0+ (https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op) Maybe it is a good think to add some kind of legacy switch (maybe auto set in constructor) and diffentiate some code on that. So there is pre 7.0 support. |
Some sort of null coalesce support would be really great...
Not supported unfortunately: |
if you do: and use the template_plugin_modifiercompiler_default with the change as suggested in my post. You can do it. |
Thanks a lot for the tip! Has been working with smarty for several years but hadn't bumped in on this modifier previously. It seems to be the functionality I was looking for. |
I've added a specific unit test for this case and it passes perfectly in PHP8. |
https://github.com/php/php-src/blob/php-8.0.0beta1/UPGRADING
For example, this patch causes a fatal error
Fatal error: Uncaught Error: Undefined constant "TEST" in public_html/smarty/demo/templates_c/234d0264d74c0efb651cf2c28d8975810c02182d_0.file.index.tpl.cache.php:35
due to
https://github.com/php/php-src/blob/php-8.0.0beta1/UPGRADING
The code like this should not be used
The text was updated successfully, but these errors were encountered: