handle invalid timezones Australia/ACT and Europe/Kiev

[kevinpapst]

Fixes #342

I hope the code comments explain the reasoning behind the changes.

All my systems seem to support both variants, but I had multiple bugs raised about Australia/ACT and Europe/Kiev. Can't say when this happens, maybe it depends on the tzdata lib on the system where PHP was compiled. Or some PHP builds remove all timezone name which are only so called "links".

In the official table (see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones ) both problematic TZ are only links, so I replaced them with their real TZ identifier.

Edit: I used two approaches, so you can decide which way is the better one. Use the correct TZ identifier directly (Sydney) or use the try&catch approach (Kiev). Maybe try&catch in both places is the best approach for maximum BC?

[stelgenhof]

Thank you very much @kevinpapst for this PR! Unfortunately there are sometimes backwards incompatible changes introduced in PHP. In the past there were similar BC issues with historical DST changes between PHP versions.

We could also move away from the timezone identifiers (type 3) altogether, and use the UTC offset (type 1) or the timezone abbreviation (type 2), as these seem less prone to changing. The benefit of the timezone identifier is of course it is more human friendlier than the other types.

[kevinpapst]

Yeah, I fought in the past already with the Kiev removal, but that was in the scope of my time-tracking app, so I could take care of it myself. I use try&catch for it.

I think the timezone identifier is totally fine to use, it doesn't change regularly and the try&catch approach works flawless to handle this situation. Changing it to TZ offset would be a bigger BC break, as many of the $timezone variables are public and therefor you would change the public API.

How to move forward?

1. Shall I change the the Australia class to try&catch as well?
2. Never used psalm, what is the equivalent of @phpstan-ignore-line for the "dead catch" warning?

[stelgenhof]

Yeah, wouldn't think the timezone identifier changes regularly either, but thought the others could be an option instead. However indeed that would break Yasumi's API :(

I think the try&catch approach is a good approach. Please go ahead and use the same for Australia. Let me check on that dead catch issue in regards to Psalm.

[kevinpapst]

I already did change Australia. And Psalm doesn't report it, it was only PHPStan. Checks are green, ready for review/merge from my end.

[kevinpapst]

Ok for the Changelog, but tell me how to test these changes.

The result depends on the environment / PHP runtime.

[thrashzone13]

Hi @stelgenhof @kevinpapst !

I just wanted to suggest using phpversion() . It's self explaining and better than using a try catch

[kevinpapst]

Does not depend on phpversion afaik, so this approach is not usable. Afaik this depends on the libicu version PHP was compiled against (or something like that, not sure about the exact internals). But the issue happened with PHP 8.1 and 8.3 and I have enough systems with these version where it doesn't happen.

[stelgenhof]

I was thinking the same to use the PHP version for testing. If that turns out to be difficult, we can forgo the tests :)

[stelgenhof]

@kevinpapst Can you only update the changelog? If done, then I'll merge it. Thanks!

[kevinpapst]

@stelgenhof Changelog adjusted: done.

Any chance you could create a bugfix release? I have production bugs with several users.