Problem/Motivation

This META issue is a spin-off from #1792992: Are typed @param / @return part of the documentation gate? where several people voiced the view that all @param and @return directives in docblocks MUST include type hints. However, some stated that it only SHOULD be a suggestion since "core does not follow this standard yet." The focus of this META issue is to add missing type hinting to @param and @return directives throughout all of existing core documentation.

Documentation patches that include type hinting are time consuming to both review and commit because one must dig into the actual code to confirm that the type hints are both correct and complete. Hence, please be patient and try to limit type hint patches to covering only a limited number of docblocks (10-15 as a guess).

Proposed resolution

Add type hinting for all @param and @return directives in a series of "small" patches.

How to's

How to review patches for this issue:

# This is a one-liner to aid in reviewing patches on this Drupal.org meta issue: https://www.drupal.org/node/1800046
# You can use netbeansdrupalcomposed to gather the dependencies, which are phpcs and Drupal's Coder module
# https://www.drupal.org/sandbox/mile23/2197899
# Substitute any module path you want in order to check only that module.
# The resulting list will be a bit of gobbledygook in CSV, but will show you whether or not the
# patch you're evaluating successfully removed the errors we're looking for.
phpcs --standard="/path/to/drupal/coder/coder_sniffer/Drupal" --report-csv book/ | grep -F $'Missing param\nReturn type missing'

The above one-liner can also be found in this github gist: https://gist.github.com/paul-m/227822ac7723b0e90647

  1. Check that the parameters are identified correctly. For example, if the function signature is function_name($foo_bar), the parameter should be listed as $foo_bar in the docblock, not $foobar.
  2. Check that all parameters are documented, unless our standards explicitly specify omitting them (as with form constructors).
  3. Check that the parameters are in the order specified in the function signature.
  4. Read each parameter description and make sure that it is grammatically correct, clear English, with proper capitalization, punctuation, etc. (Each parameter description should form a complete English sentence if the words "This parameter is..." were to be added to the beginning, but those words should not be included in the docblock.) Keep an eye out for typos.
  5. Look up each function in the patch on api.drupal.org (D8). Read the function carefully and ensure that the parameter description properly documents how the parameter is used.
  6. Look at the type hint documented for each parameter. Make sure the types correspond to the allowed types at http://drupal.org/node/1354#param-return-data-type.
  7. Make sure all possibilities for the parameter are documented; for example, if the function optionally accepts a NULL value for a string parameter, then it should be documented as string|null.
  8. Also check the function's callers and string references (linked on api.d.o) and make sure the values being passed to the function are in the datatypes specified.
  9. Apply the patch to your local repository, and check that no other functions in these files are missing parameter documentation.
  10. Document what you reviewed and what you found specifically in your review comment.

Remaining Tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Update the issue summary noting if allowed during the beta Instructions
Module/File Issue # Notes
Action module #1802070: Add missing type hinting to Action module docblocks
Aggregator module #1807160: Add missing type hinting to Aggregator module docblocks
Ban module #1816732: Add missing type hinting to Ban module docblocks
Block module #1807674: Add missing type hinting to Block module docblocks
Book module #1807678: Add missing type hinting to Book module docblocks
Breakpoint module
Color module #1816760: Add missing type hinting to Color module docblocks
Comment module #1808478: Add missing type hinting to Comment module docblocks
Config module #1810002: Add missing type hinting to Config module docblocks
Contact module #1811226: Add missing type hinting to Contact module docblocks
Contextual module #1816764: Add missing type hinting to Contextual module docblocks
Dblog module #1811242: Add missing type hinting to Dblog module docblocks
Field module #1801998: Add missing type hinting to Field module docblocks
Field_ui module #1816782: Add missing type hinting to Field_ui module docblocks
File module #1811858: Add missing type hinting to File module docblocks
Filter module #1811328: Add missing type hinting to Filter module docblocks
Forum module #1811334: Add missing type hinting to Forum module docblocks
Help module #1811862: Add missing type hinting to Help module docblocks
Image module #1802060: Add missing type hinting to Image module docblocks
Language module #1811214: Add missing type hinting to Language module docblocks
Locale module #1807062: Add missing type hinting to Locale module docblocks
Menu_ui module #1800774: Add missing type hinting to menu_ui module docblocks
Node module #1800546: Add missing type hinting to Node module docblocks
Openid module #1816834: Add missing type hinting to Openid module docblocks
Overlay module #1816840: Add missing type hinting to Overlay module docblocks
Path module #1811874: Add missing type hinting to Path module docblocks
PHP module #1816846: Add missing type hinting to PHP module docblocks
Rdf module
Seach module #1811882: Add missing type hinting to Search module docblocks
Shortcut module
Simpletest module #1805340: Add missing type hinting to Simpletest module docblocks
Simpletest base test classes #1805346: Add missing type hinting to base test classes in Simpletest module docblocks
Statistics module #1811892: Add missing type hinting to Statistics module docblocks
Syslog module #1816858: Add missing type hinting to Syslog module docblocks
System module #1800330: Add missing type hinting to System module docblocks
Taxonomy module #1807002: Add missing type hinting to Taxonomy module docblocks
Toolbar module
Tracker module #1811888: Add missing type hinting to Tracker module docblocks
Translation module
Update module #1811202: Add missing type hinting to Update module docblocks
User module #1800174: Add missing type hinting to User module docblocks
Xmlrpc module
Bootstrap.inc file #1326456: Add missing @param in includes A-C, plus other corrections to some docblocks
Common.inc file #1326456: Add missing @param in includes A-C, plus other corrections to some docblocks
Database sub-system

