TheNetworg/oauth2-azure

Deprecations with PHP 8.1

spackmat opened this issue · 7 comments

spackmat commented

Hi,

I switched an app using this package to PHP 8.1 and that still works well, but raises some deprecation warnings about return types of the methods from League\OAuth2\Client\Provider\AbstractProvider that are implemented in TheNetworg\OAuth2\Client\Provider\Azure:

Method "League\OAuth2\Client\Provider\AbstractProvider::getBaseAuthorizationUrl()" might add "string" as a native return type declaration in the future. Do the same in child class "TheNetworg\OAuth2\Client\Provider\Azure" now to avoid errors or add an explicit @return annotation to suppress this message.

And so on for the other methods. In thephpleague/oauth2-client this PR https://github.com/thephpleague/oauth2-client/pull/919/files removes at least the message as fixing the code would break BC with PHP 5.6 that is still supported. This could be done here, too. I could submit that as a PR, if that helps. (But maybe it could be a better idea to ditch PHP 5.6 and move forward as it is EOL for nearly three years now.)

hajekj commented

I agree that ditching 5.6 is probably most reasonable to do, also as per telemetry: https://packagist.org/packages/thenetworg/oauth2-azure/php-stats (which doesn't contain all the data, especially about actual usage). This would mean that we need to update everything to include types?

  • 👍1
spackmat commented

I created a PR for that. Only missing return types of methods that override inherited methods raise that deprecation errors. I bumped the PHP versions to ^7.1|^8.0 to allow the explicit return types and added inheritdoc annotations to those overriding methods to mark them accordingly.

Edit: If you decide to keep the old PHP versions, that explicit return types must be changed to PHPDoc annotations, that also suppresses the deprecation notices.

  • 👍1
curry684 commented

which doesn't contain all the data, especially about actual usage

This is not interesting as what doesn't show in the Packagist stats will remain locked to the old versions.

spackmat commented

Some activity on the topic :)

My PR #155 ist still open since December, some thoughts about that?

hajekj commented

Apologies, I just merged it. Will do a release when I review all the changes tomorrow! Sorry for the late response!

  • 👍2
pau1mck commented

Hi there, do we have any thoughts on when the release will happen? Thanks

hajekj commented

I have just released v2.1.0 which should contain all the changes made: https://github.com/TheNetworg/oauth2-azure/releases/tag/v2.1.0

  • 👍2