DateTime: Fix type error - modifyClone

[lukaspijak]

• bug fix
• BC break? yes/no?

I have a problem with Nette\Utils\DateTime. I use modifyClone method:

$date = new Nette\Utils\DateTime('now');

$date->modifyClone('xx'); // Invalid date string

And this cause the error:

TypeError: Nette\Utils\DateTime::modifyClone(): Return value must be of type Nette\Utils\DateTime, bool returned

because DateTime::modify returns DateTime|false and Warning:

E_WARNING: DateTime::modify(): Failed to parse time string (xx) at position 0 (x): The timezone could not be found in the database

We can solve it by that object will only cloned like that:

    public function modifyClone(string $modify = ''): static
    {
        $dolly = clone $this;
        return $modify ? (@$dolly->modify($modify) ?: $dolly) : $dolly;
    }

or we can throw an exception:

    public function modifyClone(string $modify = ''): static
    {
        $dolly = clone $this;

        $dolly = @$dolly->modify($modify);
        if ($dolly === false) {
            throw new \Exception('Failed to parse time string');
        }
        return $dolly;
    }

What do you think?

[dg]

An exception would certainly seem better than silencing an error.

How about adding a new modify() that returns static and throws an exception in case of an error, and modifyClone would call it?

[lukaspijak]

Yes I do not like silencing errors too but i do not want to cause some BC break.

Better?

[dg]

Great, thanks!

[dg]

There is a slight complication here, because PHP since version 8.3 throws its own exceptions https://wiki.php.net/rfc/datetime-exceptions

[lukaspijak]

Maybe you can create a Nette exception that inherits from the native one.

public function modifyClone(string $modify = ''): static
{
    $dolly = clone $this;

    try {
        $dolly = @$dolly->modify($modify);
        if ($dolly === false) {
            throw new NetteDateException('Failed to parse time string');
        }
        return $dolly;
    }
    catch (NativeDateException $e) {
        throw new NetteDateException($e->getMessage(), $e->getCode(), $e);
    }
}

if (PHP_VERSION_ID >= 80300)
{
    class NetteDateException extends \NativeDateException
    {
    }
}
else
{
    class NetteDateException extends \Exception
    {
    }
}

Something like this?

[dg]

Look, I don't think I want to deal with this. For one thing, the modifyClone() function was created at a time when you couldn't write (clone $dt)->modify() in PHP, so I see it as irrelevant today. And since PHP 8.3 will bring a whole new range of exceptions, I won't be introducing my own exceptions.

[lukaspijak]

Ok, no problem