I'm going to take http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/ (accessed 2013-09-23) as being the current agreed standard, and use the equivalent headings there as my references.
WordPress uses a customized documentation schema that draws inspiration from PHPDoc, an evolving standard for providing documentation to PHP code, which is maintained by phpDocumentor. Where there may be ambiguity, the proposed phpDoc recommendation can be consulted as a secondary reference.
phpDocumentor is not really the technical maintainer for PHPDoc - that would be http://phpdoc.de/ - however, since that never made it out of 1.0beta, then the implementation and features offered by phpDocumentor 1.x, one of many PHP Documentation Generation Applications (DGAs), has become the de facto standard. Since Mike van Riel took over phpDocumentor 2.0, he's wanted to get PHPDoc formally drawn up into the PSR so that all PHP projects can follow it, so that all DGAs, and IDEs, can all be expecting the same format of documentation.
This is probably the simple most obvious point right at the outset - there shouldn't need to be a custom parser to read the DocComments. Anyone should be able to run phpDocumentor, Doxygen, APIGen or similar applications against the WP code base, and themes and plugins that use the same documentation format, and come up with the same content output. If WordPress deviates from the expected syntax in some way, then the parsing is going to raise errors and fail in those programs.
Should global variables be included in the list of documented things? e.g. @global WP_Post $post
? If not, then perhaps it can be explicitely mentioned to not be documented (not sure why you wouldn't).
There's no mention anywhere about whether there's an expectation that themes and plugins SHOULD use the documentation guidelines listed here. It would be suitable to add a note here about that.
Document "what" and "when" - "why" should rarely be included.
I agree with this, but I think more emphasis needs to be given to make this clear that you're specifically talking about Short Descriptions here (I missed it first time around). If "what" is required, beyond the short description, then the code is either not self-documenting, or the structural element is doing too much. Similarly, the "why" might well be suitable to be included within the long description - the short description tells me what the function does, but why is it needed in the first place? The wording here doesn't make this clear.
Use of present tense for short description e.g. Display, and not Displays; Change and not Changed, or Changes, is good practice.
What I'd like to see is some common preferred verbs suggested for the start of the short description for consistency in terminology. i.e. Display vs Show vs Echo, or Return vs Get, or Filter vs Change etc. Similarly for filter functions where you've got one item coming in, and one item being returned - I tend to use Existing and Amended e.g.
/**
* ...
*
* @param string $text Existing markup.
*
* @return string Amended markup.
*/
function foobar( $args ) {
//...
}
That's not to say that each of these preferred verbs and words MUST be used, but SHOULD be considered before trying other words.
There's no note about what length of line documentation should run to (historically 80 like code, but on modern screens 120 is better), and where the next line should start from (e.g. same as line above for long description, or x spaces in for tag descriptions). Give a more complex example (real-world) to highlight where style ambiguities can be addressed.
This title should just be "DocBlock Formatting", since a PHPDoc in a DocComment is a DocBlock.
Short and long descriptions using a period at the end - good. Indicates that these are sentences, not accidentally truncated and that they could be displayed elsewhere, like a DGA output.
Looking at a few random core files, I can see evidence of different orders of tags, so the following suggestion comes with an awareness of historical implementation. Functions tend to have @since
tag straight after the short (and optional long) description(s). In methods however, @since
seems to come after the @see
and other tags. This seems inconsistent - the @see
is optional, whereas the @since
isn't (or shouldn't be), so having it as the first tag means it's always easy to find when scanning.
The list also separates the @global
tag from the @param
tags with several other tags in between. Since these are for similar things (variables used within the function or method), I think that having these together would make more sense for most developers.
The list doesn't include @see
or @link
, yet the example does.
My preferred order would be:
/**
* Short description. (use period)
*
* Long description.
*
* @since x.x.x
*
* @access (for functions: only use if private)
* @link URL short description.
* @see Function/method/class relied on
* @uses FQSEN
*
* @global type $varname short description.
* @param type $var Description.
* @param type $var Optional. Description.
* @return type Description. (@return void if a non-returning function)
*/
In that:
@since
stands out on it's own, right below the description, as it's an often-wanted tag, and can be consistently the first tag.- Next block is the optional tags, including
@uses
(not in your example). The advantage of using@uses
with a Fully Qualified Structural Element Name instead of@see
, is that DGAs can generate a reciprocal@used-by
tag, meaning docs can link both ways between what a function uses, and what uses that function.@see
does have it's own usage of course, but it is different to@use
, so@use
shouldn't be replaced with@see
. @global
moved down to be next to@param
.@return void
- see below.
There are no explicit notes about spacing - either clear lines between different tags or groups of tags, or for lining up type and description in global, param and return. While in the grand scheme of things it doesn't matter one way or another, it's these details that the standard should be covering, so a consistent implementation of style can be reached, as well as the documentation content.
With some quick and probably slightly inaccurate regexes, I see 6726 or so functions in WP core, and 2441 instances of @return
. Of those, only 62 are @return void
. There are about 1000 instance of return;
. Now, there will be undocumented functions, but also non-returning functions and without more careful analysis or knowing most functions are documented, it will be difficult to determine, but that means that the majority of documented functions are not @return void
. As such, there's no historical precedent to adhere to.
With that in mind, my viewpoint is outlined in this March 2012 article I wrote: Why @return
void is wrong in PHP documentation. Note the points raised in the comments as well. In short, since WP isn't generally using @return void
currently, it would be wrong on several levels to use it now.
I'm delighted that this level of documentation is being done, but not how it is being done, from the example.
Use of the as-yet-unsupported-by-any-external-DGA @type
tag is great to see, since it is part of the draft PSR. However, the use of 'key'
instead of $key
(the latter is used in the PSR) is both confusing and an un-necessary deviation from what DGAs in the wider PHP world will be expecting. Even more odd, is that the example for the Hooks section does (correctly) use $key
, so the examples currently shown in the standards seems to introduce inconsistencies between itself.
There's no reason I can think of for this deviation - WP_Parser can be updated to convert that piece of data to display inside single quotes for dev.WP, and I agree that it would make the documentation there cleaner. Let's keep the original source documentation as close to the PSR as possible and handle the conversion outside of the WP core files.
Instead of giving a dummy example, document the mentioned wp_remote_*()
or some other function in the way it should be, and paste that in instead. It will also help clear up what you mean by 'originating' function, as I don't think that's worded particularly well.
Similar points to above regarding @see
instead of @use
and @return void
, but otherwise, spot-on. @deprecated
, 3-digit version number, and a sentence-case description, appearing right underneath @since
. Lovely.
While @access
is useful for indicating functions that should be considered private, for class methods and class properties they are redundant and even if they exist currently, I feel they should be removed in favour of ensuring the visibility keyword is in front of the method or property definition. This makes the code more self-documenting, helps IDEs filter down what methods / properties are available for auto-complete, and reduces maintenance overhead since there's no having to ensure existing visibility keyword and @access
value match up.
Dropping @access
will then also make @since
the first tag, again making it consistent with other types of Structural Elements.
The arrays documentation correctly uses the @type
tag, as per the PSR, but here the tag is the deprecated @var
. This might be used here due to historical implementation, but that in itself brings up an interesting sidebar.
I'd like to think that this documentation standard is aimed at the whole WordPress community - core, themes, plugins and anything else that emerges. As such, the standard here should represent the end goal of the best practice that we as a community want - and in this case, it shouldn't be to still use deprecated tags. We know existing documentation and code standards aren't great within areas of WP core, but if a halfway-standard is seen to be the norm in the meantime, then new plugins will start using @var
instead of @type
. I'm not expecting all WP core @var
to be updated to @type
just for the sake of it, but the standard should push what is ultimately desired, as well as being seen to be consistent within the standard itself.
Fine (although I personally don't like the completely redundant brackets - but that seems to be the de facto code standard for WP unfortunately).
The push to get these documented is fantastic! While presumably WP_Parser will handle this well, this is where a plugin for phpDocumenter or other DGAs would be needed to pull out the documentation, but that's fine. What we're documenting here is over and above what the PSR says, in a consistent format to a general DocBlock, rather than deviating from it.
Nothing much else to add here - although you've naturally got the @since
as the first tag, emphasising the previous suggestions to get it first elsewhere as well.
I'm not entirely comfortable with this suggestion, but I'll make it anyway. Is it worth adding a note (more aimed at plugins and themes) that they should try to reduce duplicate calls to hooks? My rationale is that, if you've got more than one occurrence of something happening (here, an action hook being done, or a filter being applied), then that should be refactored into it's own function anyway, just the same as if you were doing some other function call with identical arguments all over the place.
Hopefully, //duplicate_hook
will be seen as a code smell, and a chance to refactor the code, rather than a marker that needs documenting with some as-yet-undecided format in the future.
While single line comments are correct, the multi-line comments example is wrong. They SHOULD NOT start with /**
, as that makes the comment into a DocComment. When the code is tokenized, these are two different things (T_COMMENT
vs T_DOC_COMMENT
), and may cause DGAs or other applications looking for DocComments to un-necessarily parse these comments as well. /*
at the start is sufficient for a standard multi-line comment.
What URL are you expecting for the @link
in the example here?
While @subpackage
is deprecated, I can see 623 occurrences in core, so there is some historical baggage for this. However, at least some of theme are redundant, since they occur on functions where the file-headers that function appears under already has that @package
and @subpackage
(see wp-includes/taxonomy.php for instance). Once these are removed (since the functions inherit those tags from the file-level headers anyway), then the number might be cut down to a level where changing it to @package WordPress\Taxonomies
is doable when the file-level headers are being updated anyway.
See my points for Class Properties.
I've addressed most of the bits I disagree with here in all the notes above, so I won't repeat them.
@deprecated
- the format here which includes the "replacement function name" instead of a Description is different to what is given in an example previously.
@name
- there are only 8 occurrences of this in WP core, and most of them are redundant, as the name of the global variable, and in (or should be in) @global
is identical. It would be worth targetting this so that all uses of the @name
tag is removed. It's not even mentioned in the PSR, which suggests it was considered part of the de facto standard to then mark as deprecated.
@package
- "Found in doc block at..." => "Found in DocBlock at..."
@subpackage
- "For page-level docblock" => "For file-level DocBlock"
@uses
- "Note: This tag has been used in the past, but should no longer be used." - Why?
Ah, just got down to see this, so some of my points about @var
and @subpackage
are moot. Might be nice to include a reference to this Deprecated Tags section for each and every reference to those tags higher up the page, so that others don't come along and miss out this very important section! Waiting until the PSR is finalised does make sense before dropping those tags, and I fully support the decision here.
In terms of multi-word package / subpackage names, I've asked for some clarification on what the PSR might recommend, if anything. While I realise that WordPresss historically uses an underscore separator, I've personally had issues where underscores were considered to be like namespace level separators \
(as per PSR-0 autoloading), and therefore made what should be one level into two e.g. WordPress\Press\This or WordPress\Installer\WP\Config. If this did need changing, there are currently 189 instances of subpackages with underscores in (but as above, some of these may well be redundant at the function-level).
One other tag that I'd like to suggest is noted here, so it can gain some traction across the community, is the @wordpress-plugin
tag. It's technically an annotation, the format of which is described in the PSR. It's been added to the popular WordPress Plugin Boilerplate since May 2013, and while phpDocumentor and other DGAs can't do anything with it now, there's no reason an extension couldn't be written for them so that they could, even if it's just to stop the plugin headers from compacting on to one line within the output, or directly link to the Plugin or Author URI etc. It also neatly solves the problem of having both PHPDoc headers and the plugin headers recognised by WP within the same DocComment, since non-tag content MUST NOT appear under tags.