Support for range integers + drop support for PHP 7.2 & 7.3

[bnowak]

It adds support for some basic https://phpstan.org/writing-php-code/phpdoc-types#integer-ranges.

[bnowak]

@DjordyKoert That feature would work on any versions except the lowest ones (the single CI job with --prefer-lowest flag). I've checked that it works from the very lowest dependencies (comparing with the lowest installed by default):

SYMFONY_REQUIRE=5.4.* composer update \
phpdocumentor/reflection-docblock=4.3.4 \
symfony/property-info=5.4.10 \
phpstan/phpdoc-parser=1.13.0 \ # dependency of phpdocumentor/type-resolver=1.8.2 so not needed to specify in composer
phpdocumentor/type-resolver=1.8.2

but phpdocumentor/type-resolver=1.8.2 requires PHP 7.3 at least so my question is: could we bump dependencies and php to that version as minimum?

I know bumping PHP to next major version could be to too radical, so my alternative would be bumping only phpdocumentor/reflection-docblock & symfony/property-info versions (root dependencies) and make the feature (tests actually) PHP & phpdocumentor/type-resolver versions specific (that it will work from these the lowest versions).

Would do you think?

[DjordyKoert]

I wouldn't even mind bumping minimum php version to 7.4. Both 7.2 & 7.3 barely get any installs anymore see packagist stats.

[bnowak]

Cool 👍

Would you mean just only bumping it up in composer or also any other related adjustments in code (like getting rid of some backward-compatible workarounds etc.)?

[DjordyKoert]

Would you mean just only bumping it up in composer or also any other related adjustments in code (like getting rid of some backward-compatible workarounds etc.)?

I don't think we have any real backward-compatible workarounds, only in a few tests I think it looks like.

• https://github.com/search?q=repo%3Anelmio%2FNelmioApiDocBundle%207.2&type=code
• https://github.com/search?q=repo%3Anelmio%2FNelmioApiDocBundle%207.3&type=code

Feel free to remove this, otherwise I will remove them myself 😃

[bnowak]

👍 thanks for replying

I'm going to handle that tomorrow 😉

[DjordyKoert]

@DjordyKoert That feature would work on any versions except the lowest ones (the single CI job with --prefer-lowest flag). I've checked that it works from the very lowest dependencies (comparing with the lowest installed by default):

SYMFONY_REQUIRE=5.4.* composer update \
phpdocumentor/reflection-docblock=4.3.4 \
symfony/property-info=5.4.10 \
phpstan/phpdoc-parser=1.13.0 \ # dependency of phpdocumentor/type-resolver=1.8.2 so not needed to specify in composer
phpdocumentor/type-resolver=1.8.2

but phpdocumentor/type-resolver=1.8.2 required PHP 7.3 at least so my question is: could we bump dependencies and php to that version as minimum?

I think we should also bump symfony/property-info to ^5.4.10|^6.0|^7.0, that should ensure that the test ran on --prefer-lowest will pass

[bnowak]

@DjordyKoert in general PR is ready for review. Finally, I decided to bump only to PHP 7.3 (plus needed composer dependencies) which is enough for this feature.

However, I noticed failed pipelines (on SF >= 6.3) which are not related to my changes. It seems that the change which broke them was introduced in https://github.com/nelmio/NelmioApiDocBundle/actions/runs/8231364006/job/22506509816 but I'm not sure how it should be fixed.

[DjordyKoert]

However, I noticed failed pipelines (on SF >= 6.3) which are not related to my changes. It seems that the change which broke them was introduced in nelmio/NelmioApiDocBundle/actions/runs/8231364006/job/22506509816 but I'm not sure how it should be fixed.

I already have an issue open at swagger-php about this :) zircote/swagger-php#1556

[DjordyKoert]

@DjordyKoert in general PR is ready for review. Finally, I decided to bump only to PHP 7.3 (plus needed composer dependencies) which is enough for this feature.

LGTM, I would still like to drop support for PHP 7.3 in this and make PHP 7.4 the minimum supported version would you like & be able to do this? :)

Additionally an update inside the changelog.md would be appreciated.

[bnowak]

👍 will handle that

[bnowak]

@DjordyKoert adjusted to PHP 7.4 as minimum version, but I'm struggling with 2 errors in BazingaFunctionalTest (https://github.com/nelmio/NelmioApiDocBundle/actions/runs/8279847555/job/22655096369?pr=2236) which I cannot reproduce locally in docker (using PHP 7.4 image). It occurs only in --prefer-lowest job. I double checked that have all composer installed dependencies exactly the same as in CI job, but different results.

Do you have any idea what else I could check?

[DjordyKoert]

Do you have any idea what else I could check?

It looks like there is a caching issue. On my local machine the first test (testModelComplexDocumentationBazinga) passes whenever no cache is created yet. The tests after fail (because they use the cache created in testModelComplexDocumentationBazinga).

Rerunning the full BazingaFunctionalTest without deleting that /var directory (aka without removing cache) also causes testModelComplexDocumentationBazinga to fail.....

[DjordyKoert]

@bnowak 514239c fixed it 🥳

[bnowak]

cool, thanks for help with that :)

[bnowak]

@DjordyKoert thanks for approving ❤️

will zircote/swagger-php#1556 be a blocker for merging it or not?

[DjordyKoert]

will #2236 (comment) be a blocker for merging it or not?

Nope! 😄

[bnowak]

sorry, I meant swagger-php issue (comment updated) 😅

[DjordyKoert]

sorry, I meant swagger-php issue (comment updated) 😅

Still a nope hahaha. That is an issue on its own.

[DjordyKoert]

Thanks a lot @bnowak for this new feature + the other things that you were willing to pick up ❤️