Support for custom models file

brendt commented 3 years ago

I'm not sure why, but with certain versions of this plugins things stop working. Especially when it comes to model properties not being found.

I would like to suggest a feature where you can instruct psalm to use the existing models files, instead of generating a new one on the fly.

I had a look at \Psalm\LaravelPlugin\Plugin::ingestModelStubs and thing the feature wouldn't be that difficult to implement. In my mind it would look something like this:

Read an optional config value that contains a path to the models file, if this value is set it should be used instead of a dynamically generated one
Copy this file over to the plugin's cache directory
Set self::$model_classes to contain the correct value — still need to figure that one out, but that shouldn't be any rocket science.

I want to PR this myself but I'll need some pointers on what the right approach is here. Would love your input.

mr-feek commented 3 years ago

Hey @brendt !

I'm sorry but I don't really work with Laravel much anymore at the dayjob, so I don't have much incentive to dig into this.

I can say that I really wanted the ability for this plugin to not rely on ide-helper. I think an option to not generate ide-helper stuff would be great. Would be very happy to merge a PR doing this!

brendt commented 3 years ago

Awesome, can you point me to a guide or something that explains best practices for psalm plugin development?

caugner commented 3 years ago

I'm not sure why, but with certain versions of this plugins things stop working

@brendt Can you please elaborate:

What is your environment? Please provide the output of: composer show | grep -E 'psalm|laravel'
What exactly stopped working? Please post the output of a Psalm run with the latest version of both vimeo/psalm and psalm/plugin-laravel.

Thank you! 🙏

ppmathis commented 3 years ago

I'll throw in another reason why using a pre-generated helper file would be useful: I am using Laravel Sail for my local development environment, so there is no direct access to the database unless I would explicitly forward the specific Docker container.

When I initially tried to use this plugin in a Sail project, I was wondering why Psalm did not seem to get any kind of model stubs whatsoever and the cache/ directory was empty as well. As it turns out, the ModelStubProvider uses a NullOutput and was therefor suppressing errors printed by barryvdh/laravel-ide-helper about being unable to reach the database.

Running Psalm inside Sail works (sail php ./vendor/bin/psalm), however this is kinda tedious to use and does not nicely integrate into IDE tooling. In my own projects I usually have a generate-ide-helper.sh which looks something like this:

#!/bin/sh
SAIL="./vendor/bin/sail"

"${SAIL}" artisan ide-helper:generate -n
"${SAIL}" artisan ide-helper:models -n -M
# ... other helper generators if applicable, e.g. Lighthouse

This allows me to quickly regenerate any helpers whenever I knowingly changed some models and my IDE picks them automatically up. With the current behavior of this plugin I would also either have to:

(a) always run Psalm within Sail to ensure it has an up-to-date helpers file as well
(b) run Psalm once within Sail to generate the cached helper file, then run Psalm outside as normal - while the latter would always fail to generate a new helpers file, the previously generated helpers file is being kept as it seems
(c) ensure that the database is accessible when running Psalm outside of Sail

If specifying a pre-generated helper file was possible, this plugin would simply use the same helper that my IDE uses and I would not have to worry about inconsistent helpers or providing database access while running Psalm on the host.

mr-feek commented 3 years ago

PRs welcome!

brendt commented 3 years ago

@caugner the issue is from a while back, so I don't remember any more details than the ones provided here. It doesn't really matter to me though: all these problems would be solved if I could manually specify a pregenerated ide helper file. I think @ppmathis came up with another good use case.

Like I said I'm able to contribute some time to PR this, but I'd like some pointers to where to start, maybe an high-level explanation of how psalm currently generates IDE helper data, etc.

mr-feek commented 3 years ago

I believe we would need 2 configuration options -- one for the path to the models file and one for the path to the facades file.

The symfony plugin supports config options, so we could likely use them to model our implementation https://github.com/psalm/psalm-plugin-symfony#configuration

https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Plugin.php#L87-L94

The files are generated / supplied to the plugin via a simple Provider pattern -- you could simply create a different type of provider for using an existing file https://github.com/psalm/psalm-plugin-laravel/blob/master/src/Providers/ModelStubProvider.php

psalm / psalm-plugin-laravel

Support for custom models file #139