Opened 11 years ago
Closed 11 years ago
#27167 closed enhancement (duplicate)
Drop @access in favour of @internal
Reported by: | GaryJ | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.9 |
Component: | General | Keywords: | |
Focuses: | docs | Cc: |
Description
Our documentation standards say that:
"@access: Only use for functions if private. If the function or class method is private, it is intended for internal use only, and there will be no documentation for it in the code reference."
AFAIK, @access
was introduced via phpDocumentor into the defacto phpDoc standards during PHP 4. The public
, protected
and private
visbility keywords were not introduced for class methods and properties until PHP5, but since then, @access
has been redundant for use in classes.
There are 979 instances of @access
. 464 of them are marked as public, 119 and protected and 396 are private, meaning that nearly 60% of them shouldn't be there according to the standards. Many of them are also wrong anyway and contradict the code. e.g.
class FooBar { /** * @access protected */ var $my_property; /** * @access private */ function _my_func() {} }
The @access
tag is NOT part of the upcoming PSR-5 document, which means that documentation generators may not support it.
Since the only non-redundant usage for @access
is on functions, and the only valid value according to the documentation standards is @access private
, I propose that this is swapped out for the more descriptively accurate @internal
.
@internal
IS part of PSR-5. It can be used in two ways:
- As a normal tag "indicating that the associated structural element is used purely for the internal workings of this piece of software". WP-Parser already excludes importing functions and methods marked with
@internal
unless explicitly requested.
- As an inline tag to "add internal comments or additional description text inline to the Long Description". This should take the form of
{@internal Some comment.}}
.
WP currently has 124 instances of @internal
. Of those, the majority are for missing short and long descriptions (inline tag). Of the remainder, some are normal tags which should be inline tags, and only a few are actually used for what are intended to be private functions. Those elements which use the normal tag incorrectly, will not be appearing in WP-Parser results.
To summarise:
@access
is being used more incorrectly than correctly according to our documentation standards.@access
is inconsistent with the code it is describing.@access
is not in PSR-5, and is generally redundant.@internal
is in PSR-5, is more flexible and more descriptive.@internal
is not used correctly, causing incorrect WP-Parser results and therefore missing entries on DevHub code reference.
A plan:
- Convert normal
@internal
tags to inline{@internal ...}}
tags where appropriate. - Remove
@access public
and@access protected
from everywhere. - Convert
@access private
to@internal
for functions where correct. - Remove remaining
@access private
instances.
Thoughts?
Change History (4)
#2
@
11 years ago
We still use the var
keyword plus @access private|protected
for all properties introduced in a class prior to WP 3.2 (when we moved to PHP 5.2). Properties meant to be explicit get shifted to public
, but we deliberately don't want to specify a property meant to be private or protected as public
and we also can't use private
or protected
because that can cause major errors in existing plugins.
Thus the examples that follow "Many of them are also wrong anyway and contradict the code" are actually correct.
WP Parser can parse whatever we tell it to. For this case, we can easily parse @access for var
properties and determine whether to document it as private or protected. Same for methods.
I'm fine with converting @access public
+ var
to public
, but as voiced in #22234, this isn't just a find and replace. It needs to happen with a fine-toothed comb to make sure the documentation is actually correct, otherwise we back ourselves into a corner.
We can move to @internal for private-use functions. But @access private|protected
for methods meant to be private or protected is more explicit to me.
We know we're not following PSR-5, we're just doing what is practical for a codebase riddled with private-use functions, properties, and methods that we can't actually lock down without breaking things.
My thought is that Trac can be a poor venue for discussions of this nature. Is this the result of an IRC or some other discussion (either real-time or async) with those heavily involved in inline docs? It would be good to cross-reference if so.