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

Fixing phpdoc for Resource::call method return type #2113

Merged

Conversation

amackiewicz
Copy link
Contributor

@amackiewicz amackiewicz commented Aug 4, 2021

Found that issue by the way of using Google_Service_Drive->files->export.

Files::export calling Resource::call and returning its output. In normal circumstances it returns psr ResponseInterface, just in some cases (when request is defered) it returns RequestInterface.

Current @return in Resource::call method has 2 issues:

  1. Its solid class Response instead of ResponseInterface
  2. Its cheating IDE which dont know that File::export could return ResponseInterface which end up with code as following. Of course its triggers ERROR: TypeError: Return value of Foxstone\BackendPHP\service\GoogleDriveService::exportFileContents() must be an instance of GuzzleHttp\Psr7\Request, instance of GuzzleHttp\Psr7\Response returned

Example of generated code (type returns by exportFileContents method was inherited from types returns from File::export (and then from Resource::call) which is wrong):

    /**
     * @throws Exception
     */
    public function exportFileContents(
        string $fileId,
        string $exportMimetype
    ): ResponseInterface {
        $this->connect();

        return self::$GoogleServiceDrive->files->export(
            $fileId,
            $exportMimetype,
            ['alt' => 'media']
        );
    }

@amackiewicz amackiewicz requested a review from a team as a code owner August 4, 2021 03:40
@google-cla
Copy link

google-cla bot commented Aug 4, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Aug 4, 2021
@amackiewicz
Copy link
Contributor Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Aug 4, 2021
@bshaffer
Copy link
Contributor

bshaffer commented Aug 4, 2021

@amackiewicz This looks great, although I'm not sure when it would return array for these functions. Can you elaborate?

@amackiewicz
Copy link
Contributor Author

amackiewicz commented Aug 6, 2021

Hey @bshaffer ! well, me too :) but REST::execute phpdoc saying that it could return array (https://github.com/googleapis/google-api-php-client/blob/master/src/Http/REST.php#L44).
I've changed phpdoc also for that method, check it out plz

src/Service/Resource.php Outdated Show resolved Hide resolved
@amackiewicz
Copy link
Contributor Author

any updates here @bshaffer ? :)

@bshaffer bshaffer merged commit 3a98175 into googleapis:master Aug 27, 2021
@bshaffer bshaffer mentioned this pull request Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants