search

psalm / psalm-plugin-doctrine

Stubs to let Psalm understand Doctrine better
86 stars 43 forks source link

feat: add support for psalm 5 #133

Closed nesl247 closed 1 year ago

nesl247 commented 1 year ago

This is only a change to composer.json to see if CI passes.

Closes #132

orklah commented 1 year ago

Seems like the main obstacle seems to be a BC break in Psalm 5: https://github.com/vimeo/psalm/blob/master/UPGRADING.md?plain=1#L24

We would need to guard against that and use the correct method for each version. Or we could drop support for V4 and only use the new method

nesl247 commented 1 year ago

I spent quite some time looking at this, and can't figure out how to make this work. The functionality that is supposed to be the newer version does not do what we want from what I can tell because the only time it sets the Node is if something is changed on the node as it's being visited. I'm not really familiar with psalm's internal workings, so hopefully you'll have better luck. I do have a feeling that they may have entirely broken the functionality that is needed here.

orklah commented 1 year ago

Hey @danog would you have some time for an advice here? I didn't entirely followed what we did with the getChildNodes. Do you have an example of what we can do to replace it?

danog commented 1 year ago

What is being done here with getChildNodes is precisely the reason why that method was a bad idea, in this case it's being used to simply access the type_params property indirectly

weirdan commented 1 year ago

I've updated a bunch of tests for Psalm 5 and marked them accordingly. If Psalm 4 support is going to be dropped, it would be easy to grep for Psalm 4 substring to drop unused tests.

orklah commented 1 year ago

Looks good, thanks everyone! If we can keep V4 compatibility without too much effort, let's do that for now!

olegpro commented 1 year ago

@orklah can you bump version? :)

Thanks!

orklah commented 1 year ago

done :)