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

Support for custom models file #139

Open
brendt opened this issue Apr 8, 2021 · 7 comments
Open

Support for custom models file #139

brendt opened this issue Apr 8, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@brendt
Copy link
Contributor

brendt commented Apr 8, 2021

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.

@brendt brendt added the enhancement New feature or request label Apr 8, 2021
@mr-feek
Copy link
Collaborator

mr-feek commented Apr 16, 2021

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!

@mr-feek mr-feek removed their assignment Apr 16, 2021
@brendt
Copy link
Contributor Author

brendt commented Apr 16, 2021

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

@caugner
Copy link
Contributor

caugner commented Jul 13, 2021

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

@brendt Can you please elaborate:

  1. What is your environment? Please provide the output of: composer show | grep -E 'psalm|laravel'
  2. 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
Copy link

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

mr-feek commented Aug 2, 2021

PRs welcome!

@brendt
Copy link
Contributor Author

brendt commented Aug 3, 2021

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

mr-feek commented Aug 3, 2021

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

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