Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1: fix deprecation notice #919

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Oct 28, 2021

As of PHP 8.1, PHP will start throwing deprecation warnings along the lines of:

Deprecated:  Return type of [CLASS]::[METHOD]() should either be compatible with [PHP NATIVE INTERFACE]::[METHOD](): [TYPE], or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

These type of deprecation notices relate to the Return types for internal methods RFC in PHP 8.1.

Basically, as of PHP 8.1, these methods in classes which implement PHP native interfaces are expected to have a return type declared.
The return type can be the same as used in PHP itself or a more specific type. This complies with the Liskov principle of covariance, which allows the return type of a child overloaded method to be more specific than that of the parent.

The return type for JsonSerializable::jsonSerialize() is supposed to be mixed, but as that is a PHP 8.0 type, it cannot yet be added.
Ref: https://www.php.net/manual/en/class.jsonserializable.php

With that in mind, I've added the annotation instead.

Note: While attributes are a PHP 8.0 feature only, due to the syntax choice for #[], they will ignored in PHP < 8 and can be safely added.

As of PHP 8.1, PHP will start throwing deprecation warnings along the lines of:
```
Deprecated:  Return type of [CLASS]::[METHOD]() should either be compatible with [PHP NATIVE INTERFACE]::[METHOD](): [TYPE], or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
```

These type of deprecation notices relate to the [Return types for internal methods RFC](https://wiki.php.net/rfc/internal_method_return_types) in PHP 8.1.

Basically, as of PHP 8.1, these methods in classes which implement PHP native interfaces are expected to have a return type declared.
The return type can be the same as used in PHP itself or a more specific type. This complies with the Liskov principle of covariance, which allows the return type of a child overloaded method to be more specific than that of the parent.

The return type for `JsonSerializable::jsonSerialize()` is supposed to be `mixed`, but as that is a PHP 8.0 type, it cannot yet be added.
Ref: https://www.php.net/manual/en/class.jsonserializable.php

With that in mind, I've added the annotation instead.

Note: While attributes are a PHP 8.0 feature only, due to the syntax choice for `#[]`, they will ignored in PHP < 8 and can be safely added.
@jrfnl jrfnl mentioned this pull request Oct 28, 2021
moorscode added a commit to Yoast/wordpress-seo that referenced this pull request Oct 28, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Oct 28, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 9, 2021

👋 Hiya all! Anything I can do to move this (and my other PRs) forward ? All relatively small changes and atomic PRs, so would be great if they could get some traction....

@ramsey
Copy link
Contributor

ramsey commented Nov 9, 2021

I'll try to take a look and get them merged this week, @jrfnl. Thanks so much for contributing!

@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #919 (47bba07) into master (d0f43ef) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##              master      #919   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       180       180           
===========================================
  Files             20        20           
  Lines            464       464           
===========================================
  Hits             464       464           

@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 10, 2021

Thanks @ramsey, appreciated!

jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Nov 11, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
@Matts Matts mentioned this pull request Nov 25, 2021
@ruudk
Copy link

ruudk commented Nov 29, 2021

Is there anything I can help with to get this PR merged?

@ramsey ramsey merged commit 9d1eb91 into thephpleague:master Nov 29, 2021
@jrfnl jrfnl deleted the feature/php-8.1-fix-missing-return-type branch November 30, 2021 08:00
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Nov 30, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Nov 30, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 6, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 9, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
@Bilge
Copy link

Bilge commented Dec 9, 2021

Can we get a tag for this? Otherwise we cannot upgrade to PHP 8.1...

jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 16, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 16, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
@MyIgel
Copy link

MyIgel commented Dec 19, 2021

Hi @ramsey, is there a plan for the next bugfix release which could contain that change?

Imho it should be planned soon as its currently a blocker for many people for using PHP 8.1. WHats your opinion / the release schedule of this package on that?

jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 20, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implenetation of the JSONSerializableInterface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and was not worked as desired on the newer version. Bumping the version in this commit seemed out of scope.

This is addressed in the following ticket: humbug/php-scoper#539
jrfnl pushed a commit to Yoast/wordpress-seo that referenced this pull request Dec 23, 2021
PHP Scoper is prefixing the attribute use statement that will need to be pulled into the TheLeague OAuth Client implementation of the `JSONSerializable` interface.
See thephpleague/oauth2-client#919

This change removes the prefix from the use statement.

PHP Scoper was updated to 0.15.0 during investigation of this issue and it still does not work as desired on the newer version. Bumping the version in this commit seemed out of scope.

PHP 8.1 compatibility for PHP Scoper is being tracked in the following ticket: humbug/php-scoper#539
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants