psalm / psalm-plugin-laravel

A Psalm plugin for Laravel
MIT License
307 stars 71 forks source link

Custom phpdoc getting removed #193

Open eithed opened 3 years ago

eithed commented 3 years ago

Is your feature request related to a problem? Please describe. When investigating an issue that was ultimately reported on https://github.com/barryvdh/laravel-ide-helper/issues/1236 I've found that, within psalm plugin, php artisan ide-helper:models command is run with '--reset' => true, parameter, thus removing phpdoc blocks that are provided ie within ModelStubProvider::generateStubFile:

$models_generator_command->run(
            new ArrayInput([
                '--nowrite' => true,
                '--reset' => true,
            ]),
            new NullOutput()
        );

In my case class for which I'm running psalm has following declaration:

/**
 * @property \App\DTO\QuoteGroup\Premium[]|null $premium
 * @mixin IdeHelperQuoteGroup
 */
class QuoteGroup extends Model {

protected $casts = [
        'premium' => Serializer::class . ':' . Premium::class,
];

(with Serializer::get returning mixed type) and as such will be generated within _ide_helper_models.php as this:

namespace App\Models{
/**
 * @property \App\DTO\QuoteGroup\Premium[]|null $premium

But within models.stubphp, because phpdoc is ignored, it'll be generated as this:

* @property mixed|null|null $premium

Currently I don't see any other issues when operating on the $premium parameter, but I can imagine that as soon as I'll try to access a property / method that exists on \App\DTO\QuoteGroup\Premium I'll get a false positive about missing property / method when running analysis for mixed.

Describe the solution you'd like

$models_generator_command->run(
            new ArrayInput([
                '--nowrite' => true,
                '--reset' => true,
            ]),
            new NullOutput()
        );

to change to:

$models_generator_command->run(
            new ArrayInput([
                '--nowrite' => true,
            ]),
            new NullOutput()
        );

Describe alternatives you've considered I don't think there are any alternatives

mr-feek commented 3 years ago

cc @caugner who added the --reset functionality here: https://github.com/psalm/psalm-plugin-laravel/pull/170

caugner commented 3 years ago

Hm, I'm experiencing the same issue, where the plugin (or ide-helper) doesn't automatically create @property annotations for relationships, an adding those annotations manually doesn't solve the issue (because custom PHPDoc is not copied over to the model stubs, as per #170).

Unfortunately this is a dilemma, and the only solution I can see would be if the model stub generation (by ide-helper) would resolve types in existing PHPDoc, or just copied the imports as well. 🤔

eithed commented 3 years ago

@caugner I don't understand what you mean. Didn't you add '--reset' => true,? This actually prevents the annotations from being copied from the stubs generated by barryvdh/laravel-ide-helper

caugner commented 3 years ago

@eithed So the reason I added '--reset' => true was that barryvdh/laravel-ide-helper does not copy the imports corresponding to the annotations, so if you use an imported type in one of your annotations, Psalm will always report that this type does not exist in the namespace of your model (because it doesn't, the import is just missing).

eithed commented 3 years ago

Ah, I see, thank you for the explanation - in my case I've used the class name including namespace hence it would work, but I can see now why that will not always be the case.

I guess the solution is to either:

Will try to create a PR for barryvdh/laravel-ide-helper to expand the annotation to full class name.

frostfire64 commented 2 years ago

Is there any hope this issue can be resolved soon? Sadly, it renders psalm almost unusable for my use case (baked-in phpdoc generated from ide-helper vs generated from psalm-plugin).

mr-feek commented 2 years ago

@frostfire64 up for a PR?