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

feat: add support for Redis facade #615

Merged
merged 2 commits into from
Jul 30, 2020
Merged

feat: add support for Redis facade #615

merged 2 commits into from
Jul 30, 2020

Conversation

canvural
Copy link
Collaborator

@canvural canvural commented Jul 21, 2020

Closes #581
Closes #14

I'll update the PR once the new PHPStan version is released.

@canvural canvural force-pushed the redis-facade branch 2 times, most recently from 2a2b226 to e1b4110 Compare July 21, 2020 22:03
@szepeviktor
Copy link
Collaborator

What a simple solution!

@canvural
Copy link
Collaborator Author

What a simple solution!

Indeed. Thanks to the @mixin support in PHPStan

I think with this way we will finally have a solution to #203 But it'll be time-consuming to add the annotation to every class.

@mfn
Copy link
Contributor

mfn commented Jul 22, 2020

Is there anything the laravel project could do to improve this? For example they are using @mixin too in some of their phpdoc (https://github.com/laravel/framework/blob/0b12ef19623c40e22eff91a4b48cb13b3b415b25/src/Illuminate/Queue/Capsule/Manager.php#L11 and others).

@canvural
Copy link
Collaborator Author

@mfn @mixin should be on the facade class to be able to understand the calls like Redis::lrange

Laravel usually has @see annotations on their facades. Maybe those can be replaced with @mixin instead 🤔 That way both IDE's can autocomplete facade method calls, and static analyzers will be happy.

@canvural canvural marked this pull request as ready for review July 30, 2020 13:04
@canvural canvural merged commit a42b2f6 into master Jul 30, 2020
@canvural canvural deleted the redis-facade branch July 30, 2020 13:07
@szepeviktor
Copy link
Collaborator

@canvural We are almost out of problems!

@canvural
Copy link
Collaborator Author

@canvural We are almost out of problems!

That's a problem!

@mfn
Copy link
Contributor

mfn commented Jul 31, 2020

Laravel usually has @see annotations on their facades.

I kind of read it but didn't realize until I saw a Laravel PR today which added annotations for method and I saw on the boundary the @see annotation. Now I got it.

Bold idea: add a temp source hack to see what happens if phpstan/larastan interprets @see as mixin for once :)

@mfn
Copy link
Contributor

mfn commented Jul 31, 2020

We are almost out of problems!

I'm sure I can create new ones soon 😝

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

Successfully merging this pull request may close these issues.

laravel redis not able to check Redis Facade publish, hmget, expire
3 participants