Modernize code

[tcitworld]

• Require PHP ~7.4~ 7.3
• Add typing everywhere possible
• Small internal improvements

[staabm]

thanks for the contribution.

this PR is to big for me to review. I can not afford the time to review such big changes.

maybe other maintainers see it differently, but I cannot invest my time here.

[phil-davis]

First question - do we want to already drop support for those old PHP versions. PHP 7.3 only just went off the end of support. Maybe we should keep 7.3 support for another 6 months?

@staabm @DeepDiver1975

(I will push the button to run the CI, so we at least see the result)

[codecov[bot]]

Codecov Report

Merging #562 (c10f513) into master (06feff3) will increase coverage by 0.20%. The diff coverage is 98.74%.

@@             Coverage Diff              @@
##             master     #562      +/-   ##
============================================
+ Coverage     98.34%   98.55%   +0.20%     
+ Complexity     1884     1854      -30     
============================================
  Files            71       71              
  Lines          4536     4489      -47     
============================================
- Hits           4461     4424      -37     
+ Misses           75       65      -10     

Impacted Files	Coverage Δ
lib/ITip/Message.php	100.00% <ø> (ø)
lib/Property/Boolean.php	53.33% <80.00%> (+3.33%)	:arrow_up:
lib/Component.php	98.73% <93.75%> (-0.43%)	:arrow_down:
lib/Cli.php	97.74% <96.55%> (-0.33%)	:arrow_down:
lib/DateTimeParser.php	97.72% <97.43%> (-0.73%)	:arrow_down:
lib/Property.php	98.98% <97.67%> (-0.01%)	:arrow_down:
lib/BirthdayCalendarGenerator.php	98.18% <100.00%> (ø)
lib/Component/Available.php	100.00% <100.00%> (ø)
lib/Component/VAlarm.php	100.00% <100.00%> (ø)
lib/Component/VAvailability.php	100.00% <100.00%> (ø)
... and 71 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 06feff3...c10f513. Read the comment docs.

[phil-davis]

CI for the older versions is going to fail. @tcitworld you could remove the CI pipelines for 7.1 and 7.2 - assuming that we will be OK to drop those.

[tcitworld]

you could remove the CI pipelines for 7.1 and 7.2

Done.

Sorry for such a big commit but there's not many things that can be extracted in smaller ones. I can comment inline on changes that aren't just putting types, if that helps.

[phil-davis]

@tcitworld the code-style checker in CI suggests (insists!) on removing the "redundant" PHPdoc of types that are already declared inline in the code.

[staabm]

PHP 7.3 only just went off the end of support. Maybe we should keep 7.3 support for another 6 months?

fine with me

[phil-davis]

@tcitworld you should be able to locally do: composer cs-fixer

and that will make the code style edits that php-cs-fixer insists on having.

[tcitworld]

For some reason php-cs-fixer didn't report anything (running on php 8, might be the issue). Had to do the edits manually.

[phil-davis]

@tcitworld CI passes - that's great.

We want to still support PHP 7.3. For example, both ownCloud and NextCloud use the sabre libraries, and they both still support PHP 7.3 (for however many months). And probably there are other consumers of sabre that are similar. So we don't want to prevent those from being able to use releases.

Please add back the PHP 7.3 CI, adjust composer.json etc for PHP 7.3 and see what fails (if anything)

The PHP 7.4 new features are documented here: https://www.php.net/manual/en/migration74.new-features.php

I you have already used any of those, then that will need to be reverted (or this PR will get "stuck" for probably 6 months)

[tcitworld]

I removed the typed properties and restored type information as phpdoc. Will open another PR with php 7.4 specific features later.

[phil-davis]

Thanks - I will scroll through the diffs...

[phil-davis]

@tcitworld time has gone by and we had another discussion about this sort of stuff - https://github.com/sabre-io/xml/issues/215 We are going to drop PHP 7.3 and just keep support for PHP 7.4 and up. I did a PR like this in sabre/xml https://github.com/sabre-io/xml/pull/220 which has a much smaller code-base.

I will cherry-pick the code you have provided here into a branch of my own, rebase it and try to get back the PHP 7.4-only things.

Sorry for the pain and long delay, but thanks for the work you have done in the lots of detail needed for this.

[tcitworld]

Thanks! You may use this branch as well that I had kept.

[phil-davis]

Note: this will fix #584

[phil-davis]

Note: see #586 for the latest code for this. I took https://github.com/tcitworld/vobject/tree/modernize-php-7.4 and rebased it, then kept going myself. IMO that is ready to become a first RC1 for a major release 5.0.0

After that is sorted out and merged, we can close this PR, because the code here will have found its way into master.

[phil-davis]

PR #586 has been merged - thanks @tcitworld I will do a PR to increase the phpstan level, then we should be able to release a major version 5.