IntlExtension >= 3.8 throws Exception when formatDateTime is used and $timezone === null

[TPantelConventus]

Background

The Exception occurred after I updated from 3.7 to 3.8 and the Workaround for me right now is to downgrade to 3.7.

Description

I use the fomat_datetime filter in many places in my app like this:

{{ presentation.presentationFileUploadedAt | format_datetime('medium', timezone=false, locale=app.request.locale) }}

Before the update of "twig/intl-extra" from 3.7 to 3.8 that worked without any problems, but when i updated to 3.8 it suddenly gave me Excpetions:

An exception has been thrown during the rendering of a template ("Twig\Extra\Intl\IntlExtension::createDateFormatter(): Argument #5 ($timezone) must be of type ?DateTimeZone, bool given, called in /var/www/html/vendor/twig/intl-extra/IntlExtension.php on line 379").

That was wondering me a lot, so I looked into IntlExtension.php to look for the corresponding line:

/**
     * @param \DateTimeInterface|string|null  $date     A date or null to use the current time
     * @param \DateTimeZone|string|false|null $timezone The target timezone, null to use the default, false to leave unchanged
     */
    public function formatDateTime(Environment $env, $date, ?string $dateFormat = 'medium', ?string $timeFormat = 'medium', string $pattern = '', $timezone = null, string $calendar = 'gregorian', string $locale = null): string
    {
        $date = twig_date_converter($env, $date, $timezone);

        $formatterTimezone = $timezone;
        if (null === $formatterTimezone) {
            $formatterTimezone = $date->getTimezone();
        } elseif (\is_string($formatterTimezone)) {
            $formatterTimezone = new \DateTimeZone($timezone);
        }
      // THE NEXT LINE IS LINE 379
  $formatter = $this->createDateFormatter($locale, $dateFormat, $timeFormat, $pattern, $formatterTimezone, $calendar); 

        if (false === $ret = $formatter->format($date)) {
            throw new RuntimeError('Unable to format the given date.');
        }

        return $ret;
    }

So formatDateTime (which the twig filter uses) accepts $timezone === false, but then in line 379 createDateFormatter() is called with $formatterTimezone being false, throwing the Exception, because:

private function createDateFormatter(?string $locale, ?string $dateFormat, ?string $timeFormat, string $pattern, ?\DateTimeZone $timezone, string $calendar): \IntlDateFormatter

The solution would be to modify the formatDateTimeFunction:

/**
     * @param \DateTimeInterface|string|null  $date     A date or null to use the current time
     * @param \DateTimeZone|string|false|null $timezone The target timezone, null to use the default, false to leave unchanged
     */
    public function formatDateTime(Environment $env, $date, ?string $dateFormat = 'medium', ?string $timeFormat = 'medium', string $pattern = '', $timezone = null, string $calendar = 'gregorian', string $locale = null): string
    {
        $date = twig_date_converter($env, $date, $timezone);

        $formatterTimezone = $timezone;
        if (null === $formatterTimezone || false === $formatterTimezone ) {
            $formatterTimezone = $date->getTimezone();
        } elseif (\is_string($formatterTimezone)) {
            $formatterTimezone = new \DateTimeZone($timezone);
        }
      // THE NEXT LINE IS LINE 379
  $formatter = $this->createDateFormatter($locale, $dateFormat, $timeFormat, $pattern, $formatterTimezone, $calendar); 

        if (false === $ret = $formatter->format($date)) {
            throw new RuntimeError('Unable to format the given date.');
        }

        return $ret;
    }

Replacing the twig filter from timezone=false to timezone=null is NOT a valid solution, as it prevents the Exception from being thrown, BUT results in a wrong time being displayed, because the $date variable in the function is depending on $timezone and not $formatterTimezone

[xabbuh]

probably related to #3903

/cc @keulinho

[fabpot]

What about checking for null and false?

[xabbuh]

To be honest from reading the comments I did not really understand if and if so how the behaviour should have beeen different for null and false or if they should always have been treated the same way.

[lochmueller]

Hey @xabbuh
do you try this?
{{ presentation.presentationFileUploadedAt | format_datetime('medium', timezone=null, locale=app.request.locale) }} or
{{ presentation.presentationFileUploadedAt | format_datetime('medium', locale=app.request.locale) }} (null is the default)
Why you set the timezone to "false" instead of "null", because null is already the "undefined" state. I think the formatDateTime method should be type hinted with "?string" to avoid wrong variable types...?!
Regards,
Tim

[TPantelConventus]

Hey @xabbuh do you try this? {{ presentation.presentationFileUploadedAt | format_datetime('medium', timezone=null, locale=app.request.locale) }} or {{ presentation.presentationFileUploadedAt | format_datetime('medium', locale=app.request.locale) }} (null is the default) Why you set the timezone to "false" instead of "null", because null is already the "undefined" state. I think the formatDateTime method should be type hinted with "?string" to avoid wrong variable types...?! Regards, Tim

Because setting the timezone to null (the default) will lead to a different output than setting the timezone to false, which is intended and also documented in the Twig docs:

Timezone
By default, the date is displayed by applying the default timezone (the one specified in php.ini or declared in Twig -- see below), but you can override it by explicitly specifying a timezone:

{{ datetime|format_datetime(locale='en', timezone='Pacific/Midway') }}

If the date is already a DateTime object, and if you want to keep its current timezone, pass false as the timezone value:

{{ datetime|format_datetime(locale='en', timezone=false) }}

[fryght]

Looking at the code it seems that the CoreExtension::dateConverter already takes care of timezone=false (or setting the proper timezone when one is passed and loading the default timezone when null is passed)

With that, it seems extraneous to also attempt handling the timezone again in the format_datetime filter.