Skip to content

Conversation

andrewbess
Copy link

@andrewbess andrewbess commented Nov 8, 2021

Hello @tomzx

We still have problems with class \PHPSemVerChecker\Report\Report.
The problem looks like showed below when I run phpunit

During inheritance of ArrayAccess: Uncaught PHPUnit\Framework\Exception: Deprecated: Return type of PHPSemVerChecker\Report\Report::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/andrewbess/work/projects/my-project/vendor/tomzx/php-semver-checker/src/PHPSemVerChecker/Report/Report.php:161

We should add attribute #[\ReturnTypeWillChange] above methods offsetExists, offsetGet, offsetSet, offsetUnset, getIterator or we should add return types (that is not BIC) because the return types have been added there in PHP >= 8.0.

This pull request provides the fixes for the class \PHPSemVerChecker\Report\Report according to php.net ArrayAccess interface and php.net IteratorAggregate interface. Also, we fixed error reporting in src/PHPSemVerChecker/Console/Application.php.
Finally, we checked these changes in PHP 7.4, 8.0, 8.1. It works well.

Cloud you please check it?

Thank you in advance.

@tomzx
Copy link
Owner

tomzx commented Nov 9, 2021

Hello @andrewbess, just to confirm, this is not a blocking issue? I'm asking because the unit tests did not point to an issue and if there is, I'd like to add a test to prevent further errors.

@andrewbess
Copy link
Author

Hello @tomzx
Thank you for the fast reaction here.
This is priority issue for me, we cannot release our project.
However this is not blocker.

I will try to add test here.

Thank you in advance.

@andrewbess
Copy link
Author

andrewbess commented Nov 9, 2021

Hello @tomzx
I checked your tests with PHP 8.1 and we have the next results without fixes from this PR
wrong-return-types

The results of the tests with fixes provided below
tests-with-fixes

So, this library has the necessary tests.

@andrewbess andrewbess force-pushed the php-8.1-compatibility branch from ef9bee5 to d31a3d6 Compare November 22, 2021 13:40
@andrewbess
Copy link
Author

Hello @tomzx
We've fixed our pull request.
The latest changes provide compatibility with PHP 8.1, while at the same time they provide backward compatibility for 3-rd party extensions.
The using the attribute #[\ReturnTypeWillChange] provides this possibility.
Also, could you please process this PR? The problem is critical for us.
Thank you in advance.

@andrewbess
Copy link
Author

Hello @tomzx
The PHP 8.1 has been released.
Making a compatibility of our project with PHP 8.1 relates from your extension.
Could you please share your plans with releasing new version of your extension with us?
Thank you in advance.

@xmav
Copy link

xmav commented Dec 4, 2021

Hi @tomzx !
Could you please assist with delivery of this fix ?

@@ -25,7 +25,7 @@ public function __construct()
parent::__construct('PHP Semantic Versioning Checker by Tom Rochette', self::VERSION);

// Suppress deprecated warnings
error_reporting(E_ALL ^E_DEPRECATED);
error_reporting(E_ALL & ~E_DEPRECATED);
Copy link

Choose a reason for hiding this comment

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

Hi @tomzx
Please note that while this suppress E_DEPRECATED in "standalone" application there are still deprecation messages generated if this module is used as dependency. Would be great if this PR could be merged and new version released so that issue resolved everywhere

Copy link
Owner

Choose a reason for hiding this comment

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

Hello, I've removed the error silencing in v0.15.0. Let me know if this fixed your issue.

@tomzx
Copy link
Owner

tomzx commented Dec 31, 2021

@andrewbess Hello @andrewbess, I've updated the code with changes that are mostly similar to what you suggested in your PR (I've opted to use void and bool where appropriate instead of #[\ReturnTypeWillChange] and only used it for ``mixed` which becomes available in PHP 8.0.

Let me know if you're still facing issue (either by reopening this issue or creating a new one).

@tomzx tomzx closed this Dec 31, 2021
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.

3 participants