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

Generify collections using Psalm #61

Open
vv12131415 opened this issue Mar 1, 2020 · 5 comments
Open

Generify collections using Psalm #61

vv12131415 opened this issue Mar 1, 2020 · 5 comments
Labels

Comments

@vv12131415
Copy link

Since this collections lib is quite popular.
I would like to suggest it to use generics/template types like doctrine/collections does. This will make it more type safe to use this collections

@patrickkusebauch
Copy link

patrickkusebauch commented Apr 7, 2020

I am working on this right now in the form of a Psalm plugin. The goal is to make it play nicely with both PHPStan and Psalm. The issue right now is that PHPStan wants it in the form of Collection<TValue>, however at the moment Psalm only recognizes Collection and Collection<TValue> will not pass.

The idea of the plugin is to make Psalm accept 2 forms: Collection<TValue> and Collection<Tkey, TValue>. If done correctly, once ported from the plugin into the new version docs even PHPStan should recognize both forms.

TLDR; I am writing a Psalm plugin for this right now, so if you wait a day or two, it will be done. The plugin can be then ported into v 11.*.

@DusanKasan
Copy link
Owner

Sounds great! :) Take a look at the changes done in the v11, we will be changing all the returned Collection instances into CollectionInterface. This is done so that when you use the provided trait in your own class, you will get back an instance of your class instead of our generic Collection. I was wondering how to describe this in the Psalm/PHPStan land but came empty so far :/

@patrickkusebauch
Copy link

This is from straight from Psalm documentation:

<?php
/**
 * @template T
 */
trait MyTrait {}

class Foo {
    /**
     * @use MyTrait<Foo>
     */
    use MyTrait;
}

I am not sure how well it works for PHPStan, but for Psalm, if client code for the use of the trait would look like this, they in the trait you have access to the client class name.

For anyone interested in helping, https://github.com/Dance-Engineer/psalm-knapsack-collections is the plugin. Since I am a templating newbie, I would appreciate any help. The plugin is installable and works, but some of the templates are wrong and cause errors even when they should not. PRs are welcomed. One sorted, it can be ported to this REPO and only exists for people using <v11.

@vv12131415
Copy link
Author

PHPStan wants it in the form of Collection, however at the moment Psalm only recognizes Collection and Collection will not pass

What if you will all phpstan or psalm specific tags, like

@psalm-var 
@phpstan-var
@var

@DusanKasan
Copy link
Owner

DusanKasan commented Apr 17, 2020

I implemented Psalm annotations for the collection functions, CollectionTrait, CollectionInterface and the Collection in #63. There are still 10 "other issues" I can't really wrap my head around so any help is welcome :)

It's stuff like:

INFO: InvalidArgument - src/CollectionTrait.php:33:29 - Type TVal:DusanKasan\Knapsack\CollectionTrait as mixed should be a subtype of TVal:DusanKasan\Knapsack\Collection as mixed (see https://psalm.dev/004)
        return static::from(filter($this->getItems(), $function));

@DusanKasan DusanKasan added this to the 11.0.0 milestone Apr 19, 2020
@DusanKasan DusanKasan removed this from the 11.0.0 milestone Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants