[Clock] Add DatePoint: an immutable DateTime implementation with stricter error handling and return types

[nicolas-grekas]

Q	A
Branch?	6.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	-
License	MIT
Doc PR	-

The native DateTime/Immutable classes have some legacy that we can get rid of by creating a child class.

Here comes Symfony\Component\Clock\DatePoint, which extends the native DateTimeImmutable and provides the following advantages:

• usable in defaults expressions
• shorter (short) class name
• throws exceptions instead of returning false
• and last but not least, uses the global static clock to define "now" (aka new DatePoint is compatible with Clock::set())

This also opens the possibility to add more methods to this DatePoint class in the future if we want to. <- we don't want that :)

[derrabus]

The native DateTime/Immutable classes have some legacy that we can get rid of by creating a child class.

Nice. 🙂

This also opens the possibility to add more methods to this DateTime class in the future if we want to.

That's my biggest fear about adding such a class, tbh.

If by "more methods" we mean polyfills for features that will be added in future PHP versions: yes, totally. But please, let's not open this class up for all kinds of utility methods. 🙈

Oh, and please add tests for that new class. 🙏🏻

[GromNaN]

This also opens the possibility to add more methods to this DateTime class in the future if we want to.

That's my biggest fear about adding such a class, tbh.

If by "more methods" we mean polyfills for features that will be added in future PHP versions: yes, totally. But please, let's not open this class up for all kinds of utility methods. 🙈

We don't want to create a new Carbon class.

[antonkomarev]

Have you looked at the Brick DateTime library? Its API looks very good and has many types for many cases. Much more stricter and more powerful than Carbon.

[derrabus]

We don't want to create a new Carbon class.

My point exactly.

[nicolas-grekas]

PR ready, with tests !

We don't want to create a new Carbon

Same here, I stroke the line in the PR description. Guarding this is why we have a core-team :)

Have you looked at the Brick DateTime library

I didn't, but we just ruled such approaches out so 🤷

[nicolas-grekas]

Next steps to consider:

• doctrine types (Feature request: Add support for TypeFieldMapper doctrine/DoctrineBundle#1696 would be nice to help with that, help wanted)
• form types

But before, votes pending @symfony/mergers ;)

[wouterj]

I think it's not a good idea to name this class similar to one from PHP's stdlib that has different behavior. You'll have to scroll up to the use statements to know if new DateTime means the PHP or Symfony one in any project.

The only case where this is desired is our polyfill packages, which provide polyfills for stdlib features. Eventhough all behavior changes in this class are probably the desired behavior for DateTime of all PHP internal devs, there is no official agreement/RFC that this is truly the behavior of the next PHP version as far as I know. We should make sure not to pretend to make a polyfill.

[kbond]

I think it's not a good idea to name this class similar to one from PHP's stdlib that has different behavior.

Timestamp?

[Kocal]

To me, PHP should have only offered a DateTime class (no DateTimeImmutable) which would be immutable, and I understand that you want to improve things.

But I don't think this is the job of a framework / component to do that, as the class name is identical but for a totally different and breaking behavior. Sure you explicity import it, but people will need to check twice if this DateTime is imported from the Symfony Clock or is the one from PHP core.

[nicolas-grekas]

DateTime has my preference. It's how it should have been from the beginning. Unless they go with a very very bad BC break, PHP can't fix this. The ship has sailed on the topic 🤷 .

Figuring out the namespace requires a basic IDE and hovering the symbol, no need to scroll up. For ppl that don't use use for root-namespaced classes (following Symfony's recommended CS), there's nothing to do.

there is no official agreement/RFC that this is truly the behavior of the next PHP version

I'm not sure to get what you mean here. The proposed class is not a polyfill. It's a stricter DateTime. We don't need any agreement from php-core to be future proof, thanks to LSP.

Still, if we want another name, I'd propose Now.
This came to my mind when I realized that the implementation of the now($modifier) helper can be reduced to the following (if you look at the attached patch, the implementation has a few more lines, but that's just for optimizing performance.):

function now(string $modifier = 'now')
{
    return new Now($modifier);
}

[kbond]

Not a fan of Now:

public function getLastUpdated(): Now

[GromNaN]

ClockDateTime would be descriptive. Even if that repeats the Symfony\Component\Clock namespace.

[ro0NL]

BetterDateTime? because why not 😅

[nicolas-grekas]

DateMark
DatePoint
DateTick
?

[wouterj]

To be honest, since typing my previous message I'm a bit on the fence on whether we should want this feature at all. I don't feel comfortable with a framework that "fixes"/"patches" the standard library.
In my eye, Symfony has always provided low and high level APIs on top of the standard library. This is strong: you're not writing some custom language based on PHP, you're just writing PHP. I like how we as Symfony developers aren't "writing Symfony" like everyone used to say in the jQuery days, but most people in the community are aware that they are writing PHP.

Of course, this is only one class, hence I'm not 100% against it, but if we accept this, would we also accept e.g. adding new namespaced functions that introduce strict error handling to stdlib functions that currently return false on error? (i.e. https://github.com/azjezz/psl) And if we don't (I hope we don't!), why would we accept this?

---

This is a language issue that should be solved on language level imho. Create a new date time object (you can directly properly namespace it) and make it behave like it should with our current experience. In a major version or two, deprecate the current date time objects and we're done. PHP has managed much bigger API BC breaks like this (e.g. mysql_* to MySQLi/PDO).

[nicolas-grekas]

The core motivation for adding this class on my side is to be able to create date-time instances that leverage the global static clock. Aka to make new DateTime mockable. That's a unique feature that can't be provided by any future PHP version. The rest is a nice bonus to me and I'd agree with you if strictness was the only argument.

One could say that the now() helper is enough, but it can't be used as a default argument value. Adding the class would:

function foo(DateTime $date = new DateTime) {...}

[wouterj]

Thanks for the explanation. While using Cronos, I haven't found the need to new DateTime as a default value, but I can see the added value of it for the Clock component. Now, your Now name suggestion also makes lots of sense :)

I'm now learning towards reducing this class to exactly this use-case: being the now() equivalent for cases where you need a constant expression, without all the "API improvements". Something along the lines of this:

class Now extends \DateTimeImmutable
{
    public function __construct()
    {
        $now = Clock::get()->now();

        parent::__construct($now->format('c'), $now->getTimezone());
    }
}

This does not broaden the scope of the Clock component and keeps us far away from inventing our own language :)

[nicolas-grekas]

I like it too! PR updated, TimePoint FTW!

[weaverryan]

TimePoint is fine for me. I prefer DatePoint as "Date" makes a closer association with DateTime than the word "Time".

But we can also call it RyansDate - that's probably my favorite 😇

[nicolas-grekas]

Updated to DatePoint 🤘

I like that typing Date when using autocompletion might help discover the new class.

[OskarStark]

That makes sense 👍🏻😃

[fabpot]

Thank you @nicolas-grekas.