[PHP 8.4] Fixes for implicit nullability deprecation

[Ayesh]

Fixes all issues that emits a deprecation notice on PHP 8.4.

See:

• RFC
• PHP 8.4: Implicitly nullable parameter declarations deprecated

[DannyvdSluijs]

Thanks for the contribution @Ayesh. We are currently hard at work in reviving the repository and are currently focussing on cleaning up any old issues and pull request. After which we are going to work on merging PR's.

This PR would work and resolve issues for PHP 8.4 without a doubt. But this repository also supports older version of PHP (currently still even PHP 5.3) which don't support the ? (nullable type declaration). We are looking into supporting only more current versions of PHP but for Composer we would still need to support all the way back to PHP 7.2 (which is already EOL). You can check the comment Luckily the nullable type declaration was introduced in PHP 7.1 so we should be good once we can shed the older versions.

But still very much appreciated to put in the effort and bringing this to our attention.

[Ayesh]

Thank you for the quick response @DannyvdSluijs. I was pleasantly surprised to see this library supporting PHP versions that old.

I jumped to submit the fix when I saw the notices when running Composer, but you are right, it probably needs coordination to release a version with a higher minimum PHP version.

I'm not sure if closing this PR will help. If that's the case, I will happily submit a new PR against the new branches if we are about to release a PHP 7.1+ version.

Thank you.

[nicolas-grekas]

It would be nice to move quickly on this one so that the rest of us can enjoy a deprecation-free composer experience ASAP :pray:

[Seldaek]

Yeah this is a tricky one as it will require the php version bump, it is gonna have to be for v6, which does not exist yet, and there are quite a few open things before it could be tagged https://github.com/justinrainbow/json-schema/projects/2#card-11979917

But IMO this could be merged as soon as the php requirement is bumped, and perhaps an alpha version tagged at some point so we could use that in composer.

[mvorisek]

In theory, this PR can be implemented using traits for all supported PHP versions. 35 changes, so max. 35 traits :)

[Seldaek]

We already have problems finding enough time to maintain this lib, so let's please avoid suggesting ways to waste even more time :)

[stof]

This would require bumping the PHP requirement to at least PHP 7.1+, which can be made compatible with the composer needs of supporting PHP 7.2.

[nicolas-grekas]

What about releasing v5.3.0 with the bump to 7.1?

[erayd]

What about releasing v5.3.0 with the bump to 7.1?

Absolutely not. Increasing the PHP minimum version that far is a breaking change, and therefore necessitates a major version change for this library. We can't just drop it into the 5.x.x branch and not care about what we break.

It could go in the v6 branch though (although please see this comment in relation to that).

[stof]

If the increase is done without forgetting to update the composer metadata, it won't break BC for projects installing it through composer as composer takes the PHP requirement into account when resolving dependencies (unlike what npm does with the node compatibility info).

[Seldaek]

Yeah I am also a proponent of just bumping the php requirement in minor semver version bumps, which I think is completely fine as people can still rely on the old versions with older php versions, and it won't just update to the new one if they aren't on a modern enough php so this shouldn't break anything.

From what I understood tho @erayd wrote that support for 5.3+ is still desirable for master/v6. So I don't know, what I would personally do but that's just my 2c:

• Drop php <7.2 in v5, and tag a v5.3.0 with php8.4 support, 5.2.x can still be supported with backports for patches if really needed
• Tag a v6.0.0-alpha from master so anyone relying on php<7.2 support can still use that as it is now, and forever, but without receiving further patches because really sorry but what are you still doing on such an old php version 🙈
• Then also drop php <7.2 from master, continue cleanups and whatever is necessary to get a v6 stable out, that could take longer but at least master would have php8.4 support and 5.3.0 as well, so users all can have a version that fits whatever they need.

I think this could all be done without much work and I'm happy to even do it but I would need @erayd's blessing :)

[andypost]

it affects Drupal, hope it will be sorted out soon

$ composer why justinrainbow/json-schema
drupal/drupal     11.x-dev requires (for development) justinrainbow/json-schema (^5.2)    
composer/composer 2.7.4    requires                   justinrainbow/json-schema (^5.2.11) 

[Seldaek]

@erayd any opinion on my post above? https://github.com/justinrainbow/json-schema/pull/717#issuecomment-2017999973

[DannyvdSluijs]

See #726 for the ongoing discussions on this topic.

[Seldaek]

For anyone waiting for this, bump to 5.3.0 to fix this https://github.com/jsonrainbow/json-schema/releases/tag/5.3.0 for now while we wait for the master branch here to catch up :)

[Ayesh]

Thank you for working out the changes in 5.3 series. I just updated to latest Composer snapshot, and had zero warnings/notices on latest php-src build!

[DannyvdSluijs]

I've ported this PR in #746, this addresses the raising of the minimum PHP version as a whole including pipelines.