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

Fix deprecated messages in PHP 8.1 #31

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented Aug 29, 2023

Since the Collection class implements ArrayAccess interface, deprecated messages are thrown in PHP 8.1 when the returned type is not defined.

Example:

Return type of Iconic_WSB_NS\StellarWP\Uplink\Resources\Collection::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

This PR adds the returned types. It's important to notice that the void type was added in PHP 7.1.0. If we want to keep compatibility with older versions, we have to replace void with #[\ReturnTypeWillChange] to avoid the deprecated messages.

How to test this change:

  1. Update the composer.json
"repositories": [
...
{
      "type": "vcs",
      "url": "https://github.com/Rahmon/uplink.git"
    }
],
"require": {
...
"stellarwp/uplink": "dev-fix/collection-deprecated-warnings",
}
  1. Run composer update stellarwp/uplink

Copy link
Contributor

@jamesckemp jamesckemp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. My only concern is using Plugin @borkweb could it ever be anything different?

@borkweb
Copy link
Member

borkweb commented Aug 30, 2023

Hrm, yeah, it could be Plugin or Service as the return type. Sounds like we'll have to do #[\ReturnTypeWillChange]

@jamesckemp jamesckemp merged commit 322526f into stellarwp:main Aug 30, 2023
@Rahmon Rahmon deleted the fix/collection-deprecated-warnings branch September 4, 2023 13:34
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.

None yet

3 participants