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

DB facade alias support #126

Merged
merged 3 commits into from
Jul 4, 2021
Merged

DB facade alias support #126

merged 3 commits into from
Jul 4, 2021

Conversation

mzur
Copy link
Contributor

@mzur mzur commented Jan 29, 2021

This failing test highlights the issue that the DB alias for Illuminate\Support\Facades\DB is not properly handled by this plugin.

It could be solved with a new src/Stubs/DBFacadeAlias.stubphp:

<?php

class DB extends Illuminate\Support\Facades\DB
{}

However, I wanted to hear if there is a better solution.

mzur added a commit to biigle/core that referenced this pull request Jan 29, 2021
The DB facade alias is not properly handled by the Laravel Psalm
plugin: psalm/psalm-plugin-laravel#126
@gander
Copy link

gander commented Mar 1, 2021

Any progress?

@mzur
Copy link
Contributor Author

mzur commented Mar 2, 2021

I'm still waiting on feedback from the maintainers.

@mr-feek
Copy link
Collaborator

mr-feek commented Mar 18, 2021

Sorry but I don't use facades so I don't have much motivation to make them work with this plugin -- however I will happily merge any PRs that fix the issue!

@mzur
Copy link
Contributor Author

mzur commented Mar 26, 2021

IIRC in my initial test the additional stub fixed the issue. But now somehow it doesn't. I'll take this up again when I have more time. Meanwhile if anyone is interested in this, please feel free to suggest a solution.

@mzur mzur mentioned this pull request Mar 26, 2021
@mzur mzur marked this pull request as ready for review July 2, 2021 14:20
@mzur
Copy link
Contributor Author

mzur commented Jul 2, 2021

I had another look: Does anything speak against making the facade stub function static? Since it's a facade, the function should be called statically anyway. This resolves my failing test case but I don't know about any other possible implications.

@mr-feek mr-feek added the fix label Jul 4, 2021
@mr-feek mr-feek merged commit cf9fbd8 into psalm:master Jul 4, 2021
@mr-feek
Copy link
Collaborator

mr-feek commented Jul 4, 2021

thanks @mzur !

@mzur mzur deleted the db-facade-alias branch July 4, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants