Bump PHP dep from 7.2 to 7.4 [v5]

[DerManoMann]

Time to move on!?

Useful features:

• property typehints
• arrow functions

Other things to consider:

• bump symfony/finder to ^5.0
• bump symfony/yaml to ^5.0
• bump friendsofphp/php-cs-fixer (dev/suggest) to ^3.48
• bump doctrine/annotations (dev/suggest) to ^2.0
• bump phpunit/phpunit (dev) to ^9.0

Bump swagger-php version to 5

Add disclaimer about annotations being deprecated -> https://github.com/doctrine/annotations/pull/486

Remove all code marked deprecated

Add rector to manage refactoring and code upgrades.

Re-implement TokenScanner using nikic/php-parser

Plan is to move forward to v5 reasonably soon after 8.4 is out.

[DjordyKoert]

May I ask why this package will not move to php 8 (or php 8.1 because php 8.0 is already EOL).

I got some valid feedback on https://github.com/nelmio/NelmioApiDocBundle/pull/2171 which is why I am suggesting this. I am also currently considering moving NelmioApiDocBundle to php 8.1 https://github.com/nelmio/NelmioApiDocBundle/pull/2171#discussion_r1478458499

[DerManoMann]

I think it is just a conservative approach. I do understand some of the reasons for doing it, but without a solid technical reason it still feels a bit artificial. But then, its a step in the right direction and the issue was created for this type of discussion :)

[DjordyKoert]

Union types (on top of typed property from PHP 7.4)

A reason for moving to PHP 8 would be because of the support for union types. Currently a lot of annotations have a @varcomment with a bunch of different possible types. https://github.com/zircote/swagger-php/blob/bdee7f5a9216ce103ba2c953c1c43c4a3e139e4c/src/Annotations/Response.php#L69

Currently you have written your own "type parser" (correct me if I'm wrong), this could instead be handled by PHP itself (except for arrays, php has no way to handle generics :)). https://github.com/zircote/swagger-php/blob/bdee7f5a9216ce103ba2c953c1c43c4a3e139e4c/src/Annotations/AbstractAnnotation.php#L680

This also comes with a con though at this current time, because of the Generator::UNDEFINED constant which is currently used everywhere as a default. This would require every property to also be typed with the string type which could cause confusion for users of this package who might think any kind of string can be used as a value for that property.

This could be fixed by either implementing a class (new Undefined()) or some kind of Type enum (which could maybe even include normal data types https://swagger.io/docs/specification/data-models/data-types/#any?) which replaces this.

Constructor property promotion

This is one of my personal favorites simply because it remove a lot of boilerplate code from classes.

Maybe this could also help with refactoring the annotations to no longer pass an array of properties and instead use this in combination with named parameters to cleanup the annotations. (This is not for me to decide of course, but I personally was confused about how these annotations worked when I first looked at the codebase)

Attributes

Annotations could of course also be fully dropped because PHP 8 now includes Attributes which can fully replace these. Additionally it looks like doctrine/annotations will be flagged as abandoned some time in the future. If/when this happens then this package can simply remove the dev dependency, suggestion & internal code which mention this package without having to bump the php version (and presumably a new major version) again.

These are just some of my thoughts about it hahaha 😄

[DerManoMann]

Fair points.

Considering that a quarter of downloads are still for v3 it feels still a bit early for such a big step. OTOH, making big steps will force others to follow suit.

I am not worried about another major version if - I suspect there will be at least one more in the future for an 'attibutes only' version at some point, so :shrug:

[afilina]

I don't mean to hijack the conversation, but I simply had to thank @DjordyKoert for taking the time to research his arguments so thoroughly. This is food for thought even in my own projects, since I deal with legacy code all the time.

[DerManoMann]

Not at all. I agree that there are some good points in general but each project is different. I still believe for swagger-php a slower approach is more appropriate, although I am not looking forward to dealing with updating two branches, at least for a bit.

[fulldecent]

Bumping deps is a breaking changes. Perhaps we will follow semver and do a deprecation notice now (i.e. in the current major version). And then later at any time we can actually make the new major version and jettison the old parts.

Here is a PR for the first step that is actionable now https://github.com/zircote/swagger-php/pull/1651