PHP ^7.4 Compatibility (Including 8.1)

[oojacoboo]

This PR includes support for PHP >= 7.4, resolving all type errors and deprecation warnings.

I should further add that PHP 7.3 is no longer receiving security updates - it's unmaintained. And, 7.4 will be unmaintained next year.

It appears evident that there is a lot of nullable behavior tied to business logic throughout the lib. It's quite unfortunate. I was able to strictly type and enforce in many places. However, in others, there were some business logic decisions that had to be made. And without greater knowledge of how some of these classes are instantiated and used, these decisions were typically permissive.

I’ve gone ahead and conformed with your code-sniffer rules @dereuromark. I will say though, I disagree with the requirement to include a docblock with PHP 8.1. Going forward, now that we have support to fully type a function, they are self documenting. If a function doesn’t require additional comments, for the function - itself, it’s arguments, return value, throws, etc - a docblock shouldn’t be required.

This PR actually took significantly more time than I expected. There were a lot of little things, especially to get all the tests passing.

The docs need updating to make clear only PHP 7.4 and greater is supported. I didn’t see them in the repo. It’d be nice to have the docs in the repo with a static site generator. I’ve used docusaurus on other OSS libs.

Closes #1795

[dereuromark]

Isn't it a bit too early for this? Propel was always a bit more on the conservative side of PHP dependency to allow also older projects to still be compatible with latest release. So at this time, we are PHP7.3+ but we could discuss to at least keep the latest minor of 7.x, so PHP7.4+ maybe. One more release at least here, and in a few months we could still discuss 8.0+ then?

[oojacoboo]

Isn't it a bit too early for this? Propel was always a bit more on the conservative side of PHP dependency to allow also older projects to still be compatible with latest release. So at this time, we are PHP7.3+ but we could discuss to at least keep the latest minor of 7.x, so PHP7.4+ maybe. One more release at least here, and in a few months we could still discuss 8.0+ then?

@dereuromark I have no problem with continuing to target ^7.4. Do you have suggestions on how that's going to be achieved though? I outlined the issue in the original comment:

Firstly, the mixed type isn’t supported in versions prior to 8.0, and 8.1 has strictly typed interfaces that require the mixed return type. We’d have to implement a custom namespaced shimmed interface instead of the core interfaces, update any logic that is typed or checking for instances of this interface, and then hope there aren’t any other side-effects in user-land from reliance on those interfaces.

[dereuromark]

We will not be able to add any such types, thats fine though. We can do that once we cross that bridge, it should be still before declaring a stable version so that we can actually add this in semver compliance.

[oojacoboo]

@dereuromark here is an example. Maybe I'm missing something here.

https://www.php.net/manual/en/class.iterator.php

As you'll notice, this interface for PHP 8.1 requires a mixed return type. In order for classes that implement this interface to be compatible, they must also implement this return type. However, the mixed return type wasn't introduced until 8.0.

[dereuromark]

Yes, but since we do 7.4+, you can just omit impossible types. This will still be compatible with 8, if we use attributes. Right?

See also this regarding polyfill usage if we need it.

[oojacoboo]

@dereuromark when you say use attributes, are you suggesting an attribute or an annotation? I haven't used attributes yet, as we're just upgrading to PHP 8, ourselves. I'm not aware of how you'd use an attribute to declare a return typing for a method. Do you have any info on that?

As for an annotation, no - that won't work b/c PHP doesn't parse annotations, they're just comments.

[dereuromark]

See this Only that it should be using FQCN, so #[\ReturnTypeWillChange]

[oojacoboo]

@dereuromark I see - thanks. I'll give it a go, targeting ^7.4 for this PR. Is there anything else preventing this from being merged in?

[codecov-commenter]

Codecov Report

Merging #1805 (3663163) into master (a1a69bd) will decrease coverage by 0.00%. The diff coverage is 86.02%.

@@             Coverage Diff              @@
##             master    #1805      +/-   ##
============================================
- Coverage     87.84%   87.84%   -0.01%     
- Complexity     7816     7836      +20     
============================================
  Files           281      281              
  Lines         22715    22747      +32     
============================================
+ Hits          19955    19981      +26     
- Misses         2760     2766       +6     

Flag	Coverage Δ
5-max	87.84% <86.02%> (-0.01%)	:arrow_down:
7.4	87.84% <86.02%> (-0.01%)	:arrow_down:
agnostic	68.54% <64.70%> (+<0.01%)	:arrow_up:
mysql	70.54% <71.32%> (-0.02%)	:arrow_down:
pgsql	70.42% <71.32%> (-0.02%)	:arrow_down:
sqlite	68.46% <72.05%> (-0.02%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Propel/Common/Config/Loader/IniFileLoader.php	94.23% <ø> (ø)
src/Propel/Runtime/Parser/CsvParser.php	88.57% <ø> (ø)
...c/Propel/Runtime/Collection/CollectionIterator.php	52.50% <31.81%> (-4.26%)	:arrow_down:
...untime/ActiveRecord/NestedSetRecursiveIterator.php	56.25% <60.00%> (ø)
src/Propel/Generator/Model/PhpNameGenerator.php	74.46% <85.71%> (+1.13%)	:arrow_up:
src/Propel/Runtime/Collection/Collection.php	94.77% <90.00%> (-0.50%)	:arrow_down:
src/Propel/Common/Config/XmlToArrayConverter.php	93.10% <100.00%> (ø)
src/Propel/Generator/Builder/DataModelBuilder.php	89.21% <100.00%> (+0.75%)	:arrow_up:
.../Propel/Generator/Builder/Om/AbstractOMBuilder.php	96.75% <100.00%> (+0.01%)	:arrow_up:
src/Propel/Generator/Builder/Om/ClassTools.php	59.09% <100.00%> (+4.09%)	:arrow_up:
... and 27 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a1a69bd...3663163. Read the comment docs.

[oojacoboo]

@dereuromark Let me know if anything else is needed.

[dereuromark]

@mringler What do you think? I think the changes look good so far and show a general improvement towards the type safe future we would like to see. A good first step - of many to come.

[dereuromark]

Any second review please? So we can move forward?