Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
I notice there are 24 places in core where the API documentation uses a tag of "@returns" (with a trailing 's') instead of the proper tag "@return" to document the return value of a function/method.
The attached patch corrects these errors.
Steps to reproduce
Proposed resolution
Remaining tasks
Rreview
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #31 | interdiff-26-31.txt | 469 bytes | tr |
| #31 | 2916306-26-for-drupal-10.patch | 13.59 KB | tr |
| #26 | interdiff-24-26.txt | 537 bytes | tr |
| #26 | 2916306-26.patch | 14.28 KB | tr |
| #25 | Screenshot 2023-10-05 at 9.49.58 AM.png | 50.28 KB | smustgrave |
Comments
Comment #2
chiranjeeb2410 commented@TR,
Patch applies cleanly to local. Good to go. RTBC for me.
Comment #3
dawehnerThank you for providing a patch. We are fixing coding/documentation standard fixes in a more structure way though, see #2571965: [meta] Fix PHP coding standards in core, stage 1. Please ensure that changes are covered by a
phpcs.ymlchange.Comment #4
tr commentedAlready covered by the check for a @return tag - "@returns" is not recognized as a @return tag. Setting this back to RTBC.
Comment #5
dawehnerWell in this case I think the right approach would be to adapt the existing check so it is able to find these mistakes as well.
Comment #6
alexpott@TR thanks for working on this. Coding standards are important and errors like these are annoying. As a coding standard fix and in order to make it we should change the core/phpcs.xml.dist file so that we don't make the same mistake again.
Specifically it is covered by #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard. We need to file a child issue of that one that removes
<exclude name="Drupal.Commenting.FunctionComment.MissingReturnComment"/>from the phpcs.xml.dist and fix them all. This is going to take a long time since it will involve writing docs :( It is also going to take a longer time because it appears that the sniffDrupal.Commenting.FunctionComment.MissingReturnCommentdoesn't actually work as described because removing the exclusion and running againstcore/lib/Drupal/Core/Form/EnforcedResponseException.phpphpcs does not find the issue. Therefore we also need to file (and fix) an issue against the Coder module to fix the rule.Comment #7
tr commentedOK, so there's been an open issue in the Coder queue FOR 17 MONTHS to check for misspelled tags, where "@returns" is mentioned specifically as something that would be caught. #2729697: Add a sniff for unknown docs @ tags
That issue has been open for 17 months with no action.
So the way I see it, let's spend the 30 seconds to fix a known problem here, then WHEN and IF that issue is addressed and Coder actually gets fixed we will know it won't happen again. But I'm NOT going to participate in a trivial issue like this for years (that's not hyperbole, as evidenced by #2729697: Add a sniff for unknown docs @ tags and many other issues), hoping for a resolution, because that sort of effort is all out of proportion with the problem.
&BTW, when a "@returns" tag is used, that means the API module documentation generator ignores the tag and the return value remains undocumented on api.drupal.org. Those 24 methods/functions using the wrong tag will remain without visible documentation for return value until the patch is committed or Coder is fixed.
There is NO REASON to postpone simple fixes like this until the Coder issues are fixed. It's sufficient that there is an issue in the Coder queue, as a placeholder for a known problem that needs to be avoided. This problem needs to be fixed at some point, so kicking it down the road by several years just inconveniences everyone who is trying to use the API documentation in the meantime.
You can fix this AND work on the Coder issue at the same time.
(Changing priority to Normal because existing documentation is being lost and not accessible on api.drupal.org).
Comment #8
tr commentedComment #9
borisson_Reverting the title change and postponing this on the coder issue.
Comment #18
quietone commentedClosed duplicate issue and moving credit here.
Still to do here is to merge / move credit to the issue fixing this by sniff.
'
Comment #22
quietone commentedComment #24
quietone commentedThis was discussed in Slack by catch, larowlan, dww, bbrala, smustgrave and longwave. The consensus was that it is better to get this trival fix instead of waiting for the sniff. Also, as longwave pointed out that eventually all methods will have a return type so this will not be needed anyway.
To that end I chose to reroll the patch, yes staying with the patch workflow. Then I searched for for others instances and fixed those. Also, fixed any new errors caught php phpcs.
Comment #25
smustgrave commentedSearching for @returns and found a few more
believe the assets ones could be ignored.
Comment #26
tr commented@smustgrave: All but one of those "few more" are in .js files. JSDoc is different than PHPDoc (JSDoc allows use of @returns, among a lot of other things that are not allowed in PHPDoc) and JavaScript code is not included in the Drupal API documentation at api.drupal.org, which is what this issue was opened to fix.
The only one missed by the patch in #24 is core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php
Here is a new patch that fixes that as well.
Comment #27
mrinalini9 commentedHi,
I have reviewed patch #26, looks good to me.
It applied cleanly and replaced all the occurrences of
@returnswith@returnat all places except the .js files in the core.So, for me, it is RTBC.
Thanks & Regards,
Mrinalini
Comment #28
smustgrave commentedThanks for pointing that out @TR
Comment #29
alexpottCommitted c3ff576 and pushed to 11.x. Thanks!
It didn't cleanly backport to 10.1.x
Comment #31
tr commentedOne hunk of the patch in #26 does not apply in Drupal 10.1.x because a usage of @returns that was recently introduced only into 11.x by #3356894: Make field selection less overwhelming by introducing groups. That code does not exist in 10.1.x, and therefore that hunk does not apply to 10.1.x.
Here is a version of #26 that applies in 10.1.x.
Comment #32
smustgrave commented10.1 patch seems good.
Comment #33
alexpottCommitted 7cbdf08 and pushed to 10.1.x. Thanks!
Comment #35
longwaveI realised that we previously banned
@inheritDocwith a rule in phpcs.xml.dist; we can do the same for@returns: #3393661: Prevent @returns from creeping back into the codebase