search

TheNetworg / oauth2-azure

Azure AD provider for the OAuth 2.0 Client.
https://packagist.org/packages/thenetworg/oauth2-azure
MIT License
230 stars 108 forks source link

Deprecations with PHP 8.1 #154

Closed spackmat closed 2 years ago

spackmat commented 2 years ago

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 2 years ago

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?

spackmat commented 2 years ago

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.

curry684 commented 2 years ago

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 2 years ago

Some activity on the topic :)

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

hajekj commented 2 years ago

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

pau1mck commented 2 years ago

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

hajekj commented 2 years ago

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