Introduce Form Template to improve inference for processed payloads

[gsteel]

Q	A
New Feature	yes
RFC	yes

Description

Introduce Form Template to improve inference for processed payloads…

This patch should allow users to provide templates for form classes with full inference of the resulting valid payload.

Added an interface method, which we could probably revert considering that Form::getData() has decent inference if you explicitly pass a flag, due to the conditional return on the interface.

There are some weird new issues around inheritance and fluent return types which I'm leaving here for now because I have no idea how to fix them, or why they were introduced by these changes. Maybe they are fixed in 5.16, so I'll update there…

Closes #238

[gsteel]

Psalm 5.16 doesn't fix the new LessSpecificReturnStatement problems. Maybe someone can take a look and see if they have any idea what's going on there.

[Slamdunk]

The overall patch looks very good to me.

My only worry is that if we don't fix the bugs/mess on PSalm here, the users could be flooded with invalid errors as well.

@Ocramius do you know how to get PSalm working, or anyone we could ask for help?

[gsteel]

Sussed it, All/most of the fluid return types in Form are annotated with @return $this, when inherited, psalm infers the return type as FormSubclass<TFilteredValues> and $this is therefore LessSpecific. Returning self or $this<TFilteredValues>, fixes the issues. I'll update the patch with the latter, because it is more precise

Ok, I don't actually think that psalm likes $this<TFilteredValues> at all. I found that either self or self<TFilteredValues> solved the issues in very specific places, but wholesale replacement of $this yielded many more errors.

I've updated the Static Analysis test case to illustrate template preservation using fluid return types, but not for all possible methods.

Also, I thought that adding an interface method might be a BC break? Happy to revert that if is.

[Slamdunk]

$ docker run --rm -v `pwd`:/app nyholm/roave-bc-check

[BC] ADDED: Method getValidatedPayload() was added to interface Laminas\Form\FormInterface                                                                                                                                                      
1 backwards-incompatible changes detected

I always thought it wasn't, but apparently it is a BC Break so yes please, revert it

[gsteel]

@Slamdunk
Can I baseline the remaining issues please 🙏
I feel like the issues are concerned with the deep inheritance hierarchy where everything extends from Element. You'll see that the issues are not applied to a SomeUserForm when extending from Form but to ancestors such as Element and Fieldset.

I'd like to try and keep this patch alive because the additional inference provided to consumers would be really useful for a project I'm working on 😁

[Slamdunk]

Can I baseline the remaining issues please 🙏

As long as you give me a reasonable guarantee that users' PSalm runs won't explode with unresolvable errors after this patch, I'm totally ok with this 👍

[gsteel]

To verify that these psalm issues don't crop up for inheritors of Form and Fieldset etc, I've fixed a load of issues with test assets to remove them from baseline completely.

Thanks @Slamdunk

[Ocramius]

Worth releasing: triggered a release