Prevent @returns from creeping back into the codebase

But isn't this covered by another sniff already? If a method has a return value, there should be a sniff already that flags the method if @return isn't specified. And if you misspell @return, then the sniff should catch this.

Unless of course core has turned off that warning ... in that case the solution is to turn it back on and fix any other problems in core that are revealed by this.

In contrib, however, PHPCS does tell us if the @return tag is missing, if the @return type is missing, and if the @return parameter is not documented.

FunctionCommentSniff: "Missing @return tag in function comment"

BTW this is the same point I brought up 6 years ago when I opened 🐛 Use "@return" instead of "@returns" Postponed , and at the time @dawehner https://www.drupal.org/project/drupal/issues/2916306#comment-12300987 🐛 Use "@return" instead of "@returns" Postponed and @alexpott https://www.drupal.org/project/drupal/issues/2916306#comment-12302537 🐛 Use "@return" instead of "@returns" Postponed said that they though turning the sniff back on for core was the right thing to do in principle.

Checking for one specific misspelling (@returns) may help a little, but it doesn't fix missing @return documentation and it doesn't fix any other misspellings or casing problems with this tag, so it's only a baby step in the right direction.

The issue to add the sniff is 📌 [Meta] Fix Drupal.Commenting.FunctionComment.InvalidNoReturn Active . It was recently RTBC but was pushed back to split into child issues. There are 5 child issues, 2 at RTBC, 1 needs work, 1 is active and the last one is to enable the sniff. I'd much rather put effort into completing those issues instead of making this change. A change that, if made, should be removed when the sniff is enabled.

Of the child issues mentioned above it is now, 3 at RTBC, 1 is active and the last one is to enable the sniff. It was just random failures in one of the issues.

Tested on 11.x today

I changed the '@return' to '@returns' in \Drupal\views\Plugin\views\field\MultiItemsFieldHandlerInterface::getItems().

That resulted in this PHPStan error

 ------ ------------------------------------------------------------------------------------------------------------------ 
  34     Method Drupal\views\Plugin\views\field\MultiItemsFieldHandlerInterface::getItems() has no return type specified.  
         🪪  missingType.return                                                                                            
 ------ ------------------------------------------------------------------------------------------------------------------ 

So, although not a phpcs rule the @returns should not be creeping back in the code base.

Shall we close this as outdated or make a change to phpcs.xml.dist to add '@returns' to the the list of 'ForbiddenAnnotations'?

Tested again with @returns in

• includes/common.inc
• modules/dblog/dblog.module
• modules/migrate/src/Row.php
• modules/views/src/Plugin/views/field/MultiItemsFieldHandlerInterface.php

And PHPStan detected all of them with the same error message as in the previous comment.

I don't think #6 covers this, because you could have a native return type in combination with @returns. Presumably phpstan would happily accept the native return type and nothing would complain about the annotation.

Based on #8, I think we should make a change to phpcs.xml.dist to add '@returns' to the the list of 'ForbiddenAnnotations'. Moving this issue to active so that can be picked up.

+1 for #9, we already use forbiddenAnnotations to prevent a mis-application of @inheritdoc so we can do the same for this.

OK. It is now forbidden.

Looks good!

nod_ → closed merge request !11308

Committed 7abcc2f and pushed to 11.x. Thanks!