Skip to content

Conversation

@provokateurin
Copy link
Member

We already log an error if no response is returned from a controller method, but that is also satisfied if a @throws annotation is present.
Luckily with the return statements we can easily check if any is present (there might be controller methods that only throw or never return), so a separate error is now logged in case the code has a return statement but no @return annotation.

@provokateurin provokateurin requested a review from come-nc May 13, 2025 08:57
@provokateurin
Copy link
Member Author

Looks like we're lucky and none of the apps we currently check in CI has this problem.

Copy link

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test for it?

@come-nc
Copy link

come-nc commented May 13, 2025

Looks like we're lucky and none of the apps we currently check in CI has this problem.

It’s not luck it’s good work 🤝

(Or the check is not working 🤷 )

@provokateurin
Copy link
Member Author

provokateurin commented May 13, 2025

No test for it?

We can not do negative testing with out integration tests :/

Or the check is not working

But I can assure you that I tested this on a https://github.com/nextcloud/end_to_end_encryption and it correctly flags the methods that have this problem :)

@provokateurin provokateurin merged commit da4f096 into main May 13, 2025
16 of 19 checks passed
@provokateurin provokateurin deleted the fix/controllermethod/prevent-missing-return-annotation branch May 13, 2025 09:24
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.

3 participants