Completed related issues

Related issues

Comments

jhodgdon’s picture

One more note... These patches take a *large* effort for writers, reviewers, and committers if they are done properly. In most (or at least many) cases, you will need to read the entire function code carefully to figure out what the argument and return value types are.

As I've said before, I personally do not think this is worth the effort to do for all of core. And just for the record, I'll just state here that I personally don't want to spend my volunteer Drupal time, as a committer, to doing final review and commit for these patches. I think I can find more valuable ways to spend my Drupal volunteer time, such as adding new features to the API module (for api.drupal.org) or reviewing/committing other patches (I'm already way behind on other clean-up efforts that I personally feel are more worthwhile).

This initiative has been tried before, and no one came up with the level of effort that would be needed to complete the project. I doubt it's available now... Sorry to be discouraging, but I really feel like the effort needed here could be better spent on other initiatives. But that's my bias... if you can come up with people who want to make it happen and are willing to spend the care and time necessary, I certainly won't stand in the way -- just don't assume that I'm making time available myself.

Lars Toomre’s picture

Issue summary:View changes

Updated the full set of modules in module table.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue for the User module. Also added links to patches for bootstrap and common.inc files.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added additional sprint meta issues to related issues section.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue for the node module.

Lars Toomre’s picture

Issue summary:View changes
Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub issue for image module.

Lars Toomre’s picture

Issue summary:View changes

Added sub issue for Action module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue for taxonomy module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue for Aggregator module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

TravisCarden’s picture

@jhodgdon, I see that the work here is going forward and that, despite your expressed wish not to spend time on it yourself, some of the issues are getting assigned to you. Is that going to be a bother or a distraction to you?

jhodgdon’s picture

If the patches have been carefully reviewed and marked RTBC by the reviewer, I can get assigned to commit them.

Lars Toomre’s picture

@TravisCarden Reviews in the sub-issues are welcome. Most to date deal with adding type hinting to *.api.php files.

Lars Toomre’s picture

Issue summary:View changes

Added comment sub-issue.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue for Config module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added Dblog sub-issue.

Lars Toomre’s picture

Issue summary:View changes

Added Filter module sub-issue.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue #1811858 for file module.

Lars Toomre’s picture

Issue summary:View changes

Added a sub-issue #1811862 for Help module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue #1811888 for the tracker module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue #1816840 for Overlay module.

Lars Toomre’s picture

Issue summary:View changes

Added sub-issue #1816846 for PHP module.

Lars Toomre’s picture

Issue summary:View changes

Updated issue summary.

jhodgdon’s picture

Issue tags:+coding standards

This issue forgot to have a tag. :)

ronan.orb’s picture

Issue Reminder Comment.

ronan.orb’s picture

Issue summary:View changes

Updated issue summary.

Mile23’s picture

Issue summary:View changes

Poll module is no longer in core.

Mile23’s picture

Issue summary:View changes

There's no menu module in core... Hijacking the menu issue to be about menu_ui instead.

Mile23’s picture

All child issues are either closed or have patches needing review.

Mile23’s picture

Issue summary:View changes

Added info about how to review the patches, available here: https://gist.github.com/paul-m/227822ac7723b0e90647

heddn’s picture

Issue summary:View changes
Issue tags:+Needs beta evaluation

Let's get a beta eval documented here. Update the issue summary noting if this allowed during the beta.