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 PHP 8.1 incompatibility errors #10

Closed
wants to merge 2 commits into from
Closed

Conversation

szeber
Copy link

@szeber szeber commented Jan 16, 2022

In PHP 8.1 a new deprecation error is triggered because the getIterator() and getCount() methods don't conform to the built in Traversable and Countable interfaces.

For example: PHP Deprecated: Return type of WMDE\PsrLogTestDoubles\LogCalls::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice and PHP Deprecated: Return type of WMDE\PsrLogTestDoubles\LogCalls::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

The 2 ways to fix this is either add an attribute is either to add the return typehint or to add the attribute. Since on PHP 7.1 these methods didn't have a return typehint, and covariance/contravariance is only fully supported in 7.4, adding the attribute should be more backwards compatible because they will just be treated as comments versions before 8.0.

@JeroenDeDauw
Copy link
Contributor

Thank you @szeber! I have added the return types. Temporarily in a different repo and composer package: https://github.com/JeroenDeDauw/PsrLogTestDoubles. Or perhaps not. Depends on if WMDE is willing to move over this repo. Either way I will continue maintaining the new version.

@gbirke
Copy link
Member

gbirke commented May 2, 2022

We now have a new 3.2 release that's 8.x compatible and will continue to support this repo

@gbirke gbirke closed this May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants