Avoiding the silence operator

[Synchro]

I've run into an issue with errors being suppressed by the @ operator. Specifically this occurred relating to use of strftime, which was deprecated in PHP 8.1. #672 reported this problem, which was "fixed" by silencing it in #811, and that code is still present in 5.0. There's a problem with using the @ operator like this – I use Smarty in a context where users can create their own templates, and I run them inside a test harness using a custom error handler to check they are correct before allowing them to be used. The problem here is that while the @ operator suppresses error output, it doesn't suppress the internal occurrence of the error, so the warning still occurs, it's just not reported by the default handler. Since I'm not using a default handler, I'm hit with the errors as if they had not been suppressed at all.

As a workaround I'm now filtering deprecation warnings during this process, however, these errors only exist in Smarty's own code (like in #811) where I don't have control over it. Ideally I'd like these errors not to be happening in the first place. In other places I've used a BC polyfill to replace strftime with a userland implementation, which I feel is a far better way of solving the problem than using @, though ultimately avoiding the deprecation altogether would better, if it's possible.

Another factor is that while this operator suppresses the deprecation notice, it also suppresses any other errors that occur within the call, for example if an invalid format string is provided.

I think this silencing approach has been used elsewhere in Smarty to avoid deprecation notices, and I just don't think it's a great policy. It's also exactly the kind of thing that gets flagged by static analysis tools and code audits. Thoughts?

[wisskid]

I agree. Let's try and find a better way

[Synchro]

That's good! Would you have any objection to adding a dep using that BC package to solve strftime issues in particular? If not I'm happy to build a PR for that. The alternative would be to try to find some other function that's compatible with that style of date formatting, which is likely to be more difficult.

[wisskid]

As discussed in #672 we'd have to use a polyfill and add a php extension to the requirements. Some people object to that. I figured there would be some consensus on how to replace strtime when the release date for php9 was coming closer.

[wisskid]

But what if we provide two meta-packages on packagist. Or actually 1 package with 2 versions. One version supports PHP up to v8 without the ext-intl extension, and the other version supports PHP from v9, with the ext-intl extension. That way, it wouldn't be an extra requirement for anyone currently running php8 and we wouldn't need a new major.

[Synchro]

Looking at the code in the BC package, I see that the only things the intl ext is used for is the %c, %x, and %X codes. I think it should be possible to map everything else without intl which might provide a halfway solution. I came across this package which claims to handle locales without intl, but it's not entirely clear.

Another alternative would be to break out the plugins that require strftime – there are only 3 occurrences of strftime in the whole of Smarty as far as I can see.

[wisskid]

And %a, %A, %b and %B I guess?

[Synchro]

Oh, yes, missed those, and %h. I don't know how the old implementation did all those without ICU/intl.