Generics annotations support

[tanerkay]

Summary

Add option use_generics_annotations for collection type hints

Before:

 * @property-read \Illuminate\Database\Eloquent\Collection|Simple[] $regularHasMany

After:

 * @property-read \Illuminate\Database\Eloquent\Collection<Simple> $regularHasMany

I feel this is is a useful transition given what has evolved since https://github.com/barryvdh/laravel-ide-helper/issues/942#issuecomment-633788705. Namely:

• the upcoming release of Laravel 9 this month
• most PhpStorm users should be on version >= 2021.3 (addition of generics support and support for future Laravel collections).

Type of change

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update
• [ ] Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

• [x] Existing tests have been adapted and/or new tests have been added
• [x] Add a CHANGELOG.md entry
• [x] Update the README.md
• [x] Code style has been fixed via composer fix-style

[tanerkay]

Still learning myself, not sure if it is best to include the key type and value type in the type hint.

With:

 * @property-read \Illuminate\Database\Eloquent\Collection<int, Simple> $regularHasMany

Without:

 * @property-read \Illuminate\Database\Eloquent\Collection<Simple> $regularHasMany

[LastDragon-ru]

phpstan with checkGenericClassInNonGenericObjectType: true wants

• @property-read \Illuminate\Database\Eloquent\Collection<int, Simple> $regularHasMany

[Legion112]

How the progress is going on this one? Will it support previous versions of laravel?

[Legion112]

In pull request, I don't see generation for \Illuminate\Database\Eloquent\Collection method such get, add, etc

[Legion112]

Each method here should have and template annotation to define return time base on template type

[tanerkay]

@Legion112

Will it support previous versions of laravel?

Yes

In pull request, I don't see generation for \Illuminate\Database\Eloquent\Collection method such get, add, etc

The idea is that the collection class is type hinted, so the IDE can infer those class methods. Right?

Each method here should have and template annotation to define return time base on template type

That didn't make sense to me. Rephrase?

[Legion112]

@tanerkay

The idea is that the collection class is type hinted, so the IDE can infer those class methods. Right?

This is wrong. You need to have a defined template that ide would be able to know what will be return then you call a method on the collection.

Look here https://psalm.dev/docs/annotating_code/templated_annotations/

Without such a template, IDE cannot determine the type of any method of the collection It would alwasy think mixed.

/**
 * @template T
 */
class MyContainer {
  /** @var T */
  private $value;

  /** @param T $value */
  public function __construct($value) {
    $this->value = $value;
  }

  /** @return T */
  public function getValue() {
    return $this->value;
  }
}

[tanerkay]

@Legion112

This is wrong. You need to have a defined template that ide would be able to know what will be return then you call a method on the collection.

Sorry, still don't understand. The templates are defined in the Laravel collection classes: \Illuminate\Database\Eloquent\Collection and \Illuminate\Support\Collection. Are you looking at Laravel 8 code?

[tanerkay]

Haven't really been following this closely or thinking too much about it as PhpStorm still has incomplete support for this, see https://youtrack.jetbrains.com/issue/WI-64963/@property-does-not-support-@template-declaration

[Legion112]

I didn't have much sleep lately 😪 . I will try my best to read the last link to understand the problem. @tanerkay

[barryvdh]

So what is the status of this, is this 100% done or not?

[mfn]

It definitely slipped my radar to go through the PR, whoopsie. Hope to do it in the next days!

[bbprojectnet]

Any news on this? ;)

[tanerkay]

Any news on this? ;)

Still not supported by PhpStorm at time of writing (version 2022.2)

Feel free to +1 the linked issue on Jetbrains' issue tracker or comment, as it would be awesome if this were implemented sooner rather than later: https://youtrack.jetbrains.com/issue/WI-64963/@property-does-not-support-@template-declaration

[shahruslan]

Is this PR waiting for a bug in PhpStorm to be fixed?

[tanerkay]

