Support setting the Date facade to use CarbonImmutable instead of Carbon

[sshead]

I love this package, but it currently breaks in my app, because in my Laravel apps I prefer to set the Date facade to use CarbonImmutable instead of Carbon (as per this excellent blog post by my fellow Aussie Michael Dyrynda).

This PR fixes that. The only things I had to change were:

1. Declare the types of datetime properties and parameters as Carbon|CarbonImmutable rather than just Carbon. This applies to activated_at, deactivated_at, created_at and updated_at.
2. Call Date::now() rather than Carbon::now() (only in ShortURLVisitFactory.php).

I've added tests as well - hope I didn't go overboard with those! If there's a better way of testing, let me know. (And if I've butchered this PR completely, apologies - I'm a spare-time hobbyist dev, so I very rarely attempt these.)

[ash-jc-allen]

Hey @sshead, thanks for the PR! 😄

When I first built this package, I was still kinda new to everything, so there's some decisions I made (because I didn't know any better) than I definitely wouldn't do now. One of them being, I would have encouraged the use of interfaces and/or immutable dates instead of directly using the Carbon\Carbon class in the signature like that.

So I love the idea behind this change! But I'm wondering if there's a way for us to do this without changing the method signature and causing a breaking change? 🤔

[sshead]

Oh, it never occurred to me that any change to a method signature would be a breaking change. Is that the case even if you're broadening the types of a parameter? With inheritance this is fine (contravariance) ... but as you can tell, I'm not a package developer!

Anyway, I think there is another way: Simply change some instances of the now() Laravel helper to Carbon::now() - in particular, where an assignment is made to a model field. For example, change:

    $this->activateAt = now();

to:

    $this->activateAt = Carbon::now();

I've tried it locally and it seems to work - tests pass, and it doesn't break my app that uses CarbonImmutable by default. Would you like me to submit a new PR with that solution?

[sshead]

OK, so deep dive was well worth it! It turns out that it's a one-line fix, at least to get it working with an app that uses CarbonImmutable. I've submitted a new PR (#285), and I'll close this one.

The method signatures that I changed previously - for activateAt() and deactivateAt() on the Builder class - are fine to leave. At worst, they may be slightly inconvenient or annoying if you've configured your app to use CarbonImmutable, since you'd have to manually pass in the right thing (e.g. using Carbon::now() instead of the global now() helper). But that's very minor.