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

Add PHPDoc return types #1230

Merged
merged 2 commits into from
Apr 8, 2022
Merged

Add PHPDoc return types #1230

merged 2 commits into from
Apr 8, 2022

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Jan 4, 2022

Symfony PHPUnit Bridge has a deprecation helper that warns when things might break in the future.

It currently reports the following errors when using Stripe PHP SDK:

Method "ArrayAccess::offsetExists()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetGet()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetSet()" might add "void" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetUnset()" might add "void" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "Countable::count()" might add "int" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "JsonSerializable::jsonSerialize()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.

By adding these PHPDoc types, the problem disappears. It's also more clear for users of the package.

Fixes #1210

@CLAassistant
Copy link

CLAassistant commented Jan 4, 2022

CLA assistant check
All committers have signed the CLA.

Symfony PHPUnit Bridge has a deprecation helper that warns when things might break in the future.

It currently reports the following errors when using Stripe PHP SDK:

```
Method "ArrayAccess::offsetExists()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetGet()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetSet()" might add "void" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "ArrayAccess::offsetUnset()" might add "void" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "Countable::count()" might add "int" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
Method "JsonSerializable::jsonSerialize()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Stripe\StripeObject" now to avoid errors or add an explicit @return annotation to suppress this message.
```

By adding these PHPDoc types, the problem disappears. It's also more clear for users of the package.

Fixes stripe#1210
@axi
Copy link

axi commented Jan 7, 2022

So I'll just wait for it to be merged, thanks

@ruudk
Copy link
Contributor Author

ruudk commented Jan 10, 2022

@yejia-stripe What do you think of this? Can it be merged?

@dcr-stripe
Copy link
Contributor

@richardm-stripe will review as they were looking at #1210.

@ruudk
Copy link
Contributor Author

ruudk commented Jan 25, 2022

Any news?

@jokaorgua
Copy link

@dcr-stripe do you have any news?

@henrikac
Copy link

henrikac commented Apr 7, 2022

Any updates on this?

@remi-stripe
Copy link
Contributor

Hey everyone, sorry for the radio silence on that one. I'll raise this internally and get you a clearer answer/update next week.

@richardm-stripe
Copy link
Contributor

Sorry for the silence, presently this PR cannot be merged as type errors are causing it to fail CI. I will spend some time to see if it can be repaired.

@richardm-stripe
Copy link
Contributor

Seems to have been an unrelated problem in the particular SHA of the master branch -- merging the latest has fixed the problem. Everything seems to be in order -- thanks for the contribution!

@richardm-stripe richardm-stripe merged commit b3bbe30 into stripe:master Apr 8, 2022
@ruudk ruudk deleted the symfony branch April 11, 2022 11:43
@ruudk
Copy link
Contributor Author

ruudk commented Apr 11, 2022

Thanks, would be great to have this tagged too 🙏

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.

Deprecation message when using with Symfony 5.4
8 participants