Is this PR waiting for a bug in PhpStorm to be fixed?

yes, I've lost track of the actual issue in their bug tracker, as they've marked them as duplicates of unrelated issues

[tanerkay]

Apparently fixed in upcoming PhpStorm version, 2022.3 :tada:

[jnoordsij]

Any update on this? Would be great to have!

I opened https://github.com/nunomaduro/larastan/issues/1470 in an effort to create a workaround on their side, but enabling this package to allow for adding generics would be much better.

[grEvenX]

@barryvdh Support for this has been out for a while in PhpStorm now and this PR should be ready for review. I've tested it on our codebase and it works perfectly 👍

I guess a decision should be made if this should now in 2023 be set as the default, or if it's needed to set use_generics_annotations to true in the ide-helper config file.

[tanerkay]

I guess a decision should be made if this should now in 2023 be set as the default, or if it's needed to set use_generics_annotations to true in the ide-helper config file.

I think it makes sense to, I just have the following concerns:

• it ties this package more with PhpStorm. might be undesired for developers who don't use it. Nevertheless, they have the option to change the config value.
• users using Laravel 8 and running vendor:publish for the first time will have the option set to true*

I guess the benefits from having the collection type hinted using generics outweighs the limited benefit from the alternative syntax.

In any case, I've changed the default in this PR to true. Can leave the decision to @barryvdh . The change is in https://github.com/barryvdh/laravel-ide-helper/pull/1298/commits/28775bd73779318b764f91ca28b2e7f3ae312478 in case it is decided to revert it.

*An alternative middle of the road option is to block the use of generics annotations if the Laravel version is not version 9 or greater, e.g.

diff --git a/src/Console/ModelsCommand.php b/src/Console/ModelsCommand.php
--- a/src/Console/ModelsCommand.php	(revision 28775bd73779318b764f91ca28b2e7f3ae312478)
+++ b/src/Console/ModelsCommand.php	(date 1674662581752)
@@ -1091,7 +1091,7 @@
     protected function getCollectionTypeHint(string $collectionClassNameInModel, string $relatedModel): string
     {
         $useGenericsSyntax = $this->laravel['config']->get('ide-helper.use_generics_annotations', false);
-        if ($useGenericsSyntax) {
+        if ($useGenericsSyntax && version_compare($this->laravel['version'], '9', '>=')) {
             return $collectionClassNameInModel . '<int, ' . $relatedModel . '>';
         } else {
             return $collectionClassNameInModel . '|' . $relatedModel . '[]';

[tanerkay]

I would not kid ~ourselves~ myself over this: PhpStorm is on the forefront and their endeavors are part of why the community thrives, too.

Fair call, I wanted to bring up the topic but acknowledge the significance of PhpStorm at the same time.

Some tests are failing, but I'd willing to approve this, makes sense (now that's 2023).

Awesome. I'll sort out tests today :)

[tanerkay]

tests updated

[tanerkay]

Is there a test now where it does not use the generics annotation?

Yes, in the namespace Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\GenericsSyntaxDisabled

https://github.com/barryvdh/laravel-ide-helper/pull/1298/files#diff-7f2f9319ecfc1e0b601eeb87d07266d79901707f53c161979a89918ce2e4d6b4

[tanerkay]

I know that creates a regression for users who just updated the package and not the config, but this is easily fixed if they care/it poses a problem for them and, in the end, this is a change for the better so we want to promote this.

That would be my preference too, but was wary that it's a regression as you mentioned. If the idea is to promote it (which I'm completely in agreeance), then let's change it :)

[Sergiobop]

I don't know if it is just me, but after this is released, every time the model command is ran, some generic annotations are being duplicated even if they are already in the phpdoc.

[grEvenX]

@Sergiobop I see I get the same experience when running:

php artisan ide-helper:models "App\MyModel" --write --reset
php artisan ide-helper:models "App\MyModel" --write

Should probably open a new issue about it.