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 return types #85

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

add return types #85

wants to merge 3 commits into from

Conversation

tacman
Copy link
Contributor

@tacman tacman commented Dec 17, 2024

This seems to pass some static tests, but I'm not sure how to set the API key to do all the unit tests.

It just gets the return type from the annotations and uses that.

@tacman
Copy link
Contributor Author

tacman commented Dec 17, 2024

@samwilson can you take a look at these issues? I can probably fix them, but perhaps they can be slightly refactored to return null instead of false.

Also, is oAuth1 still supported? How should it be handled?

src/PhpFlickr.php:338 PhanTypeMismatchReturnSuperType Returning $accessToken of type \OAuth\Common\Token\TokenInterface but retrieveAccessToken() is declared to return \OAuth\OAuth1\Token\TokenInterface|\OAuth\OAuth2\Token\TokenInterface|string (saw a supertype instead of a subtype)
src/TestApi.php:16 PhanTypeMismatchDeclaredReturnNullable Doc-block of testEcho has declared return type bool which is not a permitted replacement of the nullable return type array|bool|null declared in the signature ('?T' should be documented as 'T|null' or '?T')
src/TestApi.php:16 PhanTypeMismatchDeclaredReturnNullable Doc-block of testEcho has declared return type string[] which is not a permitted replacement of the nullable return type array|bool|null declared in the signature ('?T' should be documented as 'T|null' or '?T')
[debug] Restarted process exited 1
Script phan --allow-polyfill-parser --load-baseline .phan/baseline.php handling the phan event returned with error code 1
Script @phan was called via test

@samwilson
Copy link
Owner

OAuth 1 is the only version that's supported, I think?

This PR doesn't have any changes, by the way.

I'll start working through the Phan issues to improve things. e.g. #86.

@samwilson
Copy link
Owner

But yes, I think in general we can switch returns from string[]|bool to ?string[]. Will that be another major version bump? :-) (I think it will but that's fine.)

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.

2 participants