ClickSend / clicksend-php

A wrapper for our REST API - SMS, voice, post, email, fax.
27 stars 14 forks source link

Deprecation Notices raised in PHP 8.1 due to ArrayAccess changes #26

Open michaelbutler opened 2 years ago

michaelbutler commented 2 years ago

PHP 8.1 introduced typehints (or maybe PHP 8.0?) on some ArrayAccess interface methods, such as offsetExists. This means that (unfortunately) if your code base implements ArrayAccess, but omits the return typehints, you'll get Deprecation notices such as this:

Deprecated: Return type of ClickSend\Model\Email::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 /app/vendor/clicksend/clicksend-php/lib/Model/Email.php on line 461

See https://github.com/ClickSend/clicksend-php/blob/master/lib/Model/Email.php#L461

Here is an example PHP library which fixed this: https://github.com/patrickschur/language-detection/commit/bca4cf7bb4ed616009f9183317e6a6df4c8c668a

As a workaround, I can wrap my calls to this library to squelch these messages.

michaelbutler commented 2 years ago

I'm not sure how your swagger build process works but my recommendation to keep things simple is just just add the #[\ReturnTypeWillChange] attribute to the generated code where applicable, as explained here: https://php.watch/versions/8.1/ReturnTypeWillChange

What's nice about that solution is that older PHP versions will interpret # as a code comment, so it will work still.

michaelbutler commented 2 years ago

As mentioned in PR https://github.com/ClickSend/clicksend-php/pull/27, looks like swagger needs to fix this first, since this repo is just autogenerated code mostly.

kylekatarnls commented 2 years ago

👋 Where is the code generator script? It's very likely that extra steps can be added after the script to fix compatibility.

mriecken commented 2 years ago

Thank you for your contributions and patience. I want to assure you all they the dev team is aware of the problem and we are looking at implementing the changes you have suggested.

We have a PR that does a temporary fix, and we are looking at modifying the code generation system, and were hoping for a solution that would work from OpenAPI.

In the short term, we will be applying the PR and working out how to generate code that includes the appropriate fixes, most likely with a post-code-generation patch.

Thanks for your kind support and patience. Y'all rock.

Michael

WelshDev commented 1 year ago

This is causing some trouble for me also. A proper fix would be very helpful. Thanks

WelshDev commented 1 year ago

@mriecken is there any update on getting this patched?

BafS commented 1 year ago

Ping @mriecken, it's been more than a year now, could you please help us fixing this commercial library?

HarryWiles commented 10 months ago

We've waited so long and still no fix - sadly I think we'll have to take our $7000/m of sms to another provider 😭

sudo-plz commented 10 months ago

I've requested support through their form, hopefully this will be noticed.

BafS commented 10 months ago

I tried that multiple times and they never went back to me.

The good new is that Symfony supports it officially with the Notifier component (https://github.com/symfony/symfony/tree/6.4/src/Symfony/Component/Notifier/Bridge/ClickSend)