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

Static analysis error with psalm due to missing @template tag #6198

Open
plsoucy-tapclicks opened this issue May 9, 2023 · 3 comments
Open
Assignees
Labels
lang: php Issues specific to PHP. priority: p3 Desirable enhancement or fix. May not be included in next release.

Comments

@plsoucy-tapclicks
Copy link

Note: this is not a blocker as we can psalm-suppress the error and the code actually works fine, but I'm reporting this here since it seems like an easy fix and others might encounter the same issue.

Environment details

  • OS: Any
  • PHP version: 8.1
  • Package name and version: google/cloud-storage 1.30.2

Steps to reproduce

  1. Run psalm static analysis on code that calls $bucket->objects
    $objects = $bucket->objects(['prefix' => $gcs_path, 'fields' => 'items,nextPageToken']);

Details: vimeo/psalm#9744

Code example

use Google\Cloud\Storage\StorageClient;

$key_file = 'foo';
$gcs_path = 'bar';
$storage_client = new StorageClient(['projectId' => 1234, 'keyFilePath' => $key_file]);

$bucket = $storage_client->bucket('bucket-name');
$objects = $bucket->objects(['prefix' => $gcs_path, 'fields' => 'items,nextPageToken']);
print !empty($objects)."\n";

Error as reported by psalm on that code:

ERROR: TooManyTemplateParams - src/test.php:10:12 - Google\Cloud\Storage\ObjectIterator<Google\Cloud\Storage\StorageObject> has too many template params, expecting 0 (see https://psalm.dev/184)
$objects = $bucket->objects(['prefix' => $gcs_path, 'fields' => 'items,nextPageToken']);

Per vimeo/psalm#9744, this can be fixed by adding an @template annotation to the src/ObjectIterator.php class (I have verified this)

/**
 * Iterates over a set of {@see Google\Cloud\Storage\StorageObject} items.
 *
 * @template T
*/
class ObjectIterator implements \Iterator
@orklah
Copy link

orklah commented May 9, 2023

Well, more precisely, Iterator has 2 templates. When it is implemented, a @template-implements is expected. It should look like

@template-implements \Iterator<TKey, TValue> where TKey and TValue are either template themselves or the type of the key or the value if it's fixed.

Here, ObjectIterator seems to use int as an offset (because key() in the trait return int), so TKey should be replaced by int

So you would have something like

/**
 * @template TValue
 *
 * @template-implements \Iterator<int, TValue>
 */

That way, users of ObjectIterator will still need to fill the TValue type for their own instantation/implementations

I don't know the library enough but based on the name, it could even be

/**
 * @template TValue of object
 *
 * @template-implements \Iterator<int, TValue>
 */

to restrain the possible type of the template to objects only

@ajupazhamayil ajupazhamayil added priority: p3 Desirable enhancement or fix. May not be included in next release. lang: php Issues specific to PHP. labels May 10, 2023
@ajupazhamayil
Copy link
Contributor

I see the @template is nowhere used in google-cloud-php. We may need to change everywhere if we need to resolve this issue to have uniformity.

@orklah
Copy link

orklah commented May 10, 2023

it's used here: https://github.com/googleapis/google-cloud-php/blob/main/Storage/src/Bucket.php#L651, not sure if it's used in other places

@saranshdhingra saranshdhingra self-assigned this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: php Issues specific to PHP. priority: p3 Desirable enhancement or fix. May not be included in next release.
Projects
None yet
Development

No branches or pull requests

4 participants