Safe\DateTimeImmutable is incompatible with PHP 8

[sanmai]

\Safe\DateTimeImmutable::createFromFormat(
    DateTime::ATOM,
    '2020-04-26T07:32:25+00:00'
);

Results in the following on PHP 8:

Error: Call to a member function format() on null
vendor/thecodingmachine/safe/lib/DateTimeImmutable.php:75
vendor/thecodingmachine/safe/lib/DateTimeImmutable.php:38
vendor/thecodingmachine/safe/lib/DateTimeImmutable.php:64

Tested on v1.3.2.

[Kharhamel]

I need to wait for the doc to be updated to php8 before being able to release safe2.0
I can create a pre-release where I run the generator and the tests on php8 but it will still be incomplete

[LANGERGabrielle]

Related to https://bugs.php.net/bug.php?id=79975

[tjveldhuizen]

@Kharhamel Is there a timeline for fixing this? Now the 1.3.3-release claims to be compatible with PHP 8 (composer.json: >=7.2), I was surprised getting exceptions after upgrading PHP. Should the constraint be changed to ^7.2? Of is the incompatibility a bug and should it be fixed (for example with the PR by @janatjak)?

[webmozart]

Same problem here. This class is not compatible with PHP 8, so the version constraint should be changed.

[tjveldhuizen]

@Kharhamel How can we help moving this forward? The incompatibility is, unfortunately, blocking our upgrade to PHP 8.

[Kharhamel]

I am so sorry, I really didn't have time to work on safe in the last few months.

I actually created an separated project for the php8 version: https://github.com/thecodingmachine/safe8

Trying to manage the update to 8 on this project was just too dangerous because some huge libraries use this package.

It isn't ready yet, I have to update the rector directory to the lastest version, then hook that into packagist, then properly mark thecodingmachine/safe as deprecated. Problem is I am not great with rector

[tjveldhuizen]

I have never used Rector, unfortunately. But, certainly when you want to deprecate the current repo, I think you should correct the required PHP version in composer.json. In the current situation Composer allows to update PHP, while it is not compatible.

[tjveldhuizen]

By the way, releasing the PHP 8-version of this bundle as version 2.0.0 should not break existing installations, isn't it? Major versions are allowed to have BC-breaks, everybody using it on PHP 7 should have added a constraint like ^1.3, making it not automatically upgrade to 2.0.0.

[tpetry]

@Kharhamel I was not aware there is a specific version for PHP 8. You should add a note at the top of the readme of the current version to inform people to switch to safe8 when using php 8.

And @tjveldhuizen is correct, the current approach is problematic when a project/library is designed to run on PHP 7 and PHP 8 because both packages will conflict. I support @tjveldhuizen suggestion on making safe8 the version 2 so a dependency like thecodingmachine/safe:1.*|2.* can be used. With two different packages no library can use safe anymore if they want to support both php major versions.

[rieschl]

By the way, releasing the PHP 8-version of this bundle as version 2.0.0 should not break existing installations, isn't it? Major versions are allowed to have BC-breaks, everybody using it on PHP 7 should have added a constraint like ^1.3, making it not automatically upgrade to 2.0.0.

Actually, I think making a separate repo for php8 will cause even more headache for other packages requiring this library and want to support php7 and php8. what should they put in their require section?

why don't you just let composer sort that out? Afaik you don't even have to tag a major version. If you release a version which requires php: ^8.0, composer won't pull that on php7.
technically, I think it's easier to have two separate major versions, one for php7 and one for php8, but I'm not an expert on this.

[boesing]

I am on the +1 list to have a new major bump for PHP 8.
There is no way of telling composer to install one package only on PHP 8 while the other has to be installed on PHP 7.
Since there are plenty of libraries using thecodingmache/safe, that would be a thing tho.

Any particular reason why a new major bump in this package was not an option? Having a few more insights would be nice :-)
Thanks for all the work so far!

[rieschl]

I hope this issue can be resolved somehow.
In the meantime we use a wrapper package to have the possibility to use safe and safe8 at the same time.
It's a bit inconvenient if you use ComoserRequireChecker but better than not using Safe at all.
feel free to use it, we made it public: https://github.com/eventjet/thecodingmachine-safe-meta

Thank you very much for hard work!

[Jean85]

I'm sorry, I'm jumping in late here but... Why can't we just fix this with what I'm proposing in #278?

[rieschl]

I'm sorry, I'm jumping in late here but... Why can't we just fix this with what I'm proposing in #278?

That would really be awesome. That's the only thing that keeps us from PHP8 in some packages. Would it be possible to merge that, even though this library is deprecated, @Kharhamel ? Thank you very much!

[dbrekelmans]

This has been solved in #278