Add types to responses

[provokateurin]

From #36666

Summary

Sadly the methods have to be overridden in all the subclasses, but the code for that is always mostly the same.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[provokateurin]

The addHeader is not being override due to the lack of one feature and the existence of one (might be even two) bug(s) in psalm. The method is not used much in the server code where it would be relevant for the OpenAPI changes. This PR should be merged regardless of this missing piece.

[provokateurin]

I think the addHeader method should be deprecated to solve the issue

[nickvergessen]

I think the addHeader method should be deprecated to solve the issue

Deprecation means we can remove it in 3 years. So it won't solve your issue any time soon.

[provokateurin]

I know, I will have to add some comments about this in the OpenAPI tutorial/guidelines.

[ChristophWurst]

Is there any mechanism is place to ensure this doesn't fall apart with future modifications of the code? How does a developer check if they need to annotate changed/new APIs?

[provokateurin]

Is there any mechanism is place to ensure this doesn't fall apart with future modifications of the code?

Do you mean the code of the response classes? Those are validated by psalm

How does a developer check if they need to annotate changed/new APIs?

Developers don't need to add any new annotations because of these changes.

[nickvergessen]

You mean internal, right? We are not planning to remove the function

[provokateurin]

@come-nc @nickvergessen I will wait for your approvals as well, just to be sure everything is alright

[nickvergessen]

but fine either way

[nickvergessen]

Missed one last type, but fine by me

[provokateurin]

🎉

[ChristophWurst]

Mail's Psalm shows that all responses have to be type-annotated now due to vimeo/psalm#6870 (comment). Example: https://github.com/nextcloud/mail/blob/b7b84f452ac6951d0d2e6d96e33b9eec91ac6098/lib/Http/AttachmentDownloadResponse.php https://psalm.dev/r/92dc4c7871

ERROR: MissingTemplateParam - lib/Http/AttachmentDownloadResponse.php:29:7 - OCA\Mail\Http\AttachmentDownloadResponse has missing template params when extending OCP\AppFramework\Http\DownloadResponse, expecting 3 (see https://psalm.dev/182)
class AttachmentDownloadResponse extends DownloadResponse {

It means those responses have to specify a type for the template (for static headers and status code) https://psalm.dev/r/68242bdab6.

If I spec the type for server master I'll get an error for stable27 and older: https://psalm.dev/r/7038ef2ed1

Could there be any trick to allow responses to have types but with a default?

[provokateurin]

For subclasses you either need to implement the template parameters which should be quite easy or you just supress the error. Implementing the template parameters should be quite easy and afterwards you don't need to add the parameters when you use the type (like in return annotations), you can just leave them empty and it will use the "default" parameters.

[ChristophWurst]

If I spec the type for server master I'll get an error for stable27 and older: https://psalm.dev/r/7038ef2ed1

[provokateurin]

Well, then suppress until you support only 28+?