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

Google_Http_MediaFileUpload PHPDoc issues? #1881

Open
ThibaultVlacich opened this issue Jul 16, 2020 · 6 comments
Open

Google_Http_MediaFileUpload PHPDoc issues? #1881

ThibaultVlacich opened this issue Jul 16, 2020 · 6 comments
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@ThibaultVlacich
Copy link
Contributor

ThibaultVlacich commented Jul 16, 2020

I've been using Google_Http_MediaFileUpload to upload videos to YouTube. I have no issue with the actual functionalities of the code, only with the type hinting that seems to be incorrect for a lot of the constructor / methods parameters.

Code example

The following code is mostly taken from the documentation.

$chunkSizeBytes = 10 * 1024 * 1024; // 10 MB
$fileSize = filesize($videoFile->getRealPath());

$this->googleClient->setDefer(true);

$insertRequest = $ytService->videos->insert(
    'snippet,status',
    $ytVideo
);

$media = new \Google_Http_MediaFileUpload($this->googleClient, $insertRequest, 'video/*', null, true, $chunkSizeBytes);
$media->setFileSize($fileSize);

This works perfectly fine, but triggers a lot of linting warnings from, for example, PHPStan or Intelephense in VSCode, because of the PHPDoc block of Google_Http_MediaFileUpload constructor.

Here's the warnings:

Parameter #2 $request of class Google_Http_MediaFileUpload constructor expects 'Psr\Http\Message\RequestInterface', 'Google_Service_YouTube_Video' given.
Parameter #4 $data of class Google_Http_MediaFileUpload constructor expects string, null given.
Parameter #6 $chunkSize of class Google_Http_MediaFileUpload constructor expects bool, int given.

For the first, I'm not really sure what's the issue here. Using the result of $ytService->videos->insert is what the doc says to do, but the returned type is not compatible at all (but it works when it actually runs, so it's correct...)

Is it intentional that $chunkSize is defined as a bool (seems weird to me, but maybe there's a reason)? $data should also specify |null.

@ThibaultVlacich
Copy link
Contributor Author

ThibaultVlacich commented Jul 17, 2020

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Jul 17, 2020
@dwsupplee dwsupplee added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Jul 22, 2020
@xhezairbey
Copy link

I see It's been two years since this was reported, and I can confirm it's still an issue even with v2.12.6.

In a similar example, the ops report holds true both for $request (RequestInterface) and $chunkSize (bool)

$request = $this->service->videos->insert('snippet,status', $video, []); when $request passed onto the Google_Http_MediaFileUpload::construct() raises warning about

Expected parameter of type '\Psr\Http\Message\RequestInterface', '\Google\Service\YouTube\Video' provided

I'm about to find out wether passing the $insertRequest breaks the internal functionality for MediaFileUpload::process() since that method call is dependant on receiving the proper $request object.

@wayonhigh
Copy link

I have just come across this issue in version [2.15.3] (https://github.com/googleapis/google-api-php-client/releases/tag/v2.15.3)

Is there anyway to fix it to stop it showing as a 'Problem' in VSCode?

VObvBITBPf

@tinusg
Copy link

tinusg commented May 13, 2024

@wayonhigh Yes there is. I just found out:
Add this line before 1293:

/** @var \Psr\Http\Message\RequestInterface $insertRequest */

It's a PHPDoc comment that clarifies the actual variable type.

@wayonhigh
Copy link

wayonhigh commented May 13, 2024

@tinusg thank you! Actually, since I posted that question, I moved from Intelephense to Devsense Intelliphp and it doesn't show as a "problem"

@tinusg
Copy link

tinusg commented May 13, 2024

I'll check that out as well, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants