[FR] Support For DateTimeImmutable

[kiler129]

Problem

Running the bundle on 7.3 I discovered that last update date cannot be set to an instance of \DateTimeImmutable. The problem is caused by the typehint on UrlConcrete:

https://github.com/prestaconcept/PrestaSitemapBundle/blob/5ec8028a16c63c97ab328f7acf456eac571916bc/Sitemap/Url/UrlConcrete.php#L89-L94

Solution

The proper typehint would be \DateTimeInterface, but since the bundle supports PHP 5 I suggest that the setter is changed to manually trigger the same messages as native typehint:

    /**
     * @param DateTimeInterface|DateTime|null $lastmod
     *
     * @return UrlConcrete
     */
    public function setLastmod($lastmod = null)
    {
        if (!($lastmod instanceof \DateTimeInterface || $lastmod instanceof \DateTime)) {
            $typePassed = is_object($lastmod) ? \get_class($lastmod) : \gettype($lastmod);

            if (\PHP_MAJOR_VERSION >= 7) {
                throw new \TypeError('Argument 1 passed to ' . __METHOD__ . "() must be an instance of DateTimeInterface, '$typePassed' given");
            } else {
                \trigger_error('Argument 1 passed to ' . __METHOD__ . "() must be an instance of DateTime, '$typePassed' given", E_USER_ERROR);

                return $this;
            }
        }

        $this->lastmod = $lastmod;

        return $this;
    }

Backward compatibility

To compare the behavior:

• Old: https://3v4l.org/kgBOL
• New: https://3v4l.org/NHDnA

That way there is no difference in behavior of invalid arguments, but on PHP 7.3 it's possible to pass \DateTimeImmutable.

WDYT about such change? It will greatly simplify our workflow (so we don't have to convert the dates for every link).

[yann-eugone]

Looks like a good idea to me. I had a doubt about when the DateTimeInterface was introduced, so I checked : https://www.php.net/manual/fr/class.datetimeinterface.php >=5.5 should be enough.

Thanks for the idea btw ;)

[kiler129]

@yann-eugone I did a PR for that change: https://github.com/prestaconcept/PrestaSitemapBundle/pull/202

It simply bumps the PHP version to 5.5.0 and replaces DateTime with DateTimeInterface

[kiler129]

@yann-eugone Poke :yum:

[yann-eugone]

Sorry I'm late.

I don't understand why don't you just did the thing you were saying you will in the issue ? That solution look better to me, the issue was the version checking, as immutable dates were not introduced in PHP 7.

Could you stick to this, cancel your changes on the composer.json, and add NEXT_MAJOR methods comments everywhere ?

[yann-eugone]

This has been done and released in v2.0.0