promise icon indicating copy to clipboard operation
promise copied to clipboard

[3.x] Add template annotations

Open simPod opened this issue 4 years ago • 8 comments

I was considering adapting this lib for promises though it's currently impossible to be type-safe with it.

I found this psalm plugin https://github.com/Bocmah/psalm-reactphp-promise-plugin that only includes stubs for psalm. So I was thinking it can be simply ported into this promise lib so it's also supported by phpstan and by any static analysis overall without need for any additional setup.

simPod avatar Mar 15 '21 10:03 simPod

@clue thanks for input. I'll look into how to test this.

Also note this is only partial implementation (for happy path) as both psalm and phpstan do not support default template types yet (it's impossible to resolve template to a type when null is passed).

This is resolving onFulfilled's type but not onRejected's one. I thought it might be a good start anyway to support at least one side for now. Or we keep it here and build up as the support emerges.

Crosslinks:
https://github.com/vimeo/psalm/issues/5407
https://github.com/phpstan/phpstan/issues/4801

On top of this, phpstan also supports generics, does it make sense to address this as part of the PR?

phpstan reads @psalm- annotations so this should be addressed if you meant this.
(personally, I don't use prefixes at all in my code but as I ported the initial implementation from psalm plugin, I kept them)

simPod avatar Apr 05 '21 11:04 simPod

I'll wait for default promise value support in SA since it's fairly useless without it

simPod avatar Jan 04 '22 15:01 simPod

@simPod thanks for your work on this so far :+1:

To keep you updated: @WyriHaximus, @clue and I are currently looking into this, we'll have a call later this day to discuss the details. There's also a discussion in pslam with more information on this: #7559.

SimonFrings avatar Feb 07 '22 10:02 SimonFrings

Thank you for update and pushing it forward!

simPod avatar Feb 07 '22 11:02 simPod

One major thing for me was to tackle the bigger picture, so not just this promise package. But also async and await and observables plus await with observables and get it right as a whole.

WyriHaximus avatar Feb 07 '22 12:02 WyriHaximus

@simPod Just a heads up: I'm going to make a bunch of suggested changes based on https://psalm.dev/r/22a9692a97 later today. But want thank you for your work so far on this and for pushing it forward so far :+1:

WyriHaximus avatar Mar 25 '22 11:03 WyriHaximus

FYI just put up #223 as a draft for the 2.x branch so I can pull it in through composer and test it out again in a project without having to manually edit files again. Will try to get back shortly and hope live doesn't get in between again

WyriHaximus avatar May 19 '22 15:05 WyriHaximus

FYI just filed https://github.com/reactphp/promise/pull/227 and https://github.com/reactphp/promise/pull/228 those will not have any direct affect on this PR as it's for the upcoming v3 release. We want basic support out there and then work on getting near to a 100% support out there through this PR and PR's for 1.x and 2.x.

WyriHaximus avatar Jun 21 '22 21:06 WyriHaximus

@simPod Hey if you don't mind I'm going to use your commit as a base to get template annotations + some additional type insurance in.

WyriHaximus avatar Jun 22 '23 22:06 WyriHaximus

Awesome 👌🏿

On Fri, Jun 23, 2023, 01:00 Cees-Jan Kiewiet @.***> wrote:

@simPod https://github.com/simPod Hey if you don't mind I'm going to use your commit as a base to get template annotations + some additional type insurance in.

— Reply to this email directly, view it on GitHub https://github.com/reactphp/promise/pull/188#issuecomment-1603368759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACQAJLSS6A4Q46K5ZQJXKLXMS57TANCNFSM4ZGKGHSA . You are receiving this because you were mentioned.Message ID: @.***>

simPod avatar Jun 23 '23 06:06 simPod

PR is up at: https://github.com/reactphp/promise/pull/247

WyriHaximus avatar Jun 28 '23 14:06 WyriHaximus