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

Custom phpdoc getting removed #193

Open
eithed opened this issue Jul 26, 2021 · 7 comments
Open

Custom phpdoc getting removed #193

eithed opened this issue Jul 26, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@eithed
Copy link
Contributor

eithed commented Jul 26, 2021

Is your feature request related to a problem? Please describe.
When investigating an issue that was ultimately reported on barryvdh/laravel-ide-helper#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

@eithed eithed added the enhancement New feature or request label Jul 26, 2021
@mr-feek
Copy link
Collaborator

mr-feek commented Jul 26, 2021

cc @caugner who added the --reset functionality here: #170

@caugner
Copy link
Contributor

caugner commented Aug 6, 2021

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
Copy link
Contributor Author

eithed commented Sep 2, 2021

@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
Copy link
Contributor

caugner commented Sep 3, 2021

@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
Copy link
Contributor Author

eithed commented Sep 3, 2021

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:

  • always use full class names in phpdoc thus allowing to remove --reset (impractical, people won't do this)
  • resolve class name to full class name in barryvdh/laravel-ide-helper
  • resolve class name to full class name in psalm/psalm-plugin-laravel

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

@frostfire64
Copy link

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
Copy link
Collaborator

mr-feek commented Feb 28, 2022

@frostfire64 up for a